Add UMA stats for pen palette

Add UMA stats for pen palette

This CL adds the following metrics:
1. Usage of each pen palette option
2. Usage of each pen palette option on auto-open
3. Length of time spent in each mode
4. count of Mode cancellation

BUG=643798
Committed: https://crrev.com/50a94860a8398abd112028dc645b5d523372f26a
Cr-Commit-Position: refs/heads/master@{#418645}

[xiaoyinh(OOO Sep 11-29), 2016-09-02 21:10:17]

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

[commit-bot: I haz the power, 2016-09-02 21:10:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiaoyinh(OOO Sep 11-29), 2016-09-02 21:14:29]

xiaoyinh@chromium.org changed reviewers:
+ isherman@chromium.org, jdufault@chromium.org, stevenjb@chromium.org

[commit-bot: I haz the power, 2016-09-02 21:25:00]

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

[commit-bot: I haz the power, 2016-09-02 21:25:01]

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

[Ilya Sherman, 2016-09-02 21:39:31]

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:67:
UMA_HISTOGRAM_LONG_TIMES_100("Ash.Shelf.Palette.InLaserPointerMode",
Hmm, are values under 100 ms at all likely?  I would recommend using an
UMA_HISTOGRAM_CUSTOM_TIMES, with 50 buckets, and a higher minimum.  Ditto below.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tool_manager.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:53:
ash::PaletteModeCancelType::PALETTE_MODE_CANCEL_TYPE_COUNT);
Please create a wrapper function for emitting to this histogram, that you could
call as

 
RecordPaletteModeCancellation(PaletteModeCancelType::PALETTE_MODE_MAGNIFY_SWITCHED)

(Note, by the way, that you're already in the "ash" namespace, and hence can
drop the "ash::" prefix.)

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:21: #include
"base/metrics/histogram.h"
nit: histogram_macros

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:325: if
(is_palette_auto_opened)
nit: Please use curly braces, since the body spans multiple lines of text

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1536: +<histogram
name="Ash.Shelf.PaletteUsage" enum="PaletteTrayOptions">
nit: Please name the suffix "Palette.Usage" to match the other metrics. 
Likewise below.

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1538: +  <summary>Tracks the usage of
each pen palette option.</summary>
Please document that this is *not* recorded for auto-opened palettes.

[jdufault, 2016-09-02 21:54:30]

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:66: if (GetToolId() ==
ash::PaletteToolId::LASER_POINTER) {
What about something like

if (ShouldRecordUma(GetToolId())
  UMA_HISTOGRAM...(GetUmaName(GetToolId()), ...);

And then ShouldRecordUma / GetUmaName are defined in the anonymous namespace of
this file.

Though to avoid pulling in every tool type for this specific file maybe we want
to do something like

if (GetActiveDurationUmaName())
  UMA_HISTOGRAM(GetActiveDurationUmaName(), ...)

Where GetActiveDurationUmaName() is a virtual method that returns an std::string
instance. If the tool wants to record an UMA value, it returns a non-empty
string.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.h:41: base::TimeTicks
start_time_;
Can we use base::Time?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.h:41: base::TimeTicks
start_time_;
Add a comment, maybe something like

// When the tool last became active. Used for recording UMA metrics.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_ids.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_ids.cc:43: PaletteTrayOptions option
= PALETTE_OPTIONS_COUNT;
Can we just return the value directly?

switch (tool_id) {
  case PaletteToolId::CREATE_NOTE:
    return PALETTE_NEW_NOTE;
  // ...
}

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_ids.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_ids.h:16: enum PaletteTrayOptions {
nit: Put these after either PaletteToolId or PaletteGroupIdToString() 

When reading I think PaletteGroup / PaletteToolId are more important for
conceptual understanding.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tool_manager.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:44: if
(previous_tool->GetToolId() == ash::PaletteToolId::LASER_POINTER) {
The only time we can cancel if is if we click the view again, right? What about
recording the cancel metrics inside of the "disable" if block of
CommonPaletteTool::OnViewClicked?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:44: if
(previous_tool->GetToolId() == ash::PaletteToolId::LASER_POINTER) {
nit: drop ash:: (already in the ash namespace)

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:70:
UMA_HISTOGRAM_ENUMERATION(
Should these UMA stats also have the AutoOpen check?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:114:
WmShell::Get()->system_tray_delegate()->ShowPaletteSettings();
ShowPaletteSettings should already be recording a metric ("ShowPaletteOptions"
to be precise).

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:322: if (option ==
ash::PaletteTrayOptions::PALETTE_OPTIONS_COUNT)
Do we want to DCHECK on this?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:325: if
(is_palette_auto_opened)
nit: add {} for both if/else

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:119: bool
is_palette_auto_opened = false;
What about is_bubble_auto_opened_?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:122: int
num_actions_in_palette = 0;
num_actions_in_bubble_?

[Ilya Sherman, 2016-09-02 22:02:49]

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:66: if (GetToolId() ==
ash::PaletteToolId::LASER_POINTER) {
On 2016/09/02 21:54:30, jdufault wrote:
> What about something like
> 
> if (ShouldRecordUma(GetToolId())
>   UMA_HISTOGRAM...(GetUmaName(GetToolId()), ...);
> 
> And then ShouldRecordUma / GetUmaName are defined in the anonymous namespace
of
> this file.
> 
> Though to avoid pulling in every tool type for this specific file maybe we
want
> to do something like
> 
> if (GetActiveDurationUmaName())
>   UMA_HISTOGRAM(GetActiveDurationUmaName(), ...)
> 
> Where GetActiveDurationUmaName() is a virtual method that returns an
std::string
> instance. If the tool wants to record an UMA value, it returns a non-empty
> string.

Histogram names passed to UMA_HISTOGRAM_ macros must be runtime constant, so I
don't think your suggestion will work.  You'll either need to call the
underlying C++ code directly, or have different code paths for different
histogram names.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.h:41: base::TimeTicks
start_time_;
On 2016/09/02 21:54:30, jdufault wrote:
> Can we use base::Time?

base::Time is susceptible to timezone changes, etc.  base::TimeTicks is
generally preferred for measuring durations.

[xiaoyinh(OOO Sep 11-29), 2016-09-06 17:40:06]

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:67:
UMA_HISTOGRAM_LONG_TIMES_100("Ash.Shelf.Palette.InLaserPointerMode",
On 2016/09/02 21:39:30, Ilya Sherman wrote:
> Hmm, are values under 100 ms at all likely?  I would recommend using an
> UMA_HISTOGRAM_CUSTOM_TIMES, with 50 buckets, and a higher minimum.  Ditto
below.

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.h:41: base::TimeTicks
start_time_;
On 2016/09/02 21:54:30, jdufault wrote:
> Add a comment, maybe something like
> 
> // When the tool last became active. Used for recording UMA metrics.

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_ids.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_ids.cc:43: PaletteTrayOptions option
= PALETTE_OPTIONS_COUNT;
On 2016/09/02 21:54:30, jdufault wrote:
> Can we just return the value directly?
> 
> switch (tool_id) {
>   case PaletteToolId::CREATE_NOTE:
>     return PALETTE_NEW_NOTE;
>   // ...
> }

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_ids.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_ids.h:16: enum PaletteTrayOptions {
On 2016/09/02 21:54:30, jdufault wrote:
> nit: Put these after either PaletteToolId or PaletteGroupIdToString() 
> 
> When reading I think PaletteGroup / PaletteToolId are more important for
> conceptual understanding.

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tool_manager.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:44: if
(previous_tool->GetToolId() == ash::PaletteToolId::LASER_POINTER) {
On 2016/09/02 21:54:30, jdufault wrote:
> nit: drop ash:: (already in the ash namespace)

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:44: if
(previous_tool->GetToolId() == ash::PaletteToolId::LASER_POINTER) {
On 2016/09/02 21:54:30, jdufault wrote:
> The only time we can cancel if is if we click the view again, right? What
about
> recording the cancel metrics inside of the "disable" if block of
> CommonPaletteTool::OnViewClicked?

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:53:
ash::PaletteModeCancelType::PALETTE_MODE_CANCEL_TYPE_COUNT);
On 2016/09/02 21:39:30, Ilya Sherman wrote:
> Please create a wrapper function for emitting to this histogram, that you
could
> call as
> 
>  
>
RecordPaletteModeCancellation(PaletteModeCancelType::PALETTE_MODE_MAGNIFY_SWITCHED)
> 
> (Note, by the way, that you're already in the "ash" namespace, and hence can
> drop the "ash::" prefix.)

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tool_manager.cc:70:
UMA_HISTOGRAM_ENUMERATION(
On 2016/09/02 21:54:30, jdufault wrote:
> Should these UMA stats also have the AutoOpen check?

AutoOpen factor are not specified for mode cancellation in the document.
So I didn't add it here. Let me know if you think it's necessary.

In general, clicks on view(including both mode activation and cancellation)
already considered autoOpened factor.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:21: #include
"base/metrics/histogram.h"
On 2016/09/02 21:39:30, Ilya Sherman wrote:
> nit: histogram_macros

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:114:
WmShell::Get()->system_tray_delegate()->ShowPaletteSettings();
On 2016/09/02 21:54:30, jdufault wrote:
> ShowPaletteSettings should already be recording a metric ("ShowPaletteOptions"
> to be precise).

Should I remove mine if that's case? It might still be useful as
"ShowPaletteOptions" is recorded as user-action, and here we can compare 
PALETTE_SETTINGS_BUTTON with all other options side by side?

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:322: if (option ==
ash::PaletteTrayOptions::PALETTE_OPTIONS_COUNT)
On 2016/09/02 21:54:30, jdufault wrote:
> Do we want to DCHECK on this?

Added. Thanks

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:325: if
(is_palette_auto_opened)
On 2016/09/02 21:39:30, Ilya Sherman wrote:
> nit: Please use curly braces, since the body spans multiple lines of text

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:325: if
(is_palette_auto_opened)
On 2016/09/02 21:54:30, jdufault wrote:
> nit: add {} for both if/else

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.h (right):

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:119: bool
is_palette_auto_opened = false;
On 2016/09/02 21:54:30, jdufault wrote:
> What about is_bubble_auto_opened_?

Done.

https://codereview.chromium.org/2308823002/diff/20001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:122: int
num_actions_in_palette = 0;
On 2016/09/02 21:54:30, jdufault wrote:
> num_actions_in_bubble_?

Done.

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1536: +<histogram
name="Ash.Shelf.PaletteUsage" enum="PaletteTrayOptions">
On 2016/09/02 21:39:31, Ilya Sherman wrote:
> nit: Please name the suffix "Palette.Usage" to match the other metrics. 
> Likewise below.

Done.

https://codereview.chromium.org/2308823002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1538: +  <summary>Tracks the usage of
each pen palette option.</summary>
On 2016/09/02 21:39:31, Ilya Sherman wrote:
> Please document that this is *not* recorded for auto-opened palettes.

Done.

[xiaoyinh(OOO Sep 11-29), 2016-09-06 17:42:21]

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

[commit-bot: I haz the power, 2016-09-06 17:42:45]

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-09-06 18:46:26]

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

[commit-bot: I haz the power, 2016-09-06 18:46:27]

Dry run: This issue passed the CQ dry run.

[Ilya Sherman, 2016-09-06 20:45:32]

Thanks!  Metrics LGTM % nits:

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:97:
delegate()->EnableTool(GetToolId());
nit: Please add curly braces to the else stmt, since the if stmt has them.

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:334: DCHECK(type !=
PaletteModeCancelType::PALETTE_MODE_CANCEL_TYPE_COUNT);
nit: DCHECK_NE (and ditto above)

[xiaoyinh(OOO Sep 11-29), 2016-09-06 22:31:35]

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/palette/common_palette_tool.cc:97:
delegate()->EnableTool(GetToolId());
On 2016/09/06 20:45:32, Ilya Sherman wrote:
> nit: Please add curly braces to the else stmt, since the if stmt has them.

Done.

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/40001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:334: DCHECK(type !=
PaletteModeCancelType::PALETTE_MODE_CANCEL_TYPE_COUNT);
On 2016/09/06 20:45:32, Ilya Sherman wrote:
> nit: DCHECK_NE (and ditto above)

Done.

[xiaoyinh(OOO Sep 11-29), 2016-09-08 16:27:57]

stevenjb@, jdufault@, please take a look. Thanks!

[jdufault, 2016-09-08 19:51:35]

lgtm

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:181: if
(num_actions_in_bubble_ == 0) {
nit: drop {}

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.h (right):

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:119: // stylus
nit: fix wrapping

[xiaoyinh(OOO Sep 11-29), 2016-09-08 23:15:27]

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

[commit-bot: I haz the power, 2016-09-08 23:16:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[xiaoyinh(OOO Sep 11-29), 2016-09-08 23:20:59]

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.cc (right):

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.cc:181: if
(num_actions_in_bubble_ == 0) {
On 2016/09/08 19:51:35, jdufault wrote:
> nit: drop {}

Done.

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
File ash/common/system/chromeos/palette/palette_tray.h (right):

https://codereview.chromium.org/2308823002/diff/60001/ash/common/system/chrom...
ash/common/system/chromeos/palette/palette_tray.h:119: // stylus
On 2016/09/08 19:51:35, jdufault wrote:
> nit: fix wrapping

Done.

[commit-bot: I haz the power, 2016-09-08 23:24:16]

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

[commit-bot: I haz the power, 2016-09-08 23:24:17]

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

[xiaoyinh(OOO Sep 11-29), 2016-09-08 23:38:24]

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

[commit-bot: I haz the power, 2016-09-08 23:39:02]

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-09-09 00:48:58]

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

[commit-bot: I haz the power, 2016-09-09 00:49:03]

Dry run: This issue passed the CQ dry run.

[stevenjb, 2016-09-13 16:32:18]

lgtm w/ one suggestion

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/palette/common_palette_tool.cc:77: }
Do we anticipate adding more tools? If so I would move this into a helper
function, e.g. AddHistogramTimes().

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/palette/common_palette_tool.cc:96: }
same here.

[xiaoyinh(OOO Sep 11-29), 2016-09-14 00:51:25]

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
File ash/common/system/chromeos/palette/common_palette_tool.cc (right):

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/palette/common_palette_tool.cc:77: }
On 2016/09/13 16:32:18, stevenjb wrote:
> Do we anticipate adding more tools? If so I would move this into a helper
> function, e.g. AddHistogramTimes().

Done.

https://codereview.chromium.org/2308823002/diff/100001/ash/common/system/chro...
ash/common/system/chromeos/palette/common_palette_tool.cc:96: }
On 2016/09/13 16:32:18, stevenjb wrote:
> same here.

Done.

[xiaoyinh(OOO Sep 11-29), 2016-09-14 00:51:44]

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

[commit-bot: I haz the power, 2016-09-14 00:52:12]

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-09-14 02:53:25]

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

[commit-bot: I haz the power, 2016-09-14 02:53:26]

Dry run: This issue passed the CQ dry run.

[xiaoyinh(OOO Sep 11-29), 2016-09-14 17:55:20]

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

[commit-bot: I haz the power, 2016-09-14 17:55:44]

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-09-14 19:18:40]

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

[commit-bot: I haz the power, 2016-09-14 19:18:42]

Dry run: This issue passed the CQ dry run.

[xiaoyinh(OOO Sep 11-29), 2016-09-14 19:36:52]

The CQ bit was checked by xiaoyinh@chromium.org

[xiaoyinh(OOO Sep 11-29), 2016-09-14 19:36:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, jdufault@chromium.org, stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2308823002/#ps140001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-14 19:37:30]

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-09-14 19:45:01]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-14 19:47:14]

Description was changed from

==========
Add UMA stats for pen palette

This CL adds the following metrics:
1. Usage of each pen palette option
2. Usage of each pen palette option on auto-open
3. Length of time spent in each mode
4. count of Mode cancellation

BUG=643798
==========

to

==========
Add UMA stats for pen palette

This CL adds the following metrics:
1. Usage of each pen palette option
2. Usage of each pen palette option on auto-open
3. Length of time spent in each mode
4. count of Mode cancellation

BUG=643798

Committed: https://crrev.com/50a94860a8398abd112028dc645b5d523372f26a
Cr-Commit-Position: refs/heads/master@{#418645}
==========

[commit-bot: I haz the power, 2016-09-14 19:47:16]

Patchset 8 (id:??) landed as
https://crrev.com/50a94860a8398abd112028dc645b5d523372f26a
Cr-Commit-Position: refs/heads/master@{#418645}