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

Issue 427313002: Persist the server config that is received via kSCUP. (Closed)

Created:
6 years, 4 months ago by ramant (doing other things)
Modified:
6 years, 3 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, Robbie Shade, avd, wtc, agl
Project:
chromium
Visibility:
Public.

Description

Persist the server config that is received via kSCUP. R=rjshade@chromium.org, rch@chromium.org, asvitkine@chromium.org, rvargas@chromium.org Committed: https://crrev.com/c584fb92ddd78cb2c2a36b90f0d7be30227d3cff Cr-Commit-Position: refs/heads/master@{#295582}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Verify proof from kSCUP message #

Total comments: 4

Patch Set 3 : Added histogram and minor cleanup per comments #

Total comments: 2

Patch Set 4 : Keep disk cache entry open for multiple writes #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+135 lines, -48 lines) Patch
M net/http/disk_cache_based_quic_server_info.cc View 1 2 3 3 chunks +8 lines, -3 lines 2 comments Download
M net/quic/quic_crypto_client_stream.h View 1 2 3 2 chunks +17 lines, -0 lines 0 comments Download
M net/quic/quic_crypto_client_stream.cc View 1 2 3 4 chunks +96 lines, -45 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +14 lines, -0 lines 0 comments Download

Messages

Total messages: 30 (11 generated)
ramant (doing other things)
6 years, 4 months ago (2014-07-30 19:03:15 UTC) #1
ramant (doing other things)
Persisted server config if proof is valid after chromium receives STK and server config update. ...
6 years, 4 months ago (2014-07-30 19:46:36 UTC) #2
wtc
https://codereview.chromium.org/427313002/diff/1/net/quic/quic_crypto_client_stream.cc File net/quic/quic_crypto_client_stream.cc (right): https://codereview.chromium.org/427313002/diff/1/net/quic/quic_crypto_client_stream.cc#newcode162 net/quic/quic_crypto_client_stream.cc:162: if (cached->proof_valid()) { If we have access to the ...
6 years, 4 months ago (2014-07-31 00:43:06 UTC) #3
ramant (doing other things)
https://codereview.chromium.org/427313002/diff/1/net/quic/quic_crypto_client_stream.cc File net/quic/quic_crypto_client_stream.cc (right): https://codereview.chromium.org/427313002/diff/1/net/quic/quic_crypto_client_stream.cc#newcode162 net/quic/quic_crypto_client_stream.cc:162: if (cached->proof_valid()) { On 2014/07/31 00:43:06, wtc wrote: > ...
6 years, 4 months ago (2014-08-01 19:52:22 UTC) #4
chromium-reviews
The intention is to send certs and proofs in the SCUP. cl/71770356 is attempting to ...
6 years, 4 months ago (2014-08-01 19:54:37 UTC) #5
Ryan Hamilton
I think this looks pretty good. One issue we discussed offline is to make sure ...
6 years, 3 months ago (2014-09-15 22:37:56 UTC) #8
ramant (doing other things)
Hi Ryan, Made the changes, we have discussed. PTAL. thanks raman https://codereview.chromium.org/427313002/diff/60001/net/quic/quic_crypto_client_stream.cc File net/quic/quic_crypto_client_stream.cc (right): ...
6 years, 3 months ago (2014-09-16 21:06:33 UTC) #10
Ryan Hamilton
this change LGTM. I'd prefer to see the states simplified, but I'd be happy to ...
6 years, 3 months ago (2014-09-17 15:52:42 UTC) #11
ramant (doing other things)
+agl@: would appreciate your comments. +asvitkine@: for histograms.xml changes. https://codereview.chromium.org/427313002/diff/100001/net/quic/quic_crypto_client_stream.cc File net/quic/quic_crypto_client_stream.cc (right): https://codereview.chromium.org/427313002/diff/100001/net/quic/quic_crypto_client_stream.cc#newcode467 net/quic/quic_crypto_client_stream.cc:467: ...
6 years, 3 months ago (2014-09-17 22:13:04 UTC) #14
ramant (doing other things)
Hi Ryan and Ricardo, Made a small change to keep disk cache entry open for ...
6 years, 3 months ago (2014-09-18 03:52:13 UTC) #19
Alexei Svitkine (slow)
lgtm
6 years, 3 months ago (2014-09-18 13:41:01 UTC) #20
agl
lgtm
6 years, 3 months ago (2014-09-18 19:11:48 UTC) #22
rjshade
lgtm, thanks for doing this and looking forward to seeing the results https://codereview.chromium.org/427313002/diff/180001/net/http/disk_cache_based_quic_server_info.cc File net/http/disk_cache_based_quic_server_info.cc ...
6 years, 3 months ago (2014-09-18 19:42:51 UTC) #23
Ryan Hamilton
Still LGTM https://codereview.chromium.org/427313002/diff/180001/net/http/disk_cache_based_quic_server_info.cc File net/http/disk_cache_based_quic_server_info.cc (right): https://codereview.chromium.org/427313002/diff/180001/net/http/disk_cache_based_quic_server_info.cc#newcode227 net/http/disk_cache_based_quic_server_info.cc:227: entry_ = data_shim_->entry; On 2014/09/18 19:42:51, rjshade ...
6 years, 3 months ago (2014-09-18 19:52:52 UTC) #24
ramant (doing other things)
On 2014/09/18 19:52:52, Ryan Hamilton wrote: > Still LGTM > > https://codereview.chromium.org/427313002/diff/180001/net/http/disk_cache_based_quic_server_info.cc > File net/http/disk_cache_based_quic_server_info.cc ...
6 years, 3 months ago (2014-09-18 21:07:16 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/427313002/180001
6 years, 3 months ago (2014-09-18 21:08:13 UTC) #27
commit-bot: I haz the power
Committed patchset #4 (id:180001) as 6e75c2235199be33b0e598360473721cb5e09460
6 years, 3 months ago (2014-09-18 22:19:33 UTC) #28
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/c584fb92ddd78cb2c2a36b90f0d7be30227d3cff Cr-Commit-Position: refs/heads/master@{#295582}
6 years, 3 months ago (2014-09-18 22:20:15 UTC) #29
rvargas (doing something else)
6 years, 3 months ago (2014-09-19 00:27:31 UTC) #30
Message was sent while issue was closed.
lgtm

Powered by Google App Engine
This is Rietveld 408576698