[MD History] Add UMA stats for switching views and the CBD button.

[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}

[calamity, 2016-08-12 05:58:04]

Description was changed from

==========
[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
==========

to

==========
[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
==========

[calamity, 2016-08-12 06:03:29]

calamity@chromium.org changed reviewers:
+ tsergeant@chromium.org

[calamity, 2016-08-12 06:03:50]

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

[commit-bot: I haz the power, 2016-08-12 06:04:00]

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-08-12 06:06:08]

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

[commit-bot: I haz the power, 2016-08-12 06:06:09]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[tsergeant, 2016-08-15 02:48:38]

Looks like something went wrong in your last upload (in side_bar.js)

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.html:97:
sign-in-state={{isUserSignedIn}}
This and the above should have "quotes"

[calamity, 2016-08-15 05:56:31]

Patchset #3 (id:40001) has been deleted

[calamity, 2016-08-15 05:59:18]

Fixed.

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
chrome/browser/resources/md_history/app.html:97:
sign-in-state={{isUserSignedIn}}
On 2016/08/15 02:48:38, tsergeant wrote:
> This and the above should have "quotes"

#howdidthiseverwork

[tsergeant, 2016-08-15 07:05:18]

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.html:97:
sign-in-state="{{isUserSignedIn}}"
I don't think this is ever used? The value is always accessed as
`selectedPage.signInState` rather than `this.isUserSignedIn`

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:314: }
Nit: deindent to line up with switch

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/browser_service.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/browser_service.js:87:
chrome.send('metricsHandler:recordAction', args);
Replace args with a single argument for the action name and package it into an
array in the chrome.send call.

(We know exactly what the recordAction call expects, so there's no reason not to
expand it out and get proper type checking).

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/browser_service.js:94:
chrome.send('metricsHandler:recordInHistogram', args);
As above, use specific arguments here rather than an array.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/side_bar.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/side_bar.js:10: {type: String, notify:
true},
No need for a newline here

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/side_bar.js:24: onSelectorActivate_:
function(e) {
No need to add the e parameter here.

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/history_metrics_test.js:19: return
waitForAnimationFrame();
waitForAnimationFrame is very specific. Does flush work? Do all of these need
it?

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/md_history_browsertest.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/md_history_browsertest.js:44:
registerHistogramCallback_: function() {
I'm not a big fan of this:

Firstly, can it be local to the test, rather than applying to every test and
attaching results to window?

Secondly, since all you care about is the arguments passed to BrowserService, it
may be more appropriate to fake out the BrowserService instance and track the
calls in there.

[Dan Beam, 2016-08-15 18:07:05]

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2238163002/diff/20001/chrome/browser/resource...
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 02:48:38, tsergeant wrote:
> > This and the above should have "quotes"
> 
> #howdidthiseverwork

https://www.w3.org/TR/2011/WD-html5-20110113/tokenization.html#attribute-valu...

[calamity, 2016-08-16 04:38:33]

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

[commit-bot: I haz the power, 2016-08-16 04:38:40]

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-08-16 05:46:06]

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

[commit-bot: I haz the power, 2016-08-16 05:46:06]

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

[calamity, 2016-08-16 07:51:19]

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

[commit-bot: I haz the power, 2016-08-16 07:51:33]

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-08-16 07:53:50]

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

[commit-bot: I haz the power, 2016-08-16 07:53:51]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[calamity, 2016-08-17 03:15:29]

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.html (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.html:97:
sign-in-state="{{isUserSignedIn}}"
On 2016/08/15 07:05:17, tsergeant wrote:
> I don't think this is ever used? The value is always accessed as
> `selectedPage.signInState` rather than `this.isUserSignedIn`

Wups. This is an artifact of something I was trying earlier. Removed.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/app.js:314: }
On 2016/08/15 07:05:17, tsergeant wrote:
> Nit: deindent to line up with switch

Done.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/browser_service.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/browser_service.js:87:
chrome.send('metricsHandler:recordAction', args);
On 2016/08/15 07:05:17, tsergeant wrote:
> Replace args with a single argument for the action name and package it into an
> array in the chrome.send call.
> 
> (We know exactly what the recordAction call expects, so there's no reason not
to
> expand it out and get proper type checking).

Done.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/browser_service.js:94:
chrome.send('metricsHandler:recordInHistogram', args);
On 2016/08/15 07:05:17, tsergeant wrote:
> As above, use specific arguments here rather than an array.

Done.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/md_history/side_bar.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/side_bar.js:10: {type: String, notify:
true},
On 2016/08/15 07:05:17, tsergeant wrote:
> No need for a newline here

Done.

https://codereview.chromium.org/2238163002/diff/60001/chrome/browser/resource...
chrome/browser/resources/md_history/side_bar.js:24: onSelectorActivate_:
function(e) {
On 2016/08/15 07:05:18, tsergeant wrote:
> No need to add the e parameter here.

Done.

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/history_metrics_test.js:19: return
waitForAnimationFrame();
On 2016/08/15 07:05:18, tsergeant wrote:
> waitForAnimationFrame is very specific. Does flush work? Do all of these need
> it?

flush() doesn't seem to work. Since HistoryView is logged on the next animation
frame always, each of these does indeed need it =(

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_history/md_history_browsertest.js (right):

https://codereview.chromium.org/2238163002/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_history/md_history_browsertest.js:44:
registerHistogramCallback_: function() {
On 2016/08/15 07:05:18, tsergeant wrote:
> I'm not a big fan of this:
> 
> Firstly, can it be local to the test, rather than applying to every test and
> attaching results to window?
> 
> Secondly, since all you care about is the arguments passed to BrowserService,
it
> may be more appropriate to fake out the BrowserService instance and track the
> calls in there.
Changed this as discussed

[tsergeant, 2016-08-17 06:03:10]

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_history/app.js:303: switch
(this.$.content.selectedItem.id) {
Why not just use this.selectedPage here? Then there's no need for the asyncery
at all.

You can use a default case to handle the fact that 'history' can also be the
empty string.

As an aside, we probably should try to normalise the value in selectedPage_ so
it can't be the empty string.

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_history/synced_device_manager.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_history/synced_device_manager.js:234:
signInStateChanged_: function(signInState) {
Convention is for observer functions to leave out the `signInState` parameter
and just use `this.signinState`

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_metrics_test.js:19: var
TestMetricsBrowserService = function() { this.histogramMap = {}; };
Nit: Maybe move this declaration outside of the setup function.

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/md_history_browsertest.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/md_history_browsertest.js:83:
TEST_F('MaterialHistoryBrowserTest', 'HistoryMetricsTest', function() {
Nit: For new tests, I've been leaving out the History prefix

MaterialHistoryBrowserTest.MetricsTest

(we could probably go one step further and get rid of the Test suffix as well)

[calamity, 2016-08-18 03:11:16]

Added the sign in state back to app.js since we need it to figure out which
synced tab view will show.

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_history/app.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_history/app.js:303: switch
(this.$.content.selectedItem.id) {
On 2016/08/17 06:03:09, tsergeant wrote:
> Why not just use this.selectedPage here? Then there's no need for the asyncery
> at all.
> 
> You can use a default case to handle the fact that 'history' can also be the
> empty string.
> 
> As an aside, we probably should try to normalise the value in selectedPage_ so
> it can't be the empty string.

Done. Normalizing sounds... annoying and subtle. I'm not sure how that would
interact with the binding to the route.

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/md_history/synced_device_manager.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/md_history/synced_device_manager.js:234:
signInStateChanged_: function(signInState) {
On 2016/08/17 06:03:09, tsergeant wrote:
> Convention is for observer functions to leave out the `signInState` parameter
> and just use `this.signinState`

Done.

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_metrics_test.js:19: var
TestMetricsBrowserService = function() { this.histogramMap = {}; };
On 2016/08/17 06:03:09, tsergeant wrote:
> Nit: Maybe move this declaration outside of the setup function.

Done.

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/md_history_browsertest.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/md_history_browsertest.js:83:
TEST_F('MaterialHistoryBrowserTest', 'HistoryMetricsTest', function() {
On 2016/08/17 06:03:10, tsergeant wrote:
> Nit: For new tests, I've been leaving out the History prefix
> 
> MaterialHistoryBrowserTest.MetricsTest
> 
> (we could probably go one step further and get rid of the Test suffix as well)

Done.

[tsergeant, 2016-08-18 03:46:42]

Looks like you forgot to upload the new patchset

[calamity, 2016-08-18 07:57:46]

Patchset #7 (id:140001) has been deleted

[calamity, 2016-08-18 08:00:26]

Oops, uploaded. >_>

[calamity, 2016-08-18 08:00:35]

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

[commit-bot: I haz the power, 2016-08-18 08:00:50]

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-08-18 08:09:57]

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

[commit-bot: I haz the power, 2016-08-18 08:09:58]

Dry run: Try jobs failed on following builders:
  closure_compilation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[tsergeant, 2016-08-19 00:46:23]

Yay, this is much simpler now.

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_metrics_test.js:19: var
TestMetricsBrowserService = function() { this.histogramMap = {}; };
On 2016/08/18 03:11:15, calamity wrote:
> On 2016/08/17 06:03:09, tsergeant wrote:
> > Nit: Maybe move this declaration outside of the setup function.
> 
> Done.

Sorry, I was unclear: I meant move the whole TestMetricsBrowserService class
definition outside of setup. It's a little weird to have a class definition
inside a function that's meant to be called repeatedly.

I think keep the histogramMap inside the BrowserService class.

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/md_history/constants.js (right):

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/md_history/constants.js:24: * Histogram buckets for UMA
tracking of which view is being shown to the user.
Nit: Comment that this should be kept up to date with the enum in histograms.xml

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/md_history/synced_device_manager.js (right):

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/md_history/synced_device_manager.js:230: * @param
{boolean} signInState
Delete this to keep closure happy.

[calamity, 2016-08-19 06:23:44]

Patchset #9 (id:200001) has been deleted

[calamity, 2016-08-19 06:26:06]

calamity@chromium.org changed reviewers:
+ isherman@chromium.org

[calamity, 2016-08-19 06:26:07]

+isherman for histograms.xml

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
File chrome/test/data/webui/md_history/history_metrics_test.js (right):

https://codereview.chromium.org/2238163002/diff/120001/chrome/test/data/webui...
chrome/test/data/webui/md_history/history_metrics_test.js:19: var
TestMetricsBrowserService = function() { this.histogramMap = {}; };
On 2016/08/19 00:46:23, tsergeant wrote:
> On 2016/08/18 03:11:15, calamity wrote:
> > On 2016/08/17 06:03:09, tsergeant wrote:
> > > Nit: Maybe move this declaration outside of the setup function.
> > 
> > Done.
> 
> Sorry, I was unclear: I meant move the whole TestMetricsBrowserService class
> definition outside of setup. It's a little weird to have a class definition
> inside a function that's meant to be called repeatedly.
> 
> I think keep the histogramMap inside the BrowserService class.

Done.

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/md_history/constants.js (right):

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/md_history/constants.js:24: * Histogram buckets for UMA
tracking of which view is being shown to the user.
On 2016/08/19 00:46:23, tsergeant wrote:
> Nit: Comment that this should be kept up to date with the enum in
histograms.xml

Done.

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
File chrome/browser/resources/md_history/synced_device_manager.js (right):

https://codereview.chromium.org/2238163002/diff/160001/chrome/browser/resourc...
chrome/browser/resources/md_history/synced_device_manager.js:230: * @param
{boolean} signInState
On 2016/08/19 00:46:23, tsergeant wrote:
> Delete this to keep closure happy.

Done.

[Ilya Sherman, 2016-08-19 23:13:40]

metrics lgtm % nits:

https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resourc...
File chrome/browser/resources/md_history/constants.js (right):

https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resourc...
chrome/browser/resources/md_history/constants.js:25: * Keep this in sync with
the HistoryView enum in histograms.xml.
Please also document that this enum should be treated as append-only.

https://codereview.chromium.org/2238163002/diff/240001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2238163002/diff/240001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:18390: +<histogram
name="History.HistoryView" enum="HistoryView">
Optional nit: I think History.HistoryPageView or History.HistorySettingsView or
something like that would be a bit clearer than "HistoryView".  But, I don't
feel strongly about this -- so feel free to ignore if you disagree.

[tsergeant, 2016-08-21 23:36:28]

lgtm

[calamity, 2016-08-22 05:28:01]

The CQ bit was checked by calamity@chromium.org

[calamity, 2016-08-22 05:28:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2238163002/#ps260001
(title: "address_comments")

[commit-bot: I haz the power, 2016-08-22 05:28:16]

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-08-22 05:35:36]

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

[commit-bot: I haz the power, 2016-08-22 05:35:38]

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...)

[Dan Beam, 2016-08-23 00:07:48]

welcome to the wonderful world of vulcanize!

NOPRESUBMIT=true  # crisper.js

is what we've been doing so far

I thought there was a CL to change the global src/PRESUBMIT.py for this purpose?
 Maybe it hasn't landed yet?

[calamity, 2016-08-23 03:31:50]

https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resourc...
File chrome/browser/resources/md_history/constants.js (right):

https://codereview.chromium.org/2238163002/diff/240001/chrome/browser/resourc...
chrome/browser/resources/md_history/constants.js:25: * Keep this in sync with
the HistoryView enum in histograms.xml.
On 2016/08/19 23:13:40, Ilya Sherman wrote:
> Please also document that this enum should be treated as append-only.

Done.

https://codereview.chromium.org/2238163002/diff/240001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2238163002/diff/240001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:18390: +<histogram
name="History.HistoryView" enum="HistoryView">
On 2016/08/19 23:13:40, Ilya Sherman wrote:
> Optional nit: I think History.HistoryPageView or History.HistorySettingsView
or
> something like that would be a bit clearer than "HistoryView".  But, I don't
> feel strongly about this -- so feel free to ignore if you disagree.

Done.

[calamity, 2016-08-23 07:03:18]

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

[commit-bot: I haz the power, 2016-08-23 07:03:34]

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-08-23 08:13:10]

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

[commit-bot: I haz the power, 2016-08-23 08:13:11]

Dry run: This issue passed the CQ dry run.

[calamity, 2016-08-23 08:21:30]

The CQ bit was checked by calamity@chromium.org

[calamity, 2016-08-23 08:21:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
tsergeant@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2238163002/#ps280001
(title: "fix_test")

[commit-bot: I haz the power, 2016-08-23 08:21:45]

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-08-23 08:26:13]

Committed patchset #12 (id:280001)

[commit-bot: I haz the power, 2016-08-23 08:27:20]

Description was changed from

==========
[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
==========

to

==========
[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}
==========

[commit-bot: I haz the power, 2016-08-23 08:27:22]

Patchset 12 (id:??) landed as
https://crrev.com/881d0a47fc1d9dc71ffa075b2856f8df209408cb
Cr-Commit-Position: refs/heads/master@{#413693}