Ensure that GetBootTimes is always called, since it updates histograms

Ensure that GetBootTimes is always called, since it updates histograms
as a side effect.

BUG=chrome-os-partner:4692
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=109922

[semenzato, 2011-11-09 21:53:55]

There is probably a better way of doing this, so suggestions are welcomed.  Also
suggestions for alternative reviewers.  Thanks!

[jrbarnette, 2011-11-09 23:52:55]

On 2011/11/09 21:53:55, semenzato wrote:
> There is probably a better way of doing this, so suggestions are welcomed. 
Also
> suggestions for alternative reviewers.  Thanks!

This looks reasonable to me.  We might be spared the no-op callback if
"is_official_build" were accessible to the callback handler, but I don't see
any way to get it there.  We might also be spared the no-op if we changed
the interface to GetBootTimes not to require the callback, but that seems
like a lot more work for minimal to no advantage.

In any event, I expect that any better solution would require some refactoring:
if we're going to do that, we should consider bigger steps, like moving
statistics gathering over to Chrome OS, and having Chrome be responsible
only for reporting to UMA and possibly to users.  Given the nature of this
bug, that much work should be scheduled for later.

[jrbarnette, 2011-11-09 23:54:03]

http://codereview.chromium.org/8508028/diff/1/chrome/browser/chromeos/login/v...
File chrome/browser/chromeos/login/version_info_updater.cc (right):

http://codereview.chromium.org/8508028/diff/1/chrome/browser/chromeos/login/v...
chrome/browser/chromeos/login/version_info_updater.cc:57:
&VersionInfoUpdater::OnBootTimes));
It would help to have a comment here mentioning that GetBootTimes
reports statistics to UMA, so that no one need wonder why there's
a no-op callback, and so that the next developer who breaks this won't
be able to claim "I didn't know that was needed" :-).

[jrbarnette, 2011-11-10 00:00:59]

On 2011/11/09 21:53:55, semenzato wrote:
> There is probably a better way of doing this, so suggestions are welcomed. 
Also
> suggestions for alternative reviewers.  Thanks!

One other question:  How hard is it to add a unit test that confirms UMA
metrics get sent?  It'd be nice if a test could catch this kind of bug.

[jrbarnette, 2011-11-14 17:28:47]

On 2011/11/10 00:00:59, jrbarnette wrote:
> On 2011/11/09 21:53:55, semenzato wrote:
> > There is probably a better way of doing this, so suggestions are welcomed. 
> Also
> > suggestions for alternative reviewers.  Thanks!
> 
> One other question:  How hard is it to add a unit test that confirms UMA
> metrics get sent?  It'd be nice if a test could catch this kind of bug.

Ping.  Dave, can you weigh in?  I think we want this to get in ASAP so we
can get UMA stats back on track.

[DaveMoore, 2011-11-14 17:39:50]

lgtm