HistoricNetworkStatsInfoToValue should use statistics prefs values

HistoricNetworkStatsInfoToValue should use statistics prefs values.

Previously, the historic network stats values on the net-internals
page were only updated every hour. Now, instead of using local
prefs, HistoricNetworkStatsInfoToValue reads from statistics prefs,
and the values are up to date.

BUG=403867
Committed: https://crrev.com/e1af51dc64c3bfd83d855dbdbf17d8bd4563bae5
Cr-Commit-Position: refs/heads/master@{#319749}

[megjablon, 2015-03-06 21:51:42]

megjablon@chromium.org changed reviewers:
+ bengr@chromium.org, thestig@chromium.org

[megjablon, 2015-03-06 21:51:42]

bengr: components/*
thestig: chrome/*

[Lei Zhang, 2015-03-06 21:57:08]

thestig@chromium.org changed reviewers:
+ mmenke@chromium.org

[Lei Zhang, 2015-03-06 21:57:08]

I defer to mmenke for chrome/.../net_internals

[mmenke, 2015-03-06 22:10:40]

mmenke@chromium.org changed reviewers:
- thestig@chromium.org

[mmenke, 2015-03-06 22:20:23]

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
File chrome/test/data/webui/net_internals/bandwidth_view.js (right):

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
chrome/test/data/webui/net_internals/bandwidth_view.js:148: // the session
information, because previously there was no history.
This assumption isn't necessarily true - remember that we get both of these
asynchronously.  So we get the historic content length at some time from one
thread and we get the session content length at some other time from some other
thread.  It's theoretically possible that the historic information is from a
point in time before the current session information.

[megjablon, 2015-03-06 22:47:59]

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
File chrome/test/data/webui/net_internals/bandwidth_view.js (right):

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
chrome/test/data/webui/net_internals/bandwidth_view.js:148: // the session
information, because previously there was no history.
On 2015/03/06 22:20:23, mmenke wrote:
> This assumption isn't necessarily true - remember that we get both of these
> asynchronously.  So we get the historic content length at some time from one
> thread and we get the session content length at some other time from some
other
> thread.  It's theoretically possible that the historic information is from a
> point in time before the current session information.

I simply reverted this test back to its state before I wrote
DataReductionProxyStatisticsPrefs. We had to stop testing the historic network
stats since they wouldn't update for an hour. I think this is saying that since
we have no previous history, we know that the expected length is the same as
what we have seen in this session. It doesn't test that at this time they are
the same value. I can change the wording to something like

"The expected length should only be what has been collected in this session,
because previously there was no history."

[mmenke, 2015-03-09 16:00:21]

net_internals LGTM, with the suggested comment change.

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
File chrome/test/data/webui/net_internals/bandwidth_view.js (right):

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
chrome/test/data/webui/net_internals/bandwidth_view.js:148: // the session
information, because previously there was no history.
On 2015/03/06 22:47:59, megjablon wrote:
> On 2015/03/06 22:20:23, mmenke wrote:
> > This assumption isn't necessarily true - remember that we get both of these
> > asynchronously.  So we get the historic content length at some time from one
> > thread and we get the session content length at some other time from some
> other
> > thread.  It's theoretically possible that the historic information is from a
> > point in time before the current session information.
> 
> I simply reverted this test back to its state before I wrote
> DataReductionProxyStatisticsPrefs. We had to stop testing the historic network
> stats since they wouldn't update for an hour. I think this is saying that
since
> we have no previous history, we know that the expected length is the same as
> what we have seen in this session. It doesn't test that at this time they are
> the same value. I can change the wording to something like
> 
> "The expected length should only be what has been collected in this session,
> because previously there was no history."

That new text works for me.

[bengr, 2015-03-09 16:19:46]

data_reduction_proxy lgtm, with nit.

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.cc
(right):

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.cc:146:
base::Value*
#include base/Value if you forward declare in the header.

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h
(right):

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h:71:
base::Value* HistoricNetworkStatsInfoToValue();
Can base::Value be forward declared instead?

[megjablon, 2015-03-09 20:15:40]

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
File chrome/test/data/webui/net_internals/bandwidth_view.js (right):

https://codereview.chromium.org/989663002/diff/1/chrome/test/data/webui/net_i...
chrome/test/data/webui/net_internals/bandwidth_view.js:148: // the session
information, because previously there was no history.
On 2015/03/09 16:00:21, mmenke wrote:
> On 2015/03/06 22:47:59, megjablon wrote:
> > On 2015/03/06 22:20:23, mmenke wrote:
> > > This assumption isn't necessarily true - remember that we get both of
these
> > > asynchronously.  So we get the historic content length at some time from
one
> > > thread and we get the session content length at some other time from some
> > other
> > > thread.  It's theoretically possible that the historic information is from
a
> > > point in time before the current session information.
> > 
> > I simply reverted this test back to its state before I wrote
> > DataReductionProxyStatisticsPrefs. We had to stop testing the historic
network
> > stats since they wouldn't update for an hour. I think this is saying that
> since
> > we have no previous history, we know that the expected length is the same as
> > what we have seen in this session. It doesn't test that at this time they
are
> > the same value. I can change the wording to something like
> > 
> > "The expected length should only be what has been collected in this session,
> > because previously there was no history."
> 
> That new text works for me.

Done.

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.cc
(right):

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.cc:146:
base::Value*
On 2015/03/09 16:19:46, bengr wrote:
> #include base/Value if you forward declare in the header.

Already included.

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h
(right):

https://codereview.chromium.org/989663002/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h:71:
base::Value* HistoricNetworkStatsInfoToValue();
On 2015/03/09 16:19:46, bengr wrote:
> Can base::Value be forward declared instead?

Done.

[megjablon, 2015-03-09 22:13:34]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2015-03-09 22:13:34]

The patchset sent to the CQ was uploaded after l-g-t-m from bengr@chromium.org,
mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/989663002/#ps20001 (title:
"Addressing comments")

[commit-bot: I haz the power, 2015-03-09 22:14:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/989663002/20001

[commit-bot: I haz the power, 2015-03-09 22:21:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-09 22:22:35]

Patchset 2 (id:??) landed as
https://crrev.com/e1af51dc64c3bfd83d855dbdbf17d8bd4563bae5
Cr-Commit-Position: refs/heads/master@{#319749}