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

Issue 2238163002: [MD History] Add UMA stats for switching views and the CBD button. (Closed)

Created:
4 years, 4 months ago by calamity
Modified:
4 years, 4 months ago
Reviewers:
tsergeant, Ilya Sherman
CC:
chromium-reviews, Patrick Dubroy, michaelpg+watch-md-ui_chromium.org, dbeam+watch-history_chromium.org, pam+watch_chromium.org, arv+watch_chromium.org, asvitkine+watch_chromium.org, chrome-apps-syd-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@start_focus_in_search_bar
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[MD History] Add UMA stats for switching views and the CBD button. This CL adds a UMA stat to the MD History page for tracking when the clear browsing data button is clicked and adds 'History.HistoryView' which is logs which view of the MD history page is being shown to users. BUG=627888 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/881d0a47fc1d9dc71ffa075b2856f8df209408cb Cr-Commit-Position: refs/heads/master@{#413693}

Patch Set 1 #

Patch Set 2 : #

Total comments: 3

Patch Set 3 : add test, add grouped history #

Total comments: 16

Patch Set 4 : address comments #

Patch Set 5 : run metrics test _after_ animation frame #

Patch Set 6 : rebase, vulcanize #

Total comments: 10

Patch Set 7 : address comments #

Total comments: 4

Patch Set 8 : rebase #

Patch Set 9 : replaceApp() #

Patch Set 10 : address comments #

Total comments: 4

Patch Set 11 : address_comments #

Patch Set 12 : fix_test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+255 lines, -85 lines) Patch
M chrome/browser/resources/md_history/app.html View 1 2 3 4 5 6 7 1 chunk +3 lines, -2 lines 0 comments Download
M chrome/browser/resources/md_history/app.js View 1 2 3 4 5 6 7 8 9 10 11 chunks +45 lines, -19 lines 0 comments Download
M chrome/browser/resources/md_history/app.crisper.js View 1 2 3 4 5 6 7 8 9 10 13 chunks +52 lines, -15 lines 0 comments Download
M chrome/browser/resources/md_history/app.vulcanized.html View 1 2 3 4 5 6 7 8 9 10 4 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/resources/md_history/browser_service.js View 1 2 3 4 5 1 chunk +9 lines, -0 lines 0 comments Download
M chrome/browser/resources/md_history/constants.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +15 lines, -0 lines 0 comments Download
M chrome/browser/resources/md_history/list_container.html View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/md_history/list_container.js View 1 2 3 4 5 6 7 4 chunks +10 lines, -7 lines 0 comments Download
M chrome/browser/resources/md_history/side_bar.html View 1 2 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/md_history/side_bar.js View 1 2 3 3 chunks +9 lines, -15 lines 0 comments Download
M chrome/browser/resources/md_history/synced_device_manager.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/resources/md_history/synced_device_manager.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +5 lines, -11 lines 0 comments Download
A chrome/test/data/webui/md_history/history_metrics_test.js View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +69 lines, -0 lines 0 comments Download
M chrome/test/data/webui/md_history/history_synced_tabs_test.js View 1 2 3 4 5 6 7 8 9 10 5 chunks +6 lines, -6 lines 0 comments Download
M chrome/test/data/webui/md_history/md_history_browsertest.js View 1 2 3 4 5 6 2 chunks +6 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +16 lines, -0 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 53 (32 generated)
calamity
4 years, 4 months ago (2016-08-12 06:03:29 UTC) #3
tsergeant
Looks like something went wrong in your last upload (in side_bar.js) https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resources/md_history/app.html File chrome/browser/resources/md_history/app.html (right): ...
4 years, 4 months ago (2016-08-15 02:48:38 UTC) #8
calamity
Fixed. https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resources/md_history/app.html File chrome/browser/resources/md_history/app.html (right): https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resources/md_history/app.html#newcode97 chrome/browser/resources/md_history/app.html:97: sign-in-state={{isUserSignedIn}} On 2016/08/15 02:48:38, tsergeant wrote: > This ...
4 years, 4 months ago (2016-08-15 05:59:18 UTC) #10
tsergeant
https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resources/md_history/app.html File chrome/browser/resources/md_history/app.html (right): https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resources/md_history/app.html#newcode97 chrome/browser/resources/md_history/app.html:97: sign-in-state="{{isUserSignedIn}}" I don't think this is ever used? The ...
4 years, 4 months ago (2016-08-15 07:05:18 UTC) #11
Dan Beam
https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resources/md_history/app.html File chrome/browser/resources/md_history/app.html (right): https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resources/md_history/app.html#newcode97 chrome/browser/resources/md_history/app.html:97: sign-in-state={{isUserSignedIn}} On 2016/08/15 05:59:18, calamity wrote: > On 2016/08/15 ...
4 years, 4 months ago (2016-08-15 18:07:05 UTC) #12
calamity
https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resources/md_history/app.html File chrome/browser/resources/md_history/app.html (right): https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resources/md_history/app.html#newcode97 chrome/browser/resources/md_history/app.html:97: sign-in-state="{{isUserSignedIn}}" On 2016/08/15 07:05:17, tsergeant wrote: > I don't ...
4 years, 4 months ago (2016-08-17 03:15:29 UTC) #21
tsergeant
https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resources/md_history/app.js File chrome/browser/resources/md_history/app.js (right): https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resources/md_history/app.js#newcode303 chrome/browser/resources/md_history/app.js:303: switch (this.$.content.selectedItem.id) { Why not just use this.selectedPage here? ...
4 years, 4 months ago (2016-08-17 06:03:10 UTC) #22
calamity
Added the sign in state back to app.js since we need it to figure out ...
4 years, 4 months ago (2016-08-18 03:11:16 UTC) #23
tsergeant
Looks like you forgot to upload the new patchset
4 years, 4 months ago (2016-08-18 03:46:42 UTC) #24
calamity
Oops, uploaded. >_>
4 years, 4 months ago (2016-08-18 08:00:26 UTC) #26
tsergeant
Yay, this is much simpler now. https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui/md_history/history_metrics_test.js File chrome/test/data/webui/md_history/history_metrics_test.js (right): https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui/md_history/history_metrics_test.js#newcode19 chrome/test/data/webui/md_history/history_metrics_test.js:19: var TestMetricsBrowserService = ...
4 years, 4 months ago (2016-08-19 00:46:23 UTC) #31
calamity
+isherman for histograms.xml https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui/md_history/history_metrics_test.js File chrome/test/data/webui/md_history/history_metrics_test.js (right): https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui/md_history/history_metrics_test.js#newcode19 chrome/test/data/webui/md_history/history_metrics_test.js:19: var TestMetricsBrowserService = function() { this.histogramMap ...
4 years, 4 months ago (2016-08-19 06:26:07 UTC) #34
Ilya Sherman
metrics lgtm % nits: https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resources/md_history/constants.js File chrome/browser/resources/md_history/constants.js (right): https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resources/md_history/constants.js#newcode25 chrome/browser/resources/md_history/constants.js:25: * Keep this in sync ...
4 years, 4 months ago (2016-08-19 23:13:40 UTC) #35
tsergeant
lgtm
4 years, 4 months ago (2016-08-21 23:36:28 UTC) #36
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/2238163002/260001
4 years, 4 months ago (2016-08-22 05:28:16 UTC) #39
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/242796)
4 years, 4 months ago (2016-08-22 05:35:38 UTC) #41
Dan Beam
welcome to the wonderful world of vulcanize! NOPRESUBMIT=true # crisper.js is what we've been doing ...
4 years, 4 months ago (2016-08-23 00:07:48 UTC) #42
calamity
https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resources/md_history/constants.js File chrome/browser/resources/md_history/constants.js (right): https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resources/md_history/constants.js#newcode25 chrome/browser/resources/md_history/constants.js:25: * Keep this in sync with the HistoryView enum ...
4 years, 4 months ago (2016-08-23 03:31:50 UTC) #43
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/2238163002/280001
4 years, 4 months ago (2016-08-23 08:21:45 UTC) #50
commit-bot: I haz the power
Committed patchset #12 (id:280001)
4 years, 4 months ago (2016-08-23 08:26:13 UTC) #51
commit-bot: I haz the power
4 years, 4 months ago (2016-08-23 08:27:22 UTC) #53
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/881d0a47fc1d9dc71ffa075b2856f8df209408cb
Cr-Commit-Position: refs/heads/master@{#413693}

Powered by Google App Engine
This is Rietveld 408576698