Delay the first watchdog histogram to the first watchdog timeout.

GPU.WatchdogThread.Event(kGpuWatchdogStart) was added too early (eg.
watchdog creation time) that it couldn't be persistent. This histogram data
was lost after crash or browser exit.

Now delay the recording of kGpuWatchdogStart until the first
OnWatchdogTimeout() to ensure this metric is created in persistent memory.

Bug: 988344, 949839
Change-Id: I8829d31209511d34220eb17a4f39017929d7e0a1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929867
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718462}

gpu/ipc/service/gpu_watchdog_thread.cc

@@ -10,6 +10,7 @@
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
 #include "base/message_loop/message_loop_current.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/power_monitor/power_monitor.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
@@ -81,7 +82,6 @@ GpuWatchdogThreadImplV1::GpuWatchdogThreadImplV1()
   host_tty_ = GetActiveTTY();
 #endif
   base::MessageLoopCurrent::Get()->AddTaskObserver(&task_observer_);
-  GpuWatchdogHistogram(GpuWatchdogThreadEvent::kGpuWatchdogStart);
 }
 
 // static
@@ -124,7 +124,7 @@ void GpuWatchdogThreadImplV1::OnForegrounded() {
 
 void GpuWatchdogThreadImplV1::GpuWatchdogHistogram(
     GpuWatchdogThreadEvent thread_event) {
-  UMA_HISTOGRAM_ENUMERATION("GPU.WatchdogThread.Event", thread_event);
+  base::UmaHistogramEnumeration("GPU.WatchdogThread.Event", thread_event);
 }
 
 bool GpuWatchdogThreadImplV1::IsGpuHangDetectedForTesting() {
@@ -330,6 +330,14 @@ void GpuWatchdogThreadImplV1::DeliberatelyTerminateToRecoverFromHang() {
   // Should not get here while the system is suspended.
   DCHECK(!suspension_counter_.HasRefs());
 
+  // If this metric is added too early (eg. watchdog creation time), it cannot
+  // be persistent. The histogram data will be lost after crash or browser exit.
+  // Delay the recording of kGpuWatchdogStart until the first OnCheckTimeout().
+  if (!is_watchdog_start_histogram_recorded) {
+    is_watchdog_start_histogram_recorded = true;
+    GpuWatchdogHistogram(GpuWatchdogThreadEvent::kGpuWatchdogStart);
+  }
+
   // If the watchdog woke up significantly behind schedule, disarm and reset
   // the watchdog check. This is to prevent the watchdog thread from terminating
   // when a machine wakes up from sleep or hibernation, which would otherwise

gpu/ipc/service/gpu_watchdog_thread.h

@@ -228,6 +228,9 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThreadImplV1
   base::Time check_time_;
   base::TimeTicks check_timeticks_;
 
+  // whether GpuWatchdogThreadEvent::kGpuWatchdogStart has been recorded.
+  bool is_watchdog_start_histogram_recorded = false;
+
 #if defined(USE_X11)
   FILE* tty_file_;
   int host_tty_;

gpu/ipc/service/gpu_watchdog_thread_v2.cc

@@ -178,8 +178,6 @@ void GpuWatchdogThreadImplV2::Init() {
     remaining_watched_thread_ticks_ = timeout;
   }
 #endif
-
-  GpuWatchdogHistogram(GpuWatchdogThreadEvent::kGpuWatchdogStart);
 }
 
 // Running on the watchdog thread.
@@ -355,6 +353,17 @@ void GpuWatchdogThreadImplV2::OnWatchdogTimeout() {
   DCHECK(!is_backgrounded_);
   DCHECK(!in_power_suspension_);
   DCHECK(!is_paused_);
+
+  // If this metric is added too early (eg. watchdog creation time), it cannot
+  // be persistent. The histogram data will be lost after crash or browser exit.
+  // Delay the recording of kGpuWatchdogStart until the firs
+  // OnWatchdogTimeout() to ensure this metric is created in the persistent
+  // memory.
+  if (!is_watchdog_start_histogram_recorded) {
+    is_watchdog_start_histogram_recorded = true;
+    GpuWatchdogHistogram(GpuWatchdogThreadEvent::kGpuWatchdogStart);
+  }
+
   base::subtle::Atomic32 arm_disarm_counter =
       base::subtle::NoBarrier_Load(&arm_disarm_counter_);
   GpuWatchdogTimeoutHistogram(GpuWatchdogTimeoutEvent::kTimeout);
@@ -553,17 +562,12 @@ void GpuWatchdogThreadImplV2::DeliberatelyTerminateToRecoverFromHang() {
 
 void GpuWatchdogThreadImplV2::GpuWatchdogHistogram(
     GpuWatchdogThreadEvent thread_event) {
-  UMA_HISTOGRAM_ENUMERATION("GPU.WatchdogThread.Event.V2", thread_event);
-  UMA_HISTOGRAM_ENUMERATION("GPU.WatchdogThread.Event", thread_event);
+  base::UmaHistogramEnumeration("GPU.WatchdogThread.Event.V2", thread_event);
+  base::UmaHistogramEnumeration("GPU.WatchdogThread.Event", thread_event);
 }
 
 void GpuWatchdogThreadImplV2::GpuWatchdogTimeoutHistogram(
     GpuWatchdogTimeoutEvent timeout_event) {
-  // The histogram needs to be allocated in the persistent memory, or the data
-  // will be lost after crash.
-  if (!is_test_mode_)
-    DCHECK(base::GlobalHistogramAllocator::Get());
-
   if (in_gpu_initialization_) {
     base::UmaHistogramEnumeration("GPU.WatchdogThread.Timeout.Init",
                                   timeout_event);

gpu/ipc/service/gpu_watchdog_thread_v2.h

@@ -178,6 +178,9 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThreadImplV2
   bool is_add_power_observer_called_ = false;
   bool is_power_observer_added_ = false;
 
+  // whether GpuWatchdogThreadEvent::kGpuWatchdogStart has been recorded.
+  bool is_watchdog_start_histogram_recorded = false;
+
   // For the experiment and the debugging purpose
   bool in_gpu_initialization_ = false;
   int num_of_timeout_after_power_resume_ = 0;