okhttp: fix incorrect connection-level flow control handling at beginning of connection

[chrisschek]

Addressing #6685

Specifically, this addresses bugs that occur when the OkHttpChannelBuilder.flowControlWindow(int) setting is increased from its default value.

Two changes:

1. On starting a connection, ensure the value of OkHttpChannelBuilder.flowControlWindow(int) is sent via Settings.INITIAL_WINDOW_SIZE. Also send a WINDOW_UPDATE after Settings to update the connection-level window.
2. Always initialize the OutboundFlowController with an initialWindowSize of 65335 bytes per the http2 spec instead of using the inbound window size.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ chrisschek (ea6a6d0, 22eb66e, aac0867, 14ada64, 8da3539)

[ejona86]

Thanks for sending this out. I didn't send it out earlier because it clearly deserved some tests. I was on vacation for a bit over a week, thus the delay.

Thank you testing this and confirming the OutboundFlowController argument was garbage.

You'd need to sign the CLA for us to accept the PR. We can work on some tests and make a PR, if that is easiest.

[chrisschek]

I signed it

[chrisschek]

Hm, I signed the CLA (both on EasyCLA and the CNCF one linked in the contributing docs last week but it doesn't seem to be picking that up.

I took a stab at testing the changes:

1. Was pretty straightforward for the OutboundFlowController argument; the outboundFlowControl_* tests seemed redundant after my fix so consolidated them into a single test.
2. For sending INITIAL_WINDOW_SIZE in the first Settings frame, testing was trickier since that code doesn't normally run in the test. Not super happy with my end product having to change source code for the sake of testing, but seemed like a necessary evil.

[chrisschek]

Mystery solved – my commits had the wrong email address on them. :)

[ejona86]

Looks pretty good. Just one thing to fix-up.

[ejona86]

I think these three tests should still be around. They are trying to test that the outbound flow controller observes the flow control given to it. Since passing windowSize to startTransport() no longer works, these need to get something like frameHandler().settings(false, new Settings()); (but with the INITIAL_WINDOW_SIZE set to windowSize) to change the window size available.

[chrisschek]

Done. I initially thought those cases were already covered by outboundFlowControlWithInitialWindowSizeChange(), but realized it wasn't testing where the initial window size is changed before the stream is started.

[ejona86]

@chrisschek, thank you! I was on vacation last week so I'm happy that you worked on this.

Sorry you were bit by this silly bug!

[chrisschek]

@ejona86 not a problem, happy to contribute :)