[Offline pages on the NTP] Add UMA for offline-available NTP tiles

[Offline pages on the NTP] Add UMA for offline-available NTP tiles

BUG=565219
Committed: https://crrev.com/cc4e35df97aa6cb16ae13b10cf1cd1ab29aa690c
Cr-Commit-Position: refs/heads/master@{#377033}

[Marc Treib, 2016-02-22 17:02:56]

treib@chromium.org changed reviewers:
+ asvitkine@chromium.org, newt@chromium.org

[Marc Treib, 2016-02-22 17:02:56]

newt: Please look at NTP code.

asvitkine: Please look at histograms.xml.

fgorski, jianli: FYI

https://codereview.chromium.org/1721753002/diff/1/chrome/android/java/src/org...
File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
(left):

https://codereview.chromium.org/1721753002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java:967:
String displayTitle, MostVisitedItem item,
unrelated: Removed some unnecessary params here.

[newt (away), 2016-02-22 18:02:03]

lgtm

[Alexei Svitkine (slow), 2016-02-22 20:47:27]

lgtm % comment

https://codereview.chromium.org/1721753002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1721753002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:31131: +<histogram
name="NewTabPage.TileOfflineAvailable">
Nit: Since this is more like an enum histogram - since you're logging the tab
index (and doing thinkg like percentiles / means / etc - doesn't make sense on
it) can you add an <enum> entry and specify it here via enum=""? This will let
our dashboards provide better views for it.

[Marc Treib, 2016-02-23 10:11:40]

https://codereview.chromium.org/1721753002/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1721753002/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:31131: +<histogram
name="NewTabPage.TileOfflineAvailable">
On 2016/02/22 20:47:27, Alexei Svitkine (slow) wrote:
> Nit: Since this is more like an enum histogram - since you're logging the tab
> index (and doing thinkg like percentiles / means / etc - doesn't make sense on
> it) can you add an <enum> entry and specify it here via enum=""? This will let
> our dashboards provide better views for it.

With enum labels like "Tile 1", "Tile 2", or probably just "0", "1"?
I guess it makes sense to get rid of the non-helpful means etc. But then we
should do it for the other similar histograms (e.g.
NewTagPage.SuggestionsImpression) as well, to be consistent. I'd like to do all
of them in a followup CL, if that's okay with you?

[Alexei Svitkine (slow), 2016-02-23 16:36:32]

lgtm, I'm fine with the enum being added in a follow-up CL and associated with
all of these histograms.

[Marc Treib, 2016-02-23 16:58:25]

The CQ bit was checked by treib@chromium.org

[commit-bot: I haz the power, 2016-02-23 16:59:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1721753002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1721753002/1

[commit-bot: I haz the power, 2016-02-23 18:56:39]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-02-23 18:57:25]

Description was changed from

==========
[Offline pages on the NTP] Add UMA for offline-available NTP tiles

BUG=565219
==========

to

==========
[Offline pages on the NTP] Add UMA for offline-available NTP tiles

BUG=565219

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

[commit-bot: I haz the power, 2016-02-23 18:57:26]

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