Files.app: Add UMA for HardUnplugged events.

Files.app: Add UMA for HardUnplugged events.

The CL adds UMA for the number of seconds:

 * between HardUnplugged event and Suspend event
 * between SuspendDone event and HardUnplugged event

The numbers are needed for setting the best suppresing time for the hard
unplugged event.

BUG=433734
TEST=None

Committed: https://crrev.com/7f60d255a1c40caf8985552155a0ae46acd45299
Cr-Commit-Position: refs/heads/master@{#306523}

[hirono, 2014-11-17 07:25:50]

hirono@chromium.org changed reviewers:
+ mtomasz@chromium.org

[hirono, 2014-11-17 07:25:50]

PTAL the CL? Thanks!

[mtomasz, 2014-11-17 08:48:04]

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
(right):

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
chrome/browser/chromeos/extensions/file_manager/device_event_router.cc:135:
UMA_HISTOGRAM_CUSTOM_TIMES("FileBrowser.HardUnpluggedBeforeSuspend",
I think that if there was no unplugging, then we will generate a histogram with
incorrect data.

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.h
(right):

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
chrome/browser/chromeos/extensions/file_manager/device_event_router.h:103:
base::Time last_hard_unpluged_;
typo: unplugged

[hirono, 2014-11-25 02:32:13]

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
(right):

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
chrome/browser/chromeos/extensions/file_manager/device_event_router.cc:135:
UMA_HISTOGRAM_CUSTOM_TIMES("FileBrowser.HardUnpluggedBeforeSuspend",
On 2014/11/17 08:48:03, mtomasz wrote:
> I think that if there was no unplugging, then we will generate a histogram
with
> incorrect data.

Done.

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.h
(right):

https://codereview.chromium.org/723923004/diff/1/chrome/browser/chromeos/exte...
chrome/browser/chromeos/extensions/file_manager/device_event_router.h:103:
base::Time last_hard_unpluged_;
On 2014/11/17 08:48:03, mtomasz wrote:
> typo: unplugged

Done.

[mtomasz, 2014-11-25 03:55:19]

lgtm

[hirono, 2014-11-25 04:36:46]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-11-25 04:37:03]

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

[commit-bot: I haz the power, 2014-11-25 04:44:29]

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

[commit-bot: I haz the power, 2014-11-25 04:44:31]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hirono, 2014-11-25 10:07:35]

hirono@chromium.org changed reviewers:
+ jar@chromium.org

[hirono, 2014-11-25 10:07:36]

+jar - PTAL histograms.xml? Thanks!

[jar (doing other things), 2014-11-25 17:41:31]

jar@chromium.org changed reviewers:
+ asvitkine@chromium.org
- jar@chromium.org

[jar (doing other things), 2014-11-25 17:41:32]

+asvitkine for histograms.xml

[Alexei Svitkine (slow), 2014-11-25 17:48:13]

https://codereview.chromium.org/723923004/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
(right):

https://codereview.chromium.org/723923004/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/device_event_router.cc:89:
base::TimeDelta::FromSeconds(30), 50);
Nit: I suggest using UMA_HISTOGRAM_MEDIUM_TIMES(). The only difference is that
one has a cap of 3mins instead of 30s. But since we use exponential buckets, I
don't think it would be a big change to the distribution you're getting and it's
less custom code. Same for the one below.

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

https://codereview.chromium.org/723923004/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:9141: +<histogram
name="FileBrowser.HardUnpluggedAfterResume" units="milliseconds">
Nit: The metric names don't make it sound obvious what's being recorded.

Maybe something like:

FileBrowser.HardUnpluggedAfterResume.TimeSinceSuspend

Same for the other one.

[hirono, 2014-12-02 08:52:43]

https://codereview.chromium.org/723923004/diff/20001/chrome/browser/chromeos/...
File chrome/browser/chromeos/extensions/file_manager/device_event_router.cc
(right):

https://codereview.chromium.org/723923004/diff/20001/chrome/browser/chromeos/...
chrome/browser/chromeos/extensions/file_manager/device_event_router.cc:89:
base::TimeDelta::FromSeconds(30), 50);
On 2014/11/25 17:48:12, Alexei Svitkine (OOO) wrote:
> Nit: I suggest using UMA_HISTOGRAM_MEDIUM_TIMES(). The only difference is that
> one has a cap of 3mins instead of 30s. But since we use exponential buckets, I
> don't think it would be a big change to the distribution you're getting and
it's
> less custom code. Same for the one below.

Done.

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

https://codereview.chromium.org/723923004/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:9141: +<histogram
name="FileBrowser.HardUnpluggedAfterResume" units="milliseconds">
On 2014/11/25 17:48:12, Alexei Svitkine (OOO) wrote:
> Nit: The metric names don't make it sound obvious what's being recorded.
> 
> Maybe something like:
> 
> FileBrowser.HardUnpluggedAfterResume.TimeSinceSuspend
> 
> Same for the other one.

Done.

[Alexei Svitkine (slow), 2014-12-02 15:03:35]

lgtm

[hirono, 2014-12-03 00:33:01]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2014-12-03 00:34:13]

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

[commit-bot: I haz the power, 2014-12-03 01:49:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-03 01:50:10]

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