Record time between switching the active window from Ash overview mode

Record time between switching the active window from Ash overview mode

Introduce the histogram
Ash.WindowSelector.TimeBetweenActiveWindowChanges
which records the amount of time between uses
of overview mode in which the user selects a window
which was not previously active.

BUG=488180
TEST=TaskSwitchMetricsRecorderTest.VerifyTaskSwitchesFromOverviewModeAreRecorded

Committed: https://crrev.com/0a7ca3368dc1e3c0a6086b84aab005d12ecd3d70
Cr-Commit-Position: refs/heads/master@{#330260}

[tdanderson, 2015-05-14 22:20:58]

tdanderson@chromium.org changed reviewers:
+ bruthig@chromium.org

[tdanderson, 2015-05-14 22:20:58]

Ben, can you PTAL?

[bruthig, 2015-05-14 22:54:52]

See comments inline

https://codereview.chromium.org/1132293004/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1132293004/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:1007: +    overview mode (regardless of
what the user did while in overview).
I would recommend you update the description for the
Ash.WindowSelector.TimeBetweenUse to be a little more accurate so a distinction
is clear without having to be explain it here.

As for a description for "Ash.WindowSelector.TimeBetweenActiveWindowChanges" how
about this?
"The amount of time between overview mode ending's caused by the user activating
a window which was not active when overview mode started."

[tdanderson, 2015-05-14 23:21:38]

tdanderson@chromium.org changed reviewers:
+ mpearson@chromium.org, oshima@chromium.org

[tdanderson, 2015-05-14 23:21:39]

oshima@, can you please take a look at ash/ ?

mpearson@, can you please take a look at tools/ ?

https://codereview.chromium.org/1132293004/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1132293004/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:1007: +    overview mode (regardless of
what the user did while in overview).
On 2015/05/14 22:54:52, bruthig wrote:
> I would recommend you update the description for the
> Ash.WindowSelector.TimeBetweenUse to be a little more accurate so a
distinction
> is clear without having to be explain it here.
> 
> As for a description for "Ash.WindowSelector.TimeBetweenActiveWindowChanges"
how
> about this?
> "The amount of time between overview mode ending's caused by the user
activating
> a window which was not active when overview mode started."
> 

Done.

[bruthig, 2015-05-14 23:24:55]

lgtm

[oshima, 2015-05-15 00:43:55]

lgtm

[tdanderson, 2015-05-15 16:05:12]

Mark, can you PTAL? Note this has been rebased after the ordering issue in
histograms.xml was fixed.

[Mark P, 2015-05-15 17:58:37]

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

https://codereview.chromium.org/1132293004/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1024: +    mode is entered.
(1) I assume this is a clarification and not a change in functionality?
(2) For further clarification, perhaps you should mention "for the second and
later times" or add "No recorded on the first time overview mode is entered
after startup." (or after profile open, or after this chrome binary is updated
to the version that records this metric, if you keep track of this number
between chrome restarts, or ...)

This clarification should probably also be applied to the histogram description
you're adding as well.

[tdanderson, 2015-05-15 19:06:51]

Mark, I have changed the description as you suggested. Can you please take
another look?

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

https://codereview.chromium.org/1132293004/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1024: +    mode is entered.
On 2015/05/15 17:58:37, Mark P wrote:
> (1) I assume this is a clarification and not a change in functionality?

Yes, it's just a clarification since the two histograms do
track similar (but not identical) things.

> (2) For further clarification, perhaps you should mention "for the second and
> later times" or add "No recorded on the first time overview mode is entered
> after startup." (or after profile open, or after this chrome binary is updated
> to the version that records this metric, if you keep track of this number
> between chrome restarts, or ...)

Done. 

> 
> This clarification should probably also be applied to the histogram
description
> you're adding as well.

Done.

[Mark P, 2015-05-15 19:26:05]

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

https://codereview.chromium.org/1132293004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1010: +    units="seconds">
Given that this histogram recording goes through a similar code path as the one
below, I'm surprised they have different units.  Is that correct?

https://codereview.chromium.org/1132293004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1016: +    recorded on the first time
overview mode is entered after startup.
instead of
Not recorded on the first time overview mode is entered after startup.
perhaps
Only recorded on the second and later times that the user selected a window
which was not previously active.

I believe this is more correct, isn't it?

[tdanderson, 2015-05-15 19:50:51]

Hi Mark, please have another look.

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

https://codereview.chromium.org/1132293004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1010: +    units="seconds">
On 2015/05/15 19:26:05, Mark P wrote:
> Given that this histogram recording goes through a similar code path as the
one
> below, I'm surprised they have different units.  Is that correct?

Both histograms should be in seconds in my opinion, but
I'm hesitant to change the units of TimeBetweenUse since
I imagine this would complicate trying to interpret the
metrics over time.

TimeBetweenActiveWindowChanges is in a family of histograms
bruthig@ and I have been adding, which goes through a 
newly-added centralized location for task-switching metrics
(TaskSwitchMetricsRecorder). The TimeBetweenUse histogram
has been around for a lot longer and its recording
actually lives directly in the overview mode code.

https://codereview.chromium.org/1132293004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1016: +    recorded on the first time
overview mode is entered after startup.
On 2015/05/15 19:26:05, Mark P wrote:
> instead of
> Not recorded on the first time overview mode is entered after startup.
> perhaps
> Only recorded on the second and later times that the user selected a window
> which was not previously active.
> 
> I believe this is more correct, isn't it?

Yes, I like that wording better. I've updated it for both histogram
descriptions.

[Mark P, 2015-05-15 19:56:26]

histograms.xml lgtm
with minor comments

--mark

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

https://codereview.chromium.org/1132293004/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1010: +    units="seconds">
On 2015/05/15 19:50:51, tdanderson wrote:
> On 2015/05/15 19:26:05, Mark P wrote:
> > Given that this histogram recording goes through a similar code path as the
> one
> > below, I'm surprised they have different units.  Is that correct?
> 
> Both histograms should be in seconds in my opinion, but
> I'm hesitant to change the units of TimeBetweenUse since
> I imagine this would complicate trying to interpret the
> metrics over time.

Oh, as long as the code is different then this is okay with me.
I was more concerned that the code does the same measurement
and one of these units= fields in histograms.xml is wrong.  If
they're right, everything is fine.

FWIW, if you want to change how code emits to a histogram
(e..g, what units that histogram is in), you should change
the histogram name at the same time.


> 
> TimeBetweenActiveWindowChanges is in a family of histograms
> bruthig@ and I have been adding, which goes through a 
> newly-added centralized location for task-switching metrics
> (TaskSwitchMetricsRecorder). The TimeBetweenUse histogram
> has been around for a lot longer and its recording
> actually lives directly in the overview mode code.

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

https://codereview.chromium.org/1132293004/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1016: +    recorded on the second and
later times that the user selected a window which
nit: please add "after startup" somewhere in this sentence
ditto below

[tdanderson, 2015-05-15 20:05:09]

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

https://codereview.chromium.org/1132293004/diff/80001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:1016: +    recorded on the second and
later times that the user selected a window which
On 2015/05/15 19:56:26, Mark P wrote:
> nit: please add "after startup" somewhere in this sentence
> ditto below

Done.

[tdanderson, 2015-05-15 22:32:49]

The CQ bit was checked by tdanderson@chromium.org

[tdanderson, 2015-05-15 22:32:50]

The patchset sent to the CQ was uploaded after l-g-t-m from oshima@chromium.org,
bruthig@chromium.org, mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1132293004/#ps110001
(title: "rebase")

[commit-bot: I haz the power, 2015-05-15 22:35:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1132293004/110001

[commit-bot: I haz the power, 2015-05-16 01:27:16]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2015-05-18 11:30:34]

Patchset 7 (id:??) landed as
https://crrev.com/0a7ca3368dc1e3c0a6086b84aab005d12ecd3d70
Cr-Commit-Position: refs/heads/master@{#330260}