UMA for previews infobar and Lo-Fi context menu items

UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
Committed: https://crrev.com/eadad0168500ebde0d0eec52b8fe640fa004973f
Cr-Commit-Position: refs/heads/master@{#418599}

[megjablon, 2016-08-30 18:33:48]

Description was changed from

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the old DataReductionProxy.UIAction.LoFi histogram.
Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

to

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the old DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

[megjablon, 2016-09-07 22:49:50]

Patchset #1 (id:1) has been deleted

[megjablon, 2016-09-07 22:49:53]

Patchset #1 (id:20001) has been deleted

[megjablon, 2016-09-07 22:50:09]

Description was changed from

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the old DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

to

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

[megjablon, 2016-09-07 22:50:13]

megjablon@chromium.org changed reviewers:
+ asvitkine@chromium.org, bengr@chromium.org, dfalcantara@chromium.org

[megjablon, 2016-09-07 22:50:13]

dfalcantara: chrome/android/*
bengr: chrome/browser/*
asvitkine: histograms.xml

[gone, 2016-09-07 23:54:03]

lgtm

https://chromiumcodereview.appspot.com/2258283002/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionProxyUma.java
(right):

https://chromiumcodereview.appspot.com/2258283002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/preferences/datareduction/DataReductionProxyUma.java:37:
public static final int LOFI_CONTEXT_MENU_ACTION_INDEX_BOUNDARY = 5;
Shouldn't this thing be named just like the other ones?

[bengr, 2016-09-08 00:53:36]

Apparently test coverage is coming in the next CL, so lgtm.

https://codereview.chromium.org/2258283002/diff/60001/chrome/browser/previews...
File chrome/browser/previews/previews_infobar_delegate.cc (right):

https://codereview.chromium.org/2258283002/diff/60001/chrome/browser/previews...
chrome/browser/previews/previews_infobar_delegate.cc:42: void
RecordPreviewsInfoBarAction(
Probably worth writing a test that the right histogram is written with the right
information.

[megjablon, 2016-09-08 01:01:45]

On 2016/09/08 00:53:36, bengr wrote:
> Apparently test coverage is coming in the next CL, so lgtm.
> 
>
https://codereview.chromium.org/2258283002/diff/60001/chrome/browser/previews...
> File chrome/browser/previews/previews_infobar_delegate.cc (right):
> 
>
https://codereview.chromium.org/2258283002/diff/60001/chrome/browser/previews...
> chrome/browser/previews/previews_infobar_delegate.cc:42: void
> RecordPreviewsInfoBarAction(
> Probably worth writing a test that the right histogram is written with the
right
> information.

Yep. It's in the test cl. I'll get the lgtm on that before landing any of this.

[Alexei Svitkine (slow), 2016-09-08 17:13:42]

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

https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:46000: +<histogram
name="Previews.ContextMenuAction.LoFi"
This seems to be adding a new top level prefix - "Previews.". In general, we try
to suggest to re-use an existing prefix unless we expect the new category to be
extensively used. I'm not sure which case this is, however I find the name not
super obvious to me - "Previews" doesn't tell me very much. Regardless if it
will be top level or nested under something, I encourage using a name that's a
bit clearer what it's about - to developers who do not work directly on the
feature. Do you have suggestions for such a name?

And for my own benefit, what is the "previews" feature?

[megjablon, 2016-09-08 18:26:13]

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

https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:46000: +<histogram
name="Previews.ContextMenuAction.LoFi"
On 2016/09/08 17:13:41, Alexei Svitkine (very slow) wrote:
> This seems to be adding a new top level prefix - "Previews.". In general, we
try
> to suggest to re-use an existing prefix unless we expect the new category to
be
> extensively used. I'm not sure which case this is, however I find the name not
> super obvious to me - "Previews" doesn't tell me very much. Regardless if it
> will be top level or nested under something, I encourage using a name that's a
> bit clearer what it's about - to developers who do not work directly on the
> feature. Do you have suggestions for such a name?
> 
> And for my own benefit, what is the "previews" feature?

Previews is the name for all features that rewrite the page to improve data
efficiency and load latency. It includes features that use the data reduction
proxy, but also client-side rewrites that aren't appropriate to nest under the
proxy's name. Here's the design doc that summarizes all these efforts:
https://docs.google.com/a/google.com/document/d/1MYTA8pjWFNvICQpcXzAr22ggJjgQ....

[RyanSturm, 2016-09-08 20:27:14]

On 2016/09/08 18:26:13, megjablon wrote:
>
https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
> File tools/metrics/histograms/histograms.xml (right):
> 
>
https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
> tools/metrics/histograms/histograms.xml:46000: +<histogram
> name="Previews.ContextMenuAction.LoFi"
> On 2016/09/08 17:13:41, Alexei Svitkine (very slow) wrote:
> > This seems to be adding a new top level prefix - "Previews.". In general, we
> try
> > to suggest to re-use an existing prefix unless we expect the new category to
> be
> > extensively used. I'm not sure which case this is, however I find the name
not
> > super obvious to me - "Previews" doesn't tell me very much. Regardless if it
> > will be top level or nested under something, I encourage using a name that's
a
> > bit clearer what it's about - to developers who do not work directly on the
> > feature. Do you have suggestions for such a name?
> > 
> > And for my own benefit, what is the "previews" feature?
> 
> Previews is the name for all features that rewrite the page to improve data
> efficiency and load latency. It includes features that use the data reduction
> proxy, but also client-side rewrites that aren't appropriate to nest under the
> proxy's name. Here's the design doc that summarizes all these efforts:
>
https://docs.google.com/a/google.com/document/d/1MYTA8pjWFNvICQpcXzAr22ggJjgQ....

Additionally, this CL is adding Previews UMA as well:
https://codereview.chromium.org/2256603005/

[Alexei Svitkine (slow), 2016-09-08 20:32:07]

LGTM % optional comment

Sorry for the slow reviews, as I had about 20 CLs to review waiting for me this
morning and a day full of meetings.

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

https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:46000: +<histogram
name="Previews.ContextMenuAction.LoFi"
On 2016/09/08 18:26:13, megjablon wrote:
> On 2016/09/08 17:13:41, Alexei Svitkine (very slow) wrote:
> > This seems to be adding a new top level prefix - "Previews.". In general, we
> try
> > to suggest to re-use an existing prefix unless we expect the new category to
> be
> > extensively used. I'm not sure which case this is, however I find the name
not
> > super obvious to me - "Previews" doesn't tell me very much. Regardless if it
> > will be top level or nested under something, I encourage using a name that's
a
> > bit clearer what it's about - to developers who do not work directly on the
> > feature. Do you have suggestions for such a name?
> > 
> > And for my own benefit, what is the "previews" feature?
> 
> Previews is the name for all features that rewrite the page to improve data
> efficiency and load latency. It includes features that use the data reduction
> proxy, but also client-side rewrites that aren't appropriate to nest under the
> proxy's name. Here's the design doc that summarizes all these efforts:
>
https://docs.google.com/a/google.com/document/d/1MYTA8pjWFNvICQpcXzAr22ggJjgQ....

I see, so it's a project codename? I'm still not a fan of the prefix since I
would imagine very few people outside your team would understand it. Would a
more explanatory name prefix like "PageRewriter." be acceptable?

[megjablon, 2016-09-08 23:24:47]

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

https://codereview.chromium.org/2258283002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:46000: +<histogram
name="Previews.ContextMenuAction.LoFi"
On 2016/09/08 20:32:07, Alexei Svitkine (very slow) wrote:
> On 2016/09/08 18:26:13, megjablon wrote:
> > On 2016/09/08 17:13:41, Alexei Svitkine (very slow) wrote:
> > > This seems to be adding a new top level prefix - "Previews.". In general,
we
> > try
> > > to suggest to re-use an existing prefix unless we expect the new category
to
> > be
> > > extensively used. I'm not sure which case this is, however I find the name
> not
> > > super obvious to me - "Previews" doesn't tell me very much. Regardless if
it
> > > will be top level or nested under something, I encourage using a name
that's
> a
> > > bit clearer what it's about - to developers who do not work directly on
the
> > > feature. Do you have suggestions for such a name?
> > > 
> > > And for my own benefit, what is the "previews" feature?
> > 
> > Previews is the name for all features that rewrite the page to improve data
> > efficiency and load latency. It includes features that use the data
reduction
> > proxy, but also client-side rewrites that aren't appropriate to nest under
the
> > proxy's name. Here's the design doc that summarizes all these efforts:
> >
>
https://docs.google.com/a/google.com/document/d/1MYTA8pjWFNvICQpcXzAr22ggJjgQ....
> 
> I see, so it's a project codename? I'm still not a fan of the prefix since I
> would imagine very few people outside your team would understand it. Would a
> more explanatory name prefix like "PageRewriter." be acceptable?

We've already started using this name throughout the code. We have a component
named this as well. I'll defer to bengr@ on the decision of if we want to do a
rename.

[megjablon, 2016-09-13 22:05:22]

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

[commit-bot: I haz the power, 2016-09-13 22:05:58]

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

[megjablon, 2016-09-13 22:09:26]

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

[commit-bot: I haz the power, 2016-09-13 22:09:53]

Dry run: 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-09-13 23:24:53]

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

[commit-bot: I haz the power, 2016-09-13 23:24:54]

Dry run: This issue passed the CQ dry run.

[megjablon, 2016-09-13 23:32:12]

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

[commit-bot: I haz the power, 2016-09-13 23:33:16]

Dry run: 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-09-13 23:40:55]

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

[commit-bot: I haz the power, 2016-09-13 23:41:02]

Dry run: 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...)

[megjablon, 2016-09-14 17:13:22]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2016-09-14 17:13:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, bengr@chromium.org, asvitkine@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/2258283002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-14 17:13: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-09-14 17:20:24]

Description was changed from

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

to

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

[commit-bot: I haz the power, 2016-09-14 17:20:25]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-09-14 17:22:05]

Description was changed from

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566
==========

to

==========
UMA for previews infobar and Lo-Fi context menu items

Deprecating the DataReductionProxy.UIAction.LoFi histogram.

Replacing with Previews.InfoBarAction.LoFi,
Previews.InfoBarAction.Offline,
Previews.InfoBarAction.WebLite, and
Previews.ContextMenuAction.

Previews.InfoBarAction.LoFi, Previews.InfoBarAction.Offline,
and Previews.InfoBarAction.WebLite buckets:
  “Infobar shown”
  "Infobar 'Load original' clicked”
  "Infobar dismissed by user"
  "Infobar dismissed by navigation"

Previews.ContextMenuAction.LoFi buckets:
  "'Load image' context menu item shown"
  "'Load image' context menu item clicked"
  "Pages where the user has clicked 'Load image' at least once"
  "'Load images' context menu item shown"
  "'Load images' context menu item clicked"

BUG=615566

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

[commit-bot: I haz the power, 2016-09-14 17:22:06]

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