Add Startup.BrowserView.FirstPaint / .CompositingEnded histograms.

components/startup_metric_utils/browser/startup_metric_utils.cc

Index: components/startup_metric_utils/browser/startup_metric_utils.cc
diff --git a/components/startup_metric_utils/browser/startup_metric_utils.cc b/components/startup_metric_utils/browser/startup_metric_utils.cc
index f1ad613d5df94bc95887beaafc255f963664c4c6..629d8ca0aa64106c21f8500bd761888080e83da2 100644
--- a/components/startup_metric_utils/browser/startup_metric_utils.cc
+++ b/components/startup_metric_utils/browser/startup_metric_utils.cc
@@ -39,7 +39,7 @@ namespace {
 
 // Mark as volatile to defensively make sure usage is thread-safe.
 // Note that at the time of this writing, access is only on the UI thread.
-volatile bool g_non_browser_ui_displayed = false;
+volatile bool g_main_window_startup_interrupted = false;
 
 base::LazyInstance<base::TimeTicks>::Leaky g_process_creation_ticks =
     LAZY_INSTANCE_INITIALIZER;
@@ -557,12 +557,16 @@ void RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(prefs::kSameVersionStartupCount, 0);
 }
 
-bool WasNonBrowserUIDisplayed() {
-  return g_non_browser_ui_displayed;
+bool WasMainWindowStartupInterrupted() {
+  return g_main_window_startup_interrupted;
 }
 
 void SetNonBrowserUIDisplayed() {
-  g_non_browser_ui_displayed = true;
+  g_main_window_startup_interrupted = true;
+}
+
+void SetBackgroundModeEnabled() {
+  g_main_window_startup_interrupted = true;
 }
 
 void RecordStartupProcessCreationTime(const base::Time& time) {
@@ -662,7 +666,8 @@ void RecordBrowserWindowDisplay(const base::TimeTicks& ticks) {
   if (!is_first_call || ticks.is_null())
     return;
   is_first_call = false;
-  if (WasNonBrowserUIDisplayed() || g_process_creation_ticks.Get().is_null())
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())

[Alexei Svitkine (slow), 2017/04/18 16:01:32]

Nit: Instead of checking for both conditions in a bunch of places, how about
making the helper function check the second thing too?

e.g. make the helper "ShouldLogProcessCreationTicksHistogram()" and have it
check both startup interrupted and presence of procession creation ticks. This
way, all of these can still be 1-liners.

[themblsha, 2017/04/19 15:06:09]

Good idea, thanks!

     return;
 
   UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
@@ -692,7 +697,8 @@ void RecordFirstWebContentsMainFrameLoad(const base::TimeTicks& ticks) {
   if (!is_first_call || ticks.is_null())
     return;
   is_first_call = false;
-  if (WasNonBrowserUIDisplayed() || g_process_creation_ticks.Get().is_null())
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
     return;
 
   UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE_AND_SAME_VERSION_COUNT(
@@ -710,7 +716,8 @@ void RecordFirstWebContentsNonEmptyPaint(const base::TimeTicks& ticks) {
   // entry time and the startup temperature are known.
   RecordRendererMainEntryHistogram();
 
-  if (WasNonBrowserUIDisplayed() || g_process_creation_ticks.Get().is_null())
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
     return;
 
   base::StackSamplingProfiler::SetProcessMilestone(
@@ -726,7 +733,8 @@ void RecordFirstWebContentsMainNavigationStart(const base::TimeTicks& ticks,
   if (!is_first_call || ticks.is_null())
     return;
   is_first_call = false;
-  if (WasNonBrowserUIDisplayed() || g_process_creation_ticks.Get().is_null())
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
     return;
 
   base::StackSamplingProfiler::SetProcessMilestone(
@@ -758,7 +766,8 @@ void RecordFirstWebContentsMainNavigationFinished(
   if (!is_first_call || ticks.is_null())
     return;
   is_first_call = false;
-  if (WasNonBrowserUIDisplayed() || g_process_creation_ticks.Get().is_null())
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
     return;

[gab, 2017/04/18 13:52:15]

Add {} per conditional becoming multi-lines (here and elsewhere in this file)

[themblsha, 2017/04/19 15:06:09]

Alexei had a better idea :-)

 
   base::StackSamplingProfiler::SetProcessMilestone(
@@ -769,6 +778,36 @@ void RecordFirstWebContentsMainNavigationFinished(
       g_process_creation_ticks.Get(), ticks);
 }
 
+void RecordBrowserWindowFirstPaint(const base::TimeTicks& ticks) {

[Alexei Svitkine (slow), 2017/04/18 16:01:32]

Nit: TimeTicks should be passed by value (see header comments).

Please update the other functions while you're here.

[themblsha, 2017/04/19 15:06:09]

Do you mean this? https://codereview.chromium.org/1425263003

It doesn't mention anything about passing by reference. Or am I missing
something?

[Alexei Svitkine (slow), 2017/04/19 15:26:19]

From base/time/time.h:

// All time classes are copyable, assignable, and occupy 64-bits per instance.
// As a result, prefer passing them by value:
//   void MyFunction(TimeDelta arg);
// If circumstances require, you may also pass by const reference:
//   void MyFunction(const TimeDelta& arg);  // Not preferred.
//

[themblsha, 2017/04/19 15:47:33]

Ahhar! Removed references from the whole file.

+  static bool is_first_call = true;
+  if (!is_first_call || ticks.is_null())
+    return;
+  is_first_call = false;
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
+    return;
+
+  UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE(
+      UMA_HISTOGRAM_LONG_TIMES_100, "Startup.BrowserWindow.FirstPaint",
+      g_process_creation_ticks.Get(), ticks);
+}
+
+void RecordBrowserWindowFirstPaintCompositingEnded(
+    const base::TimeTicks& ticks) {
+  static bool is_first_call = true;
+  if (!is_first_call || ticks.is_null())
+    return;
+  is_first_call = false;
+  if (WasMainWindowStartupInterrupted() ||
+      g_process_creation_ticks.Get().is_null())
+    return;
+
+  UMA_HISTOGRAM_AND_TRACE_WITH_TEMPERATURE(
+      UMA_HISTOGRAM_LONG_TIMES_100,
+      "Startup.BrowserWindow.FirstPaint.CompositingEnded",
+      g_process_creation_ticks.Get(), ticks);
+}
+
 base::TimeTicks MainEntryPointTicks() {
   return g_browser_main_entry_point_ticks.Get();
 }