Add a histogram to measure browser window display

Add a histogram to measure browser window display

Measure the amount of time from app startup to when the first browser window becomes visible.

TEST=None
BUG=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=148917

[jeremy, 2012-06-26 12:26:27]

Tonyg: Review please
Sky: Owner review, thanks!

[sky, 2012-06-26 16:14:15]

https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/b...
File chrome/browser/ui/browser.cc (right):

https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/b...
chrome/browser/ui/browser.cc:1984: #if defined(OS_MACOSX) || defined(OS_WIN)
Aren't there a number of code paths that could result in showing non-browser
things for any length of time that end up skewing this number to be meaningless?

[playmobil, 2012-06-26 16:54:31]

You're probably right, but can you give an example?  First run, for example
doesn't do this (to the best of my knowledge).

Best regards,
Jeremy


On Tue, Jun 26, 2012 at 7:14 PM, <sky@chromium.org> wrote:

>
> https://chromiumcodereview.**appspot.com/10662046/diff/**
>
2001/chrome/browser/ui/**browser.cc<https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/browser.cc>
> File chrome/browser/ui/browser.cc (right):
>
> https://chromiumcodereview.**appspot.com/10662046/diff/**
>
2001/chrome/browser/ui/**browser.cc#newcode1984<https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/browser.cc#newcode1984>
> chrome/browser/ui/browser.cc:**1984: #if defined(OS_MACOSX) ||
> defined(OS_WIN)
> Aren't there a number of code paths that could result in showing
> non-browser things for any length of time that end up skewing this
> number to be meaningless?
>
>
https://chromiumcodereview.**appspot.com/10662046/<https://chromiumcodereview...
>

[sky, 2012-06-26 17:24:38]

I want to say that if there are problems reading your profile might cause
this. I'm pretty sure there is some other code that runs a nested message
loop in some scenarios during startup too.

  -Scott


On Tue, Jun 26, 2012 at 9:54 AM, Jeremy Moskovich <playmobil@google.com>wrote:

> You're probably right, but can you give an example?  First run, for
> example doesn't do this (to the best of my knowledge).
>
> Best regards,
> Jeremy
>
>
> On Tue, Jun 26, 2012 at 7:14 PM, <sky@chromium.org> wrote:
>
>>
>> https://chromiumcodereview.**appspot.com/10662046/diff/**
>>
2001/chrome/browser/ui/**browser.cc<https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/browser.cc>
>> File chrome/browser/ui/browser.cc (right):
>>
>> https://chromiumcodereview.**appspot.com/10662046/diff/**
>>
2001/chrome/browser/ui/**browser.cc#newcode1984<https://chromiumcodereview.appspot.com/10662046/diff/2001/chrome/browser/ui/browser.cc#newcode1984>
>> chrome/browser/ui/browser.cc:**1984: #if defined(OS_MACOSX) ||
>> defined(OS_WIN)
>> Aren't there a number of code paths that could result in showing
>> non-browser things for any length of time that end up skewing this
>> number to be meaningless?
>>
>>
https://chromiumcodereview.**appspot.com/10662046/<https://chromiumcodereview...
>>
>
>

[jeremy, 2012-07-25 08:41:06]

sky: Ready for another look - changed so that we don't collect metrics if UI is
displayed on startup.

[sky, 2012-07-25 15:51:48]

http://codereview.chromium.org/10662046/diff/9001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1212: static bool isFirstBrowserWindow = true;
is_first_browser_window

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
File chrome/common/startup_metric_utils.cc (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
chrome/common/startup_metric_utils.cc:9: bool* NonBrowserUIDisplayed() {
Why go through a function here? Move non_browser_ui_displayed outside the
function.

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
File chrome/common/startup_metric_utils.h (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
chrome/common/startup_metric_utils.h:10: class StartupMetricUtils {
No need to for a class here, use functions.

[jeremy, 2012-07-26 12:57:25]

Sky: Thanks for the review, all fixed and ready for another look...

http://codereview.chromium.org/10662046/diff/9001/chrome/browser/ui/browser.cc
File chrome/browser/ui/browser.cc (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/browser/ui/browser.c...
chrome/browser/ui/browser.cc:1212: static bool isFirstBrowserWindow = true;
On 2012/07/25 15:51:48, sky wrote:
> is_first_browser_window

Done.

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
File chrome/common/startup_metric_utils.cc (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
chrome/common/startup_metric_utils.cc:9: bool* NonBrowserUIDisplayed() {
On 2012/07/25 15:51:48, sky wrote:
> Why go through a function here? Move non_browser_ui_displayed outside the
> function.

Done.

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
File chrome/common/startup_metric_utils.h (right):

http://codereview.chromium.org/10662046/diff/9001/chrome/common/startup_metri...
chrome/common/startup_metric_utils.h:10: class StartupMetricUtils {
On 2012/07/25 15:51:48, sky wrote:
> No need to for a class here, use functions.

Done.

[sky, 2012-07-28 02:13:35]

LGTM

http://codereview.chromium.org/10662046/diff/13002/chrome/common/startup_metr...
File chrome/common/startup_metric_utils.cc (right):

http://codereview.chromium.org/10662046/diff/13002/chrome/common/startup_metr...
chrome/common/startup_metric_utils.cc:9: // Value may potentially be modified
and read on different threads.
Is this comment right? Isn't this only used on the UI thread?

[commit-bot: I haz the power, 2012-07-29 12:03:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jeremy@chromium.org/10662046/15007

[commit-bot: I haz the power, 2012-07-29 18:10:10]

Change committed as 148917