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

Issue 2692813002: Server push cancellation: add a finch trial parameter (Closed)

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

Description

Server push cancellation: add a finch trial parameter "enable_server_push_cancellation" to control the feature of server push cancellation. Create a HttpCacheLookupManager in HttpCache's constructor and set it to HttpNetworkSession which will set it thru reaching QuicChromiumClientSession and SpdySession. BUG=232040 Review-Url: https://codereview.chromium.org/2692813002 Cr-Original-Original-Commit-Position: refs/heads/master@{#454457} Committed: https://chromium.googlesource.com/chromium/src/+/c83d602cac3c9a7a7ef554d172b7a8cd805f13e8 Review-Url: https://codereview.chromium.org/2692813002 Cr-Original-Commit-Position: refs/heads/master@{#454679} Committed: https://chromium.googlesource.com/chromium/src/+/f04bd2c84154b59a3d870993051df4be0b13964f Review-Url: https://codereview.chromium.org/2692813002 Cr-Commit-Position: refs/heads/master@{#464737} Committed: https://chromium.googlesource.com/chromium/src/+/d7dd2db1ad3596d682aa7fd56d3fef6f218250ec

Patch Set 1 #

Patch Set 2 : sync with master #

Patch Set 3 : Add integration tests #

Patch Set 4 : sync with master #

Patch Set 5 : self review #

Total comments: 6

Patch Set 6 : collect server rst stats to verify the push cancellation #

Patch Set 7 : self review #

Patch Set 8 : sync with internal code #

Patch Set 9 : sync with internal code #

Total comments: 3

Patch Set 10 : sync after internal code landed #

Patch Set 11 : sync with master #

Patch Set 12 : fix SpdySessionTest setup after SetServerPushDelegate now only sets delegate when param enabled #

Patch Set 13 : sync with origin/master #

Patch Set 14 : Add tests files to BUILD.gn for ios usage #

Patch Set 15 : explicitly initialize the enable_server_push_cancellation to false #

Patch Set 16 : Merge branch 'master' of https://chromium.googlesource.com/chromium/src into SPC_Finch_2 #

Patch Set 17 : sync with master #

Patch Set 18 : fix flakiness #

Patch Set 19 : Run->RunUntilIdle #

Patch Set 20 : rebase with master #

Patch Set 21 : reabse with master #

Patch Set 22 : Spin the message loop to ensure the client receives the response #

Unified diffs Side-by-side diffs Delta from patch set Stats (+345 lines, -11 lines) Patch
M components/network_session_configurator/network_session_configurator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +11 lines, -0 lines 0 comments Download
M components/network_session_configurator/network_session_configurator_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +12 lines, -0 lines 0 comments Download
M net/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +4 lines, -0 lines 0 comments Download
M net/http/http_cache.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +15 lines, -8 lines 0 comments Download
M net/http/http_network_session.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +1 line, -0 lines 0 comments Download
M net/http/http_network_session.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +4 lines, -1 line 0 comments Download
M net/quic/core/quic_server_session_base_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 3 chunks +3 lines, -0 lines 0 comments Download
M net/quic/core/quic_session.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +3 lines, -0 lines 0 comments Download
M net/quic/core/quic_session.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +4 lines, -0 lines 0 comments Download
M net/spdy/spdy_session_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -0 lines 0 comments Download
M net/spdy/spdy_test_util_common.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -0 lines 0 comments Download
M net/spdy/spdy_test_util_common.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 2 chunks +3 lines, -0 lines 0 comments Download
M net/tools/quic/quic_dispatcher.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +5 lines, -0 lines 0 comments Download
M net/tools/quic/quic_dispatcher.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -0 lines 0 comments Download
M net/tools/quic/quic_simple_dispatcher.h View 1 2 3 4 5 6 7 8 2 chunks +7 lines, -0 lines 0 comments Download
M net/tools/quic/quic_simple_dispatcher.cc View 1 2 3 4 5 6 7 1 chunk +20 lines, -0 lines 0 comments Download
M net/tools/quic/quic_simple_server_session_test.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 5 chunks +5 lines, -0 lines 0 comments Download
M net/tools/quic/test_tools/mock_quic_session_visitor.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M net/url_request/url_request_quic_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 5 chunks +242 lines, -1 line 0 comments Download
M net/url_request/url_request_test_util.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 92 (65 generated)
Zhongyi Shi
Ryan and Buck, integration tests for server push cancellation, ptal! Let me if we have ...
3 years, 10 months ago (2017-02-16 07:39:50 UTC) #5
Ryan Hamilton
Sweet. Will think more about the test. https://codereview.chromium.org/2692813002/diff/100001/net/http/http_cache.cc File net/http/http_cache.cc (right): https://codereview.chromium.org/2692813002/diff/100001/net/http/http_cache.cc#newcode322 net/http/http_cache.cc:322: if (session) ...
3 years, 10 months ago (2017-02-16 15:15:07 UTC) #6
Ryan Hamilton
https://codereview.chromium.org/2692813002/diff/100001/net/url_request/url_request_quic_unittest.cc File net/url_request/url_request_quic_unittest.cc (right): https://codereview.chromium.org/2692813002/diff/100001/net/url_request/url_request_quic_unittest.cc#newcode241 net/url_request/url_request_quic_unittest.cc:241: TEST_F(URLRequestQuicTest, CancelPushIfCached) { On 2017/02/16 07:39:50, Zhongyi Shi wrote: ...
3 years, 10 months ago (2017-02-16 15:18:09 UTC) #7
Ryan Hamilton
3 years, 10 months ago (2017-02-16 15:18:10 UTC) #8
Zhongyi Shi
Added a new method in QuicSession::Visitor to collect reset(s) received from the peer. Make QuicDispatcher ...
3 years, 10 months ago (2017-02-19 20:19:53 UTC) #9
Zhongyi Shi
This CL is updated with the internal CL landed. This now checks RstErrorCode received on ...
3 years, 10 months ago (2017-02-22 20:34:35 UTC) #10
Ryan Hamilton
lgtm https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc File net/quic/core/quic_session.cc (right): https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc#newcode99 net/quic/core/quic_session.cc:99: visitor_->OnRstStreamReceived(frame); I think this should have {}s since ...
3 years, 10 months ago (2017-02-22 23:12:33 UTC) #11
Zhongyi Shi
https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc File net/quic/core/quic_session.cc (right): https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc#newcode99 net/quic/core/quic_session.cc:99: visitor_->OnRstStreamReceived(frame); On 2017/02/22 23:12:33, Ryan Hamilton wrote: > I ...
3 years, 10 months ago (2017-02-23 00:19:46 UTC) #12
Ryan Hamilton
lgtm https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc File net/quic/core/quic_session.cc (right): https://codereview.chromium.org/2692813002/diff/180001/net/quic/core/quic_session.cc#newcode99 net/quic/core/quic_session.cc:99: visitor_->OnRstStreamReceived(frame); On 2017/02/23 00:19:46, Zhongyi Shi wrote: > ...
3 years, 10 months ago (2017-02-23 00:27:00 UTC) #13
Buck
lgtm
3 years, 10 months ago (2017-02-23 18:26:26 UTC) #14
Buck
lgtm
3 years, 10 months ago (2017-02-23 18:26:27 UTC) #15
Zhongyi Shi
I forgot to fix spdy session unittests, the setup now need to set up the ...
3 years, 10 months ago (2017-02-24 00:00:16 UTC) #20
Ryan Hamilton
lgtm
3 years, 10 months ago (2017-02-24 00:06:06 UTC) #23
Zhongyi Shi
The test data files were missing on ios and caused the unittests failing, which is ...
3 years, 9 months ago (2017-03-02 23:41:39 UTC) #28
Ryan Hamilton
lgtm
3 years, 9 months ago (2017-03-03 00:11:18 UTC) #31
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/2692813002/280001
3 years, 9 months ago (2017-03-03 00:26:09 UTC) #34
commit-bot: I haz the power
Committed patchset #14 (id:280001) as https://chromium.googlesource.com/chromium/src/+/c83d602cac3c9a7a7ef554d172b7a8cd805f13e8
3 years, 9 months ago (2017-03-03 01:39:39 UTC) #37
alph
A revert of this CL (patchset #14 id:280001) has been created in https://codereview.chromium.org/2730053002/ by alph@chromium.org. ...
3 years, 9 months ago (2017-03-03 10:03:49 UTC) #38
Zhongyi Shi
Reland this cl after explicitly initialize enable_server_push_cancellation to false. The previous land did pass the ...
3 years, 9 months ago (2017-03-03 21:11:14 UTC) #44
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/2692813002/300001
3 years, 9 months ago (2017-03-03 21:18:07 UTC) #47
commit-bot: I haz the power
Committed patchset #15 (id:300001) as https://chromium.googlesource.com/chromium/src/+/f04bd2c84154b59a3d870993051df4be0b13964f
3 years, 9 months ago (2017-03-03 21:24:57 UTC) #50
sclittle
A revert of this CL (patchset #15 id:300001) has been created in https://codereview.chromium.org/2728183003/ by sclittle@chromium.org. ...
3 years, 9 months ago (2017-03-03 23:22:52 UTC) #51
Ryan Hamilton
zhongyi: is this ready to re-land?
3 years, 8 months ago (2017-04-04 19:22:03 UTC) #72
Zhongyi Shi
On 2017/04/04 19:22:03, Ryan Hamilton wrote: > zhongyi: is this ready to re-land? Not really. ...
3 years, 8 months ago (2017-04-05 00:09:08 UTC) #73
Zhongyi Shi
The integration test failure on windows was caused by indefinite order of response. The new ...
3 years, 8 months ago (2017-04-14 16:47:15 UTC) #86
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/2692813002/460001
3 years, 8 months ago (2017-04-14 16:54:53 UTC) #89
commit-bot: I haz the power
3 years, 8 months ago (2017-04-14 17:03:18 UTC) #92
Message was sent while issue was closed.
Committed patchset #22 (id:460001) as
https://chromium.googlesource.com/chromium/src/+/d7dd2db1ad3596d682aa7fd56d3f...

Powered by Google App Engine
This is Rietveld 408576698