UMA track signed in duration at signout

Track how many minutes a profile was signed in before when we sign out.

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

[Mike Lerman, 2014-08-11 19:27:26]

Cherished reviewers,

Your humble colleague requests the gift of your review.

Obsequious thanks,
Mike

[Roger Tawa OOO till Jul 10th, 2014-08-12 14:14:52]

lgtm

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
File components/signin/core/browser/signin_manager.cc (right):

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager.cc:220:
signed_in_duration.InMinutes()));
Should we be paranoid and check that delta > 0?

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
File components/signin/core/browser/signin_manager_base.cc (right):

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager_base.cc:94:
base::Time::Now().ToInternalValue());
Seems like it would be better to put this in OnSignedIn(), right before the call
to SetAuthenticatedUsername().

[Alexei Svitkine (slow), 2014-08-12 14:41:49]

lgtm

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
File components/signin/core/browser/signin_manager.cc (right):

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager.cc:218: base::TimeDelta
signed_in_duration = base::Time::Now() - signin_time;
Note: Be aware that if the user's machine time changes, you'll get inconsistent
results. But presumably that should be a minority of users.

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager.cc:220:
signed_in_duration.InMinutes()));
On 2014/08/12 14:14:52, Roger Tawa wrote:
> Should we be paranoid and check that delta > 0?

I believe it should automatically go in the underflow bucket, so its "OK" in the
sense that there won't be any crashes etc.

[Mike Lerman, 2014-08-12 18:07:11]

Thank you both!

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
File components/signin/core/browser/signin_manager.cc (right):

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager.cc:218: base::TimeDelta
signed_in_duration = base::Time::Now() - signin_time;
On 2014/08/12 14:41:49, Alexei Svitkine wrote:
> Note: Be aware that if the user's machine time changes, you'll get
inconsistent
> results. But presumably that should be a minority of users.

I assume this will indeed be a very small proportion of users.

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager.cc:220:
signed_in_duration.InMinutes()));
On 2014/08/12 14:41:49, Alexei Svitkine wrote:
> On 2014/08/12 14:14:52, Roger Tawa wrote:
> > Should we be paranoid and check that delta > 0?
> 
> I believe it should automatically go in the underflow bucket, so its "OK" in
the
> sense that there won't be any crashes etc.

Acknowledged.

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
File components/signin/core/browser/signin_manager_base.cc (right):

https://codereview.chromium.org/459853002/diff/60001/components/signin/core/b...
components/signin/core/browser/signin_manager_base.cc:94:
base::Time::Now().ToInternalValue());
On 2014/08/12 14:14:52, Roger Tawa wrote:
> Seems like it would be better to put this in OnSignedIn(), right before the
call
> to SetAuthenticatedUsername().

Done.

[Mike Lerman, 2014-08-12 18:07:14]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-12 18:09:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlerman@chromium.org/459853002/80001

[commit-bot: I haz the power, 2014-08-12 18:58:09]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/40652)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/45890)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg/bui...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-12 19:00:26]

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

[commit-bot: I haz the power, 2014-08-12 19:00:27]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Mike Lerman, 2014-08-13 18:07:40]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-13 18:09:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlerman@chromium.org/459853002/100001

[commit-bot: I haz the power, 2014-08-13 23:18:21]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-08-13 23:22:25]

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

[commit-bot: I haz the power, 2014-08-13 23:22:26]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Mike Lerman, 2014-08-14 11:16:57]

The CQ bit was checked by mlerman@chromium.org

[commit-bot: I haz the power, 2014-08-14 11:22:26]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mlerman@chromium.org/459853002/100001

[commit-bot: I haz the power, 2014-08-14 14:02:33]

Committed patchset #3 (100001) as 289545