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

Issue 2862563003: Landing Recent QUIC changes until Sat Apr 29 00:22:04 2017 +0000 (Closed)

Created:
3 years, 7 months ago by Buck
Modified:
3 years, 7 months ago
Reviewers:
Ryan Hamilton
CC:
chromium-reviews, cbentzel+watch_chromium.org, net-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Landing Recent QUIC changes until Sat Apr 29 00:22:04 2017 +0000 There are no flags to be updated deprecate FLAGS_quic_reloadable_flag_quic_bbr_keep_sending_at_recent_rate in disabled state. It achieved results that were in general comparable to "measuring aggregation" approach, but it had worse rtx rate, and worse QoE, and did not actually seem to be better in other regards. Merge internal change: 154601526 https://codereview.chromium.org/2859913003/ fix packet conservation to remove packets from recovery window when they are lost. Protected by FLAGS_quic_reloadable_flag_quic_bbr_fix_conservation2. This causes a dramatic reduction in losses compared to the previous patch. The transfer time is approximately the same. Merge internal change: 154590929 https://codereview.chromium.org/2857243002/ Fix typo in comments. Merge internal change: 154539040 https://codereview.chromium.org/2854823004/ QUIC - refactor stream creation. Guarded by --quic_reloadable_flag_quic_refactor_stream_creation. This CL is in preparation for HTTP stream pairs changes. A prototype of the entire HTTP stream pairs work is in progress. This CL is one subset of that (2nd in the series). [ HTTP stream pairs builds upon this, by having new QuicSpdySession overrides that implicitly create headers streams, and abstracting stream accounting as needed. ] Add a new factory like method QuicSession::CreateStream(), that unconditionally creates a new stream. It's virtual so subclasses of QuicSession can create streams that are appropriately specialized from QuicStream. There are new versions of Create*DynamicStream() called MaybeCreate*DynamicStream(), and ShouldCreate*DynamicStream() called ShouldCreate*DynamicStream2(). Separating this out means that fewer overrides of Create*DynamicStream() and ShouldCreate*DynamicStream() will necessary, because many of those were actually only needed to specialize the steam types. Those will go away when FLAGS_quic_reloadable_flag_quic_refactor_stream_creation is deprecated. Merge internal changes: 154351257, 154373998. https://codereview.chromium.org/2861673004/ QUIC - stream id refactor for tests. // AKA - http stream pairs bikeshed #1. This CL is in preparation for HTTP stream pairs changes. A prototype of the entire HTTP stream pairs work is in progress. This CL is one subset of that. Rework QUIC tests so that they do not hard code the assumption that all headers are on stream 3. Also, prepare to calculate stream ids according to whether stream pairs are enabled, i.e. whether they increment by 2 or 4. Merge internal change: 150949425 https://codereview.chromium.org/2854833005/ R=rch@chromium.org BUG= Review-Url: https://codereview.chromium.org/2862563003 Cr-Original-Commit-Position: refs/heads/master@{#469218} Committed: https://chromium.googlesource.com/chromium/src/+/d60018e0b72b5708797cebe57780ad559877164f Review-Url: https://codereview.chromium.org/2862563003 Cr-Commit-Position: refs/heads/master@{#469528} Committed: https://chromium.googlesource.com/chromium/src/+/bf2f59c91d37384ee7196d42b081aa70b458518b

Patch Set 1 #

Patch Set 2 : rebase and fix test bugs detected by swarm bot. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1306 lines, -739 lines) Patch
M net/http/http_stream_factory_impl_unittest.cc View 3 chunks +8 lines, -4 lines 0 comments Download
M net/quic/chromium/bidirectional_stream_quic_impl_unittest.cc View 1 15 chunks +24 lines, -17 lines 0 comments Download
M net/quic/chromium/quic_chromium_client_session.h View 1 chunk +2 lines, -0 lines 0 comments Download
M net/quic/chromium/quic_chromium_client_session.cc View 2 chunks +6 lines, -0 lines 0 comments Download
M net/quic/chromium/quic_chromium_client_session_test.cc View 1 21 chunks +52 lines, -39 lines 0 comments Download
M net/quic/chromium/quic_chromium_client_stream_test.cc View 5 chunks +24 lines, -1 line 0 comments Download
M net/quic/chromium/quic_http_stream_test.cc View 1 22 chunks +65 lines, -39 lines 0 comments Download
M net/quic/chromium/quic_network_transaction_unittest.cc View 60 chunks +277 lines, -232 lines 0 comments Download
M net/quic/chromium/quic_stream_factory_test.cc View 34 chunks +96 lines, -76 lines 0 comments Download
M net/quic/core/congestion_control/bbr_sender.h View 1 chunk +2 lines, -1 line 0 comments Download
M net/quic/core/congestion_control/bbr_sender.cc View 5 chunks +50 lines, -24 lines 0 comments Download
M net/quic/core/congestion_control/bbr_sender_test.cc View 4 chunks +2 lines, -75 lines 0 comments Download
M net/quic/core/frames/quic_window_update_frame.h View 1 chunk +1 line, -1 line 0 comments Download
M net/quic/core/quic_client_promised_info_test.cc View 3 chunks +7 lines, -2 lines 0 comments Download
M net/quic/core/quic_client_push_promise_index_test.cc View 2 chunks +5 lines, -1 line 0 comments Download
M net/quic/core/quic_connection_test.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M net/quic/core/quic_flags_list.h View 2 chunks +6 lines, -6 lines 0 comments Download
M net/quic/core/quic_headers_stream_test.cc View 14 chunks +37 lines, -20 lines 0 comments Download
M net/quic/core/quic_packet_creator_test.cc View 5 chunks +13 lines, -10 lines 0 comments Download
M net/quic/core/quic_server_session_base.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M net/quic/core/quic_server_session_base_test.cc View 10 chunks +54 lines, -28 lines 0 comments Download
M net/quic/core/quic_session.h View 5 chunks +41 lines, -0 lines 0 comments Download
M net/quic/core/quic_session.cc View 3 chunks +79 lines, -1 line 0 comments Download
M net/quic/core/quic_session_test.cc View 12 chunks +53 lines, -23 lines 0 comments Download
M net/quic/core/quic_spdy_session.h View 2 chunks +11 lines, -0 lines 0 comments Download
M net/quic/core/quic_spdy_session.cc View 1 chunk +16 lines, -0 lines 0 comments Download
M net/quic/core/quic_spdy_stream_test.cc View 22 chunks +48 lines, -32 lines 0 comments Download
M net/quic/core/quic_stream_sequencer_test.cc View 3 chunks +8 lines, -5 lines 0 comments Download
M net/quic/core/quic_write_blocked_list_test.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M net/quic/core/spdy_utils.h View 1 chunk +2 lines, -0 lines 0 comments Download
M net/quic/quartc/quartc_session.h View 1 chunk +1 line, -0 lines 0 comments Download
M net/quic/quartc/quartc_session.cc View 2 chunks +7 lines, -0 lines 0 comments Download
M net/quic/test_tools/quic_spdy_session_peer.h View 1 chunk +12 lines, -0 lines 0 comments Download
M net/quic/test_tools/quic_spdy_session_peer.cc View 1 chunk +19 lines, -0 lines 0 comments Download
M net/quic/test_tools/quic_test_utils.h View 10 chunks +33 lines, -9 lines 0 comments Download
M net/quic/test_tools/quic_test_utils.cc View 1 chunk +12 lines, -2 lines 0 comments Download
M net/tools/quic/end_to_end_test.cc View 7 chunks +22 lines, -8 lines 0 comments Download
M net/tools/quic/quic_client_base.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M net/tools/quic/quic_client_session.h View 4 chunks +6 lines, -8 lines 0 comments Download
M net/tools/quic/quic_client_session.cc View 6 chunks +27 lines, -4 lines 0 comments Download
M net/tools/quic/quic_client_session_test.cc View 3 chunks +23 lines, -4 lines 0 comments Download
M net/tools/quic/quic_dispatcher_test.cc View 1 chunk +1 line, -0 lines 0 comments Download
M net/tools/quic/quic_simple_server_session.h View 1 chunk +5 lines, -0 lines 0 comments Download
M net/tools/quic/quic_simple_server_session.cc View 3 chunks +12 lines, -2 lines 0 comments Download
M net/tools/quic/quic_simple_server_session_test.cc View 19 chunks +89 lines, -44 lines 0 comments Download
M net/tools/quic/quic_simple_server_stream_test.cc View 4 chunks +15 lines, -12 lines 0 comments Download
M net/tools/quic/quic_spdy_client_stream_test.cc View 3 chunks +7 lines, -2 lines 0 comments Download
M net/tools/quic/quic_spdy_server_stream_base_test.cc View 2 chunks +4 lines, -1 line 0 comments Download
M net/tools/quic/test_tools/quic_test_server.cc View 2 chunks +9 lines, -0 lines 0 comments Download

Messages

Total messages: 28 (18 generated)
Buck
3 years, 7 months ago (2017-05-03 22:28:45 UTC) #1
Ryan Hamilton
Please update the CL description with links to the individual chrome CLs. (THere's a script ...
3 years, 7 months ago (2017-05-03 23:11:34 UTC) #4
Ryan Hamilton
LGTM once the CL description is updated
3 years, 7 months ago (2017-05-03 23:12:00 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2862563003/1
3 years, 7 months ago (2017-05-03 23:43:08 UTC) #12
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/d60018e0b72b5708797cebe57780ad559877164f
3 years, 7 months ago (2017-05-04 00:31:05 UTC) #15
johnme
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2856243003/ by johnme@chromium.org. ...
3 years, 7 months ago (2017-05-04 14:01:49 UTC) #16
Buck
Applied fixes from https://codereview.chromium.org/2854833005/. Should be ready to go again.
3 years, 7 months ago (2017-05-04 22:12:28 UTC) #20
Ryan Hamilton
lgtm
3 years, 7 months ago (2017-05-04 22:49:13 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2862563003/20001
3 years, 7 months ago (2017-05-04 23:47:13 UTC) #25
commit-bot: I haz the power
3 years, 7 months ago (2017-05-04 23:54:56 UTC) #28
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/bf2f59c91d37384ee7196d42b081...

Powered by Google App Engine
This is Rietveld 408576698