Clear cached QUIC network stats when a QUIC handshake fails.

Clear cached QUIC network stats when a QUIC handshake fails.

BUG=

Review-Url: https://codereview.chromium.org/2699003002
Cr-Commit-Position: refs/heads/master@{#457196}
Committed: https://chromium.googlesource.com/chromium/src/+/ac7f35e6ac21e1722cd35cfa851a19af08b400a4

[Ryan Hamilton, 2017-02-16 23:13:01]

rch@chromium.org changed reviewers:
+ jri@chromium.org

[Jana, 2017-02-17 02:52:13]

One clarification question and a nit.

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
File net/http/http_server_properties_impl_unittest.cc (right):

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
net/http/http_server_properties_impl_unittest.cc:1120:
impl_.ClearServerNetworkStats(foo_https_server);
Nit: can you do a GetServerNetworkStats() before the clear, and confirm that
it's non-null?

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
File net/http/http_server_properties_manager.cc (right):

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
net/http/http_server_properties_manager.cc:321:
ScheduleUpdatePrefsOnNetworkThread(CLEAR_SERVER_NETWORK_STATS);
What's this for?

[Ryan Hamilton, 2017-02-17 03:41:53]

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
File net/http/http_server_properties_impl_unittest.cc (right):

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
net/http/http_server_properties_impl_unittest.cc:1120:
impl_.ClearServerNetworkStats(foo_https_server);
On 2017/02/17 02:52:12, Jana wrote:
> Nit: can you do a GetServerNetworkStats() before the clear, and confirm that
> it's non-null?

I can but that's tested in the previous test, and the TotT guides say not to
have redundant expectations in tests. (In other words, from the previous test we
proved that SetStats() works. We don't need to re-prove it here)

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
File net/http/http_server_properties_manager.cc (right):

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
net/http/http_server_properties_manager.cc:321:
ScheduleUpdatePrefsOnNetworkThread(CLEAR_SERVER_NETWORK_STATS);
On 2017/02/17 02:52:12, Jana wrote:
> What's this for?

The server properties manager persists the properties on the prefs thread, but
it only does this if the state actually changes. This method schedules the
persisting and the if() makes sure we only do that if state changed. Does that
help?

[Ryan Hamilton, 2017-02-17 03:41:58]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-17 03:43:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Jana, 2017-02-17 05:42:33]

lgtm

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
File net/http/http_server_properties_manager.cc (right):

https://codereview.chromium.org/2699003002/diff/1/net/http/http_server_proper...
net/http/http_server_properties_manager.cc:321:
ScheduleUpdatePrefsOnNetworkThread(CLEAR_SERVER_NETWORK_STATS);
On 2017/02/17 03:41:53, Ryan Hamilton wrote:
> On 2017/02/17 02:52:12, Jana wrote:
> > What's this for?
> 
> The server properties manager persists the properties on the prefs thread, but
> it only does this if the state actually changes. This method schedules the
> persisting and the if() makes sure we only do that if state changed. Does that
> help?

Gotcha. Ok, makes sense, thanks.

[commit-bot: I haz the power, 2017-02-17 06:26:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 06:26:14]

Dry run: This issue passed the CQ dry run.

[Ryan Hamilton, 2017-02-17 15:06:32]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2017-02-17 15:06:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-17 15:12:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-17 15:12:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Hamilton, 2017-03-15 19:24:16]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2017-03-15 19:25:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 19:35:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-15 19:35:51]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Hamilton, 2017-03-15 19:40:39]

rch@chromium.org changed reviewers:
+ isherman@chromium.org

[Ryan Hamilton, 2017-03-15 19:40:40]

isherman: histograms.xml OWNERS review please

[Ilya Sherman, 2017-03-15 20:34:47]

histograms.xml lgtm

[Ryan Hamilton, 2017-03-15 20:35:33]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2017-03-15 20:36:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-15 20:42:34]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489610133787980, "parent_rev":
"16816d2bd7b1ee49b2f947e2a1a62137bad1e8fe", "commit_rev":
"e3d3ae4d31119ad05c361865770a3a4ac30aff03"}

[commit-bot: I haz the power, 2017-03-15 20:47:36]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489610133787980, "parent_rev":
"e749339c00a7bb8daba0f6c26dfb36f44af8329e", "commit_rev":
"ac7f35e6ac21e1722cd35cfa851a19af08b400a4"}

[commit-bot: I haz the power, 2017-03-15 20:48:18]

Description was changed from

==========
Clear cached QUIC network stats when a QUIC handshake fails.

BUG=
==========

to

==========
Clear cached QUIC network stats when a QUIC handshake fails.

BUG=

Review-Url: https://codereview.chromium.org/2699003002
Cr-Commit-Position: refs/heads/master@{#457196}
Committed:
https://chromium.googlesource.com/chromium/src/+/ac7f35e6ac21e1722cd35cfa851a...
==========

[commit-bot: I haz the power, 2017-03-15 20:48:19]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/ac7f35e6ac21e1722cd35cfa851a...