Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(189)

Issue 2886723002: [Zine] Add a metric to record number of visible sections on NTP. (Closed)

Created:
3 years, 7 months ago by vitaliii
Modified:
3 years, 7 months ago
Reviewers:
jkrcal, Ilya Sherman, dgn
CC:
chromium-reviews, noyau+watch_chromium.org, asvitkine+watch_chromium.org, ntp-dev+reviews_chromium.org, agrieve+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Zine] Add a metric to record number of visible sections on NTP. Currently we record number of suggestions per section on NTP opened (CountOnNTPOpened). However, recently we needed to check how many times the user sees only one section. This CL adds a new metric SectionCountOnNTPOpened to record number of visible sections on each NTP. BUG=721276 Review-Url: https://codereview.chromium.org/2886723002 Cr-Commit-Position: refs/heads/master@{#473853} Committed: https://chromium.googlesource.com/chromium/src/+/f22c112e0e74aa001c68b93b95824a8a5b554921

Patch Set 1 : #

Total comments: 4

Patch Set 2 : dgn@ comment. #

Patch Set 3 : fix DummySuggestionsEventReporter. #

Patch Set 4 : clean rebase. #

Total comments: 1

Patch Set 5 : isherman@ nit. #

Messages

Total messages: 45 (29 generated)
vitaliii
Hi dgn@, Could you have a look at java part of the CL?
3 years, 7 months ago (2017-05-16 11:09:38 UTC) #3
vitaliii
ping.
3 years, 7 months ago (2017-05-18 06:02:32 UTC) #4
dgn
https://codereview.chromium.org/2886723002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java File chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java (right): https://codereview.chromium.org/2886723002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java#newcode76 chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java:76: visibleCategoriesCount += 1; nit: increment style consistency. Either change ...
3 years, 7 months ago (2017-05-18 22:57:06 UTC) #5
vitaliii
I've addressed your comment, please have a look. https://codereview.chromium.org/2886723002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java File chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java (right): https://codereview.chromium.org/2886723002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java#newcode76 chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java:76: visibleCategoriesCount ...
3 years, 7 months ago (2017-05-19 08:38:12 UTC) #8
dgn
lgtm
3 years, 7 months ago (2017-05-19 10:25:06 UTC) #15
vitaliii
Hi jkrcal@, Could you have a look at C++ bit?
3 years, 7 months ago (2017-05-19 10:50:14 UTC) #17
vitaliii
ping.
3 years, 7 months ago (2017-05-22 09:53:08 UTC) #20
jkrcal
lgtm Sorry about the delay, it slipped through my email filters.
3 years, 7 months ago (2017-05-22 10:03:55 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2886723002/60001
3 years, 7 months ago (2017-05-22 11:47:19 UTC) #23
commit-bot: I haz the power
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_presubmit/builds/443677)
3 years, 7 months ago (2017-05-22 11:55:53 UTC) #25
vitaliii
Hi isherman@, Could you have a look at histograms.xml? I'm just adding a new histogram.
3 years, 7 months ago (2017-05-22 12:26:33 UTC) #29
Ilya Sherman
Metrics LGTM % a nit: https://codereview.chromium.org/2886723002/diff/80001/tools/metrics/histograms/histograms.xml File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2886723002/diff/80001/tools/metrics/histograms/histograms.xml#newcode42929 tools/metrics/histograms/histograms.xml:42929: +<histogram name="NewTabPage.ContentSuggestions.SectionCountOnNtpOpened"> nit: Please ...
3 years, 7 months ago (2017-05-22 20:40:08 UTC) #32
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2886723002/100001
3 years, 7 months ago (2017-05-23 09:03:23 UTC) #35
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/273836) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, ...
3 years, 7 months ago (2017-05-23 09:10:04 UTC) #37
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2886723002/100001
3 years, 7 months ago (2017-05-23 09:56:31 UTC) #42
commit-bot: I haz the power
3 years, 7 months ago (2017-05-23 10:49:30 UTC) #45
Message was sent while issue was closed.
Committed patchset #5 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/f22c112e0e74aa001c68b93b9582...

Powered by Google App Engine
This is Rietveld 408576698