Kill NewTabPage.NumberOfMouseOvers.

Kill NewTabPage.NumberOfMouseOvers.

It's not clear what it would mean for the behavior to be correct, let
alone whether it actually is.

BUG=611341
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/c7c5c817f22c6785a44a681bc562d05c6c4646d0
Cr-Commit-Position: refs/heads/master@{#404362}

[sfiera, 2016-06-24 14:38:53]

sfiera@chromium.org changed reviewers:
+ treib@chromium.org

[sfiera, 2016-06-24 14:38:54]

Might as well.

[Marc Treib, 2016-06-24 14:48:02]

Yay! But please:
- Also remove/comment out/mark deprecated the enum entry in ntp_logging_events.h
- Also remove the code that actually sends the event (in javascript)

https://codereview.chromium.org/2091323002/diff/1/chrome/browser/ui/webui/ntp...
File chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc (right):

https://codereview.chromium.org/2091323002/diff/1/chrome/browser/ui/webui/ntp...
chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc:206: case NTP_MOUSEOVER:
Remove the case, so it'll fall into the NOTREACHED below.

[Marc Treib, 2016-06-24 14:50:55]

On 2016/06/24 14:48:02, Marc Treib wrote:
> Yay! But please:
> - Also remove/comment out/mark deprecated the enum entry in
ntp_logging_events.h
> - Also remove the code that actually sends the event (in javascript)

...and also: Mark the entry in histograms.xml as obsolete

[sfiera, 2016-07-01 11:14:41]

Description was changed from

==========
Kill NewTabPage.NumberOfMouseOvers.

It's not clear what it would mean for the behavior to be correct, let
alone whether it actually is.

BUG=611341
==========

to

==========
Kill NewTabPage.NumberOfMouseOvers.

It's not clear what it would mean for the behavior to be correct, let
alone whether it actually is.

BUG=611341
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[sfiera, 2016-07-01 16:22:14]

PTAL

[Marc Treib, 2016-07-05 08:07:31]

LGTM!

https://codereview.chromium.org/2091323002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/local_ntp/most_visited_single.js (left):

https://codereview.chromium.org/2091323002/diff/20001/chrome/browser/resource...
chrome/browser/resources/local_ntp/most_visited_single.js:43: NTP_MOUSEOVER: 9,
Hm, there's a few other ones here that say "unused here". Could you remove the
others too, or comment them all out?

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
File chrome/common/ntp_logging_events.h (right):

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
chrome/common/ntp_logging_events.h:11: // Note: Keep in sync with
browser/resources/local_ntp/most_visited_util.js
Could you add "most_visited_single.js"?

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
chrome/common/ntp_logging_events.h:43: // Deleted: NTP_MOUSEOVER = 9
Since this is not used as UMA, we could actually remove it, and make
NTP_TILE_LOADED = 9 (I don't really mind either way though).

[sfiera, 2016-07-05 18:24:40]

https://codereview.chromium.org/2091323002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/local_ntp/most_visited_single.js (left):

https://codereview.chromium.org/2091323002/diff/20001/chrome/browser/resource...
chrome/browser/resources/local_ntp/most_visited_single.js:43: NTP_MOUSEOVER: 9,
On 2016/07/05 08:07:31, Marc Treib wrote:
> Hm, there's a few other ones here that say "unused here". Could you remove the
> others too, or comment them all out?

Done.

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
File chrome/common/ntp_logging_events.h (right):

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
chrome/common/ntp_logging_events.h:11: // Note: Keep in sync with
browser/resources/local_ntp/most_visited_util.js
On 2016/07/05 08:07:31, Marc Treib wrote:
> Could you add "most_visited_single.js"?

Done.

https://codereview.chromium.org/2091323002/diff/20001/chrome/common/ntp_loggi...
chrome/common/ntp_logging_events.h:43: // Deleted: NTP_MOUSEOVER = 9
On 2016/07/05 08:07:31, Marc Treib wrote:
> Since this is not used as UMA, we could actually remove it, and make
> NTP_TILE_LOADED = 9 (I don't really mind either way though).

I'd just as soon leave it as is. We'll probably make bigger changes here later.

[sfiera, 2016-07-05 18:24:45]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2016-07-05 18:24:45]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org
Link to the patchset: https://codereview.chromium.org/2091323002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-07-05 18:24:52]

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-07-05 18:32:09]

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

[commit-bot: I haz the power, 2016-07-05 18:32:10]

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

[sfiera, 2016-07-05 18:43:45]

sfiera@chromium.org changed reviewers:
+ asvitkine@chromium.org, bauerb@chromium.org

[sfiera, 2016-07-05 18:43:46]

+bauerb for these:
    chrome/browser/ui/webui/metrics_handler.cc
    chrome/browser/ui/webui/metrics_handler.h

+asvitkine for histogram deprecation:
    tools/metrics/histograms/histograms.xml

bauerb covers these, but why aren't they covered by the same OWNERS file as the
rest of the NTP?
    chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
    chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
    chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc

And this one?
    chrome/common/ntp_logging_events.h

[Alexei Svitkine (slow), 2016-07-05 18:46:40]

lgtm

[Marc Treib, 2016-07-06 08:03:35]

On 2016/07/05 18:43:46, sfiera wrote:
> bauerb covers these, but why aren't they covered by the same OWNERS file as
the
> rest of the NTP?
>     chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
>     chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
>     chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc

They probably should be; I'll get a CL up.

> And this one?
>     chrome/common/ntp_logging_events.h

https://codereview.chromium.org/2122903002/ :)

[sfiera, 2016-07-06 08:10:36]

I love merge conflicts

[Bernhard Bauer, 2016-07-06 08:24:11]

lgtm

[sfiera, 2016-07-08 09:37:30]

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

[commit-bot: I haz the power, 2016-07-08 09:38:41]

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-07-08 11:51:33]

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

[commit-bot: I haz the power, 2016-07-08 11:51:34]

Dry run: This issue passed the CQ dry run.

[sfiera, 2016-07-08 11:56:53]

The CQ bit was checked by sfiera@chromium.org

[sfiera, 2016-07-08 11:56:53]

The patchset sent to the CQ was uploaded after l-g-t-m from treib@chromium.org,
bauerb@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2091323002/#ps100001
(title: "Add additional file to sync.")

[commit-bot: I haz the power, 2016-07-08 11:57:40]

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-07-08 14:15:55]

Committed patchset #6 (id:100001)

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

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-08 14:17:57]

Description was changed from

==========
Kill NewTabPage.NumberOfMouseOvers.

It's not clear what it would mean for the behavior to be correct, let
alone whether it actually is.

BUG=611341
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Kill NewTabPage.NumberOfMouseOvers.

It's not clear what it would mean for the behavior to be correct, let
alone whether it actually is.

BUG=611341
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-07-08 14:17:58]

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