Adding UMA metric to record the type of suggestion displayed on the NTP's Most Visited section.

Adding UMA metric to record the type of suggestion displayed on the NTP's Most Visited section.

BUG=322648
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236973

[beaudoin, 2013-11-22 19:48:59]

mathp: Overall logic.

asvitkine:
  tools/metrics/histograms/histograms.xml
dbeam:
  chrome/browser/resources/local_ntp/most_visited_util.js
  chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
  chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
sky:
  chrome/browser/resources/local_ntp/most_visited_util.js
  chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
  chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
  chrome/common/ntp_logging_events.h

[Mathieu, 2013-11-22 19:58:18]

lgtm, see nits

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/res...
File chrome/browser/resources/local_ntp/most_visited_util.js (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/res...
chrome/browser/resources/local_ntp/most_visited_util.js:33: // We got a
suggestion coming from the server.
nit: Remove reference to "We" here and below

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/ui/...
File chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/ui/...
chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc:28: void
NTPUserDataLogger::EmitThumbnailErrorRate() {
update function name to indicate expanded role

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/common/ntp_...
File chrome/common/ntp_logging_events.h (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/common/ntp_...
chrome/common/ntp_logging_events.h:28: // We got a suggestion coming from the
server.
nit: no we

[beaudoin, 2013-11-22 20:09:34]

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/res...
File chrome/browser/resources/local_ntp/most_visited_util.js (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/res...
chrome/browser/resources/local_ntp/most_visited_util.js:33: // We got a
suggestion coming from the server.
On 2013/11/22 19:58:19, Mathieu Perreault wrote:
> nit: Remove reference to "We" here and below

Done.

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/ui/...
File chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/browser/ui/...
chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc:28: void
NTPUserDataLogger::EmitThumbnailErrorRate() {
On 2013/11/22 19:58:19, Mathieu Perreault wrote:
> update function name to indicate expanded role

Renamed and called independently.

Done.

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/common/ntp_...
File chrome/common/ntp_logging_events.h (right):

https://chromiumcodereview.appspot.com/83943002/diff/30001/chrome/common/ntp_...
chrome/common/ntp_logging_events.h:28: // We got a suggestion coming from the
server.
On 2013/11/22 19:58:19, Mathieu Perreault wrote:
> nit: no we

Done.

[Alexei Svitkine (slow), 2013-11-22 20:11:52]

histograms.xml LGTM

You should probably have a chromium bug to track a change of this size.

[beaudoin, 2013-11-22 20:14:49]

On 2013/11/22 20:11:52, Alexei Svitkine wrote:
> histograms.xml LGTM
> 
> You should probably have a chromium bug to track a change of this size.

Added a bug.

[sky, 2013-11-22 21:28:40]

LGTM

[commit-bot: I haz the power, 2013-11-23 16:10:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/beaudoin@chromium.org/83943002/30002

[commit-bot: I haz the power, 2013-11-23 16:57:04]

Retried try job too often on mac_rel for step(s) telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2013-11-23 18:32:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/beaudoin@chromium.org/83943002/30002

[commit-bot: I haz the power, 2013-11-23 20:59:12]

Change committed as 236973

[Dan Beam, 2013-11-25 20:58:13]

On 2013/11/22 19:48:59, beaudoin wrote:
> mathp: Overall logic.
> 
> asvitkine:
>   tools/metrics/histograms/histograms.xml
> dbeam:
>   chrome/browser/resources/local_ntp/most_visited_util.js
>   chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
>   chrome/browser/ui/webui/ntp/ntp_user_data_logger.h

^ why ask for review if you're not going to wait for one?

> sky:
>   chrome/browser/resources/local_ntp/most_visited_util.js
>   chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
>   chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
>   chrome/common/ntp_logging_events.h

[beaudoin, 2013-11-25 21:05:26]

On 2013/11/25 20:58:13, Dan Beam wrote:
> On 2013/11/22 19:48:59, beaudoin wrote:
> > mathp: Overall logic.
> > 
> > asvitkine:
> >   tools/metrics/histograms/histograms.xml
> > dbeam:
> >   chrome/browser/resources/local_ntp/most_visited_util.js
> >   chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
> >   chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
> 
> ^ why ask for review if you're not going to wait for one?
> 
> > sky:
> >   chrome/browser/resources/local_ntp/most_visited_util.js
> >   chrome/browser/ui/webui/ntp/ntp_user_data_logger.cc
> >   chrome/browser/ui/webui/ntp/ntp_user_data_logger.h
> >   chrome/common/ntp_logging_events.h

Sorry about that I used "git cl owners" for the first time and it suggested you
and sky but I missed the fact that your files were a subset of his. I should
have pointed it out when I removed you from the review. :|