Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2696223004: Desktop NTP: Add a UMA metric NewTabPage.TilesReceivedTime (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months ago by Marc Treib
Modified:
6 months ago
Reviewers:
sfiera, Steven Holte
CC:
chromium-reviews, dbeam+watch-ntp_chromium.org, skanuj+watch_chromium.org, melevin+watch_chromium.org, ntp-dev+reviews_chromium.org, donnd+watch_chromium.org, noyau+watch_chromium.org, jfweitz+watch_chromium.org, David Black, arv+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered, pedrosimonetti+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Desktop NTP: Add a UMA metric NewTabPage.TilesReceivedTime This also adds a unit test for NTP load time histograms, and contains some cleanup in the single-iframe JS. BUG=514752 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2696223004 Cr-Commit-Position: refs/heads/master@{#451304} Committed: https://chromium.googlesource.com/chromium/src/+/848f7e4273c840fb95028b513e4902e6f6157a58

Patch Set 1 #

Patch Set 2 : histograms.xml #

Total comments: 5

Patch Set 3 : review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+202 lines, -66 lines) Patch
M chrome/browser/resources/local_ntp/most_visited_single.js View 8 chunks +37 lines, -14 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_user_data_logger.h View 2 chunks +4 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc View 1 4 chunks +49 lines, -26 lines 0 comments Download
M chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc View 1 2 2 chunks +92 lines, -0 lines 0 comments Download
M chrome/common/search/ntp_logging_events.h View 1 chunk +9 lines, -26 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 chunks +11 lines, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 23 (13 generated)
Marc Treib
PTAL!
6 months ago (2017-02-16 13:07:05 UTC) #5
Marc Treib
https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/resources/local_ntp/most_visited_single.js File chrome/browser/resources/local_ntp/most_visited_single.js (right): https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/resources/local_ntp/most_visited_single.js#newcode174 chrome/browser/resources/local_ntp/most_visited_single.js:174: var showTiles = function(info) { The old "showTiles" was ...
6 months ago (2017-02-16 13:09:29 UTC) #6
sfiera
LGTM https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc File chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc (right): https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc#newcode231 chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc:231: // at this point everything should still be ...
6 months ago (2017-02-16 14:35:33 UTC) #7
Marc Treib
Thanks! https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc File chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc (right): https://codereview.chromium.org/2696223004/diff/20001/chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc#newcode231 chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc:231: // at this point everything should still be ...
6 months ago (2017-02-16 15:13:09 UTC) #10
Marc Treib
+holte for histograms.xml, PTAL!
6 months ago (2017-02-16 15:13:30 UTC) #12
Steven Holte
lgtm
6 months ago (2017-02-16 21:32:33 UTC) #13
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/2696223004/40001
6 months ago (2017-02-17 09:09:04 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/367039)
6 months ago (2017-02-17 10:34:02 UTC) #18
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/2696223004/40001
6 months ago (2017-02-17 13:50:21 UTC) #20
commit-bot: I haz the power
6 months ago (2017-02-17 14:38:34 UTC) #23
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/848f7e4273c840fb95028b513e49...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b