RFR: 8342075: HttpClient: improve HTTP/2 flow control checks [v2]

[Daniel Fuchs]

On Tue, 22 Oct 2024 13:47:32 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Apply suggestions from code review
>>   
>>   Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 1109:
> 
>> 1107:     // having been added to any Stream::inputQ. In that case, the number
>> 1108:     // of unprocessed bytes hasn't been incremented by the stream, and
>> 1109:     // does not need to be decremented.
> 
> Hello Daniel, this comment looks a bit contradictory or confusing, since it says that this method gets called before/without the dataframe being added to the Stream::inputQ. But looking at the releaseUnconsumed method's implementation this dropDataFrame is being called from the releaseUnconsumed method, whose comment states "... called when a DataFrame that was added to a Stream::inputQ".

Maybe I should say "is called directly" ? The idea is that if the data is dropped before being added to the inputQ, then `dropDataFrame` is called directly. If the data is dropped after having been added to the inputQ, then `releaseUnconsumed` is called.

> src/java.net.http/share/classes/jdk/internal/net/http/WindowUpdateSender.java line 107:
> 
>> 105:      * the bytes from the flow control window.
>> 106:      *
>> 107:      * @implSpec
> 
> Is it intentional to use `@implSpec` here on an internal class. Or is it just to convey the intention?

It's to convey intention. There are two subclasses.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1810818124
PR Review Comment: https://git.openjdk.org/jdk/pull/21567#discussion_r1810820079