Fix incorrectly reported UMA OfflinePages.AggregatedRequestResult

Fix incorrectly reported UMA OfflinePages.AggregatedRequestResult

BUG=659372
Committed: https://crrev.com/c6b6dbfd5c30edb7187b29292444e2775db2d8e3
Cr-Commit-Position: refs/heads/master@{#427909}

[jianli, 2016-10-25 23:40:41]

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

[commit-bot: I haz the power, 2016-10-25 23:41:12]

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

[jianli, 2016-10-25 23:42:47]

jianli@chromium.org changed reviewers:
+ dimich@chromium.org, holte@chromium.org

[jianli, 2016-10-25 23:42:47]

dimich for everything
holte for UMA

[jianli, 2016-10-25 23:53:52]

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

[commit-bot: I haz the power, 2016-10-25 23:53:54]

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

[Dmitry Titov, 2016-10-26 00:05:28]

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92620: +    Deprecated 2016-10.
this one is not yet deprecated.

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92629: +  <int value="7" label="Show
offline page on disconnected network"/>
I think this should read "connected".

[jianli, 2016-10-26 00:07:50]

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92620: +    Deprecated 2016-10.
On 2016/10/26 00:05:27, Dmitry Titov wrote:
> this one is not yet deprecated.

Done.

https://codereview.chromium.org/2452593003/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92629: +  <int value="7" label="Show
offline page on disconnected network"/>
On 2016/10/26 00:05:28, Dmitry Titov wrote:
> I think this should read "connected".

Done.

[jianli, 2016-10-26 00:07:55]

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

[commit-bot: I haz the power, 2016-10-26 00:08:07]

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

[Dmitry Titov, 2016-10-26 00:09:15]

lgtm

[commit-bot: I haz the power, 2016-10-26 01:51:19]

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

[commit-bot: I haz the power, 2016-10-26 01:51:20]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Steven Holte, 2016-10-26 21:41:04]

https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92618: +<enum
name="OfflinePagesAggregatedRequestResult2" type="int">
This definition looks exactly the same as the old one.  Can you just keep using
the same enum?  It looks like you were just avoiding the reports in the success
case?

[jianli, 2016-10-26 22:22:01]

https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:92618: +<enum
name="OfflinePagesAggregatedRequestResult2" type="int">
On 2016/10/26 21:41:04, Steven Holte wrote:
> This definition looks exactly the same as the old one.  Can you just keep
using
> the same enum?  It looks like you were just avoiding the reports in the
success
> case?

The label for value 7 and 8 have been updated due to typo. Can we still use the
same enum?

[Steven Holte, 2016-10-26 22:23:59]

On 2016/10/26 22:22:01, jianli wrote:
>
https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
> File tools/metrics/histograms/histograms.xml (right):
> 
>
https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
> tools/metrics/histograms/histograms.xml:92618: +<enum
> name="OfflinePagesAggregatedRequestResult2" type="int">
> On 2016/10/26 21:41:04, Steven Holte wrote:
> > This definition looks exactly the same as the old one.  Can you just keep
> using
> > the same enum?  It looks like you were just avoiding the reports in the
> success
> > case?
> 
> The label for value 7 and 8 have been updated due to typo. Can we still use
the
> same enum?

Yes, enum labels can just be updated.

[jianli, 2016-10-26 22:35:31]

On 2016/10/26 22:23:59, Steven Holte wrote:
> On 2016/10/26 22:22:01, jianli wrote:
> >
>
https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
> > File tools/metrics/histograms/histograms.xml (right):
> > 
> >
>
https://codereview.chromium.org/2452593003/diff/40001/tools/metrics/histogram...
> > tools/metrics/histograms/histograms.xml:92618: +<enum
> > name="OfflinePagesAggregatedRequestResult2" type="int">
> > On 2016/10/26 21:41:04, Steven Holte wrote:
> > > This definition looks exactly the same as the old one.  Can you just keep
> > using
> > > the same enum?  It looks like you were just avoiding the reports in the
> > success
> > > case?
> > 
> > The label for value 7 and 8 have been updated due to typo. Can we still use
> the
> > same enum?
> 
> Yes, enum labels can just be updated.

Done.

[jianli, 2016-10-26 22:35:42]

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

[commit-bot: I haz the power, 2016-10-26 22:36:19]

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

[Steven Holte, 2016-10-26 22:55:18]

lgtm

[commit-bot: I haz the power, 2016-10-27 00:09:02]

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

[commit-bot: I haz the power, 2016-10-27 00:09:03]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jianli, 2016-10-27 00:24:55]

The CQ bit was checked by jianli@chromium.org

[jianli, 2016-10-27 00:24:56]

The patchset sent to the CQ was uploaded after l-g-t-m from dimich@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2452593003/#ps80001
(title: "Fix tests")

[commit-bot: I haz the power, 2016-10-27 00:25:37]

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, 2016-10-27 01:47:04]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-27 01:48:36]

Description was changed from

==========
Fix incorrectly reported UMA OfflinePages.AggregatedRequestResult

BUG=659372
==========

to

==========
Fix incorrectly reported UMA OfflinePages.AggregatedRequestResult

BUG=659372

Committed: https://crrev.com/c6b6dbfd5c30edb7187b29292444e2775db2d8e3
Cr-Commit-Position: refs/heads/master@{#427909}
==========

[commit-bot: I haz the power, 2016-10-27 01:48:37]

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