Record BrowserOpenTabs and BrowserWindowDisplay histograms after startup temperature evaluation.

Previously, these histograms were recorded prior to startup
temperature evaluation, which caused almost (*) no data to be
recorded in temperature-suffixed histograms. With this CL,
the values are stored in global variable and recorded to
appropriate histograms after the startup temperature is
evaluated.

(*) Most of the time, the browser window is displayed
    before startup temperature evaluation. However, if
    Chrome is launched with --silent-launch, the first
    window can be displayed later. That may explain why
    we see a few samples in temperature-suffixed
    BrowserWindowDisplay histograms. With this CL, any
    browser window display that happens after normal
    startup won't be recorded to an histogram.

Bug: 580207, 589118
Change-Id: I8419df6f42df64f60be0858462159a276d17557e
Reviewed-on: https://chromium-review.googlesource.com/1167470Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582302}

chrome/browser/metrics/startup_metrics_browsertest.cc

@@ -7,7 +7,10 @@
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/run_loop.h"
+#include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "components/startup_metric_utils/browser/startup_metric_utils.h"
 
 using StartupMetricsTest = InProcessBrowserTest;
 
@@ -15,6 +18,8 @@ namespace {
 
 constexpr const char* kStartupMetrics[] = {
     "Startup.BrowserMainToRendererMain",
+    "Startup.BrowserMessageLoopStartTime",
+    "Startup.BrowserMessageLoopStartTimeFromMainEntry3",
     "Startup.BrowserOpenTabs",
     "Startup.BrowserWindow.FirstPaint",
     "Startup.BrowserWindow.FirstPaint.CompositingEnded",
@@ -24,24 +29,31 @@ constexpr const char* kStartupMetrics[] = {
     "Startup.FirstWebContents.MainNavigationStart",
     "Startup.FirstWebContents.NonEmptyPaint2",
     "Startup.FirstWebContents.RenderProcessHostInit.ToNonEmptyPaint",
-
-    // The following histograms depend on normal browser startup through
-    // BrowserMain and are as such not caught by this browser test.
-    // "Startup.BrowserMessageLoopStartHardFaultCount",
-    // "Startup.BrowserMessageLoopStartTime",
-    // "Startup.BrowserMessageLoopStartTimeFromMainEntry2",
-    // "Startup.BrowserMessageLoopStartTimeFromMainEntry3",
-    // "Startup.LoadTime.ExeMainToDllMain2",
-    // "Startup.LoadTime.ProcessCreateToDllMain2",
-    // "Startup.LoadTime.ProcessCreateToExeMain2",
-    // "Startup.SystemUptime",
-    // "Startup.Temperature",
+    "Startup.LoadTime.ExeMainToDllMain2",
+    "Startup.LoadTime.ProcessCreateToDllMain2",
+    "Startup.LoadTime.ProcessCreateToExeMain2",
+    "Startup.SystemUptime",
+
+#if defined(OS_WIN)
+    "Startup.BrowserMessageLoopStartHardFaultCount",
+    "Startup.Temperature",
+#endif
 };
 
 }  // namespace
 
 // Verify that startup histograms are logged on browser startup.
 IN_PROC_BROWSER_TEST_F(StartupMetricsTest, ReportsValues) {
+  // This is usually done from the constructor of ChromeMainDelegate.
+  startup_metric_utils::RecordExeMainEntryPointTicks(base::TimeTicks::Now());
+
+  // This is usually done from ChromeBrowserMainParts::MainMessageLoopRun().
+  startup_metric_utils::RecordBrowserMainMessageLoopStart(
+      base::TimeTicks::Now(), false /* is_first_run */,
+      g_browser_process->local_state());
+
+  // Wait for all histograms to be recorded. The test will hang if an histogram
+  // is not recorded.
   for (auto* const histogram : kStartupMetrics) {
     while (!base::StatisticsRecorder::FindHistogram(histogram))
       base::RunLoop().RunUntilIdle();

components/startup_metric_utils/browser/startup_metric_utils.cc

@@ -52,6 +52,10 @@ base::TimeTicks g_browser_exe_main_entry_point_ticks;
 
 base::TimeTicks g_message_loop_start_ticks;
 
+base::TimeTicks g_browser_window_display_ticks;
+
+base::TimeDelta g_browser_open_tabs_duration = base::TimeDelta::Max();
+
 // An enumeration of startup temperatures. This must be kept in sync with the
 // UMA StartupType enumeration defined in histograms.xml.
 enum StartupTemperature {
@@ -602,6 +606,21 @@ void RecordBrowserMainMessageLoopStart(base::TimeTicks ticks,
   RecordSystemUptimeHistogram();
   RecordTimeOfDayGMTHistogram();
 
+  // Record values stored prior to startup temperature evaluation.
+  if (ShouldLogStartupHistogram()) {
+    if (!g_browser_open_tabs_duration.is_max()) {
+      UMA_HISTOGRAM_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
+          UMA_HISTOGRAM_LONG_TIMES_100, "Startup.BrowserOpenTabs",
+          g_browser_open_tabs_duration);
+    }
+
+    if (!g_browser_window_display_ticks.is_null()) {
+      UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
+          UMA_HISTOGRAM_LONG_TIMES, "Startup.BrowserWindowDisplay",
+          g_process_creation_ticks, g_browser_window_display_ticks);
+    }
+  }
+
   // Record timings between process creation, the main() in the executable being
   // reached and the main() in the shared library being reached.
   if (!g_process_creation_ticks.is_null() &&
@@ -625,26 +644,27 @@ void RecordBrowserMainMessageLoopStart(base::TimeTicks ticks,
 }
 
 void RecordBrowserWindowDisplay(base::TimeTicks ticks) {
-  static bool is_first_call = true;
-  if (!is_first_call || ticks.is_null())
-    return;
-  is_first_call = false;
-  if (!ShouldLogStartupHistogram())
+  DCHECK(!ticks.is_null());
+
+  if (!g_browser_window_display_ticks.is_null())
     return;
 
-  UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
-      UMA_HISTOGRAM_LONG_TIMES, "Startup.BrowserWindowDisplay",
-      g_process_creation_ticks, ticks);
+  // The value will be recorded in appropriate histograms after the startup
+  // temperature is evaluated.
+  //
+  // Note: In some cases (e.g. launching with --silent-launch), the first
+  // browser window is displayed after the startup temperature is evaluated. In
+  // these cases, the value will not be recorded, which is the desired behavior
+  // for a non-conventional launch.
+  g_browser_window_display_ticks = ticks;
 }
 
 void RecordBrowserOpenTabsDelta(base::TimeDelta delta) {
-  static bool is_first_call = true;
-  if (!is_first_call)
-    return;
-  is_first_call = false;
-
-  UMA_HISTOGRAM_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
-      UMA_HISTOGRAM_LONG_TIMES_100, "Startup.BrowserOpenTabs", delta);
+  DCHECK(g_browser_open_tabs_duration.is_max());
+  DCHECK_EQ(g_startup_temperature, UNDETERMINED_STARTUP_TEMPERATURE);
+  // The value will be recorded in appropriate histograms after the startup
+  // temperature is evaluated.
+  g_browser_open_tabs_duration = delta;
 }
 
 void RecordRendererMainEntryTime(base::TimeTicks ticks) {