Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(93)

Issue 242583002: Disable QuicFlowController for QUIC versions < QUIC_VERSION_17 (Closed)

Created:
6 years, 8 months ago by ramant (doing other things)
Modified:
6 years, 8 months ago
Reviewers:
Ryan Hamilton
CC:
chromium-reviews, cbentzel+watch_chromium.org
Visibility:
Public.

Description

Disable QuicFlowController for QUIC versions < QUIC_VERSION_17 Chrome Beta was happily advertising a flow control receive window of length 0, under the assumption that because the version of QUIC being spoken didn't support flow control, a zero length window advertisement would have no adverse effects. On the server end, the refactoring in internal change: 64733866 resulted in the QUIC version not being checked when querying QuicFlowController::IsBlocked. IsBlocked would look at the client's advertised receive window, and conclude that no data could be sent. This would never change as the flow control accounting is protected behind version checks. Ultimately this means that once a stream was marked as write blocked, it would never resume writing: https://codereview.chromium.org/242453002/diff/1/net/quic/quic_session.cc?context=&column_width=80 (line# 286) resulting in a connection timeout. This CL explicitly disables the QuicFlowController when the negotiated QUIC version is < QUIC_VERSION_17. Merge internal change: 65137349 This internal change was LGTM'ed by rch. Wanted to port this change to disable flow control. R=rch@chromium.org BUG=

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -2 lines) Patch
M net/quic/reliable_quic_stream.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M net/quic/reliable_quic_stream_test.cc View 4 chunks +29 lines, -2 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
ramant (doing other things)
6 years, 8 months ago (2014-04-18 02:43:59 UTC) #1
Ryan Hamilton
6 years, 8 months ago (2014-04-18 03:39:03 UTC) #2
lgtm

Powered by Google App Engine
This is Rietveld 408576698