Reimplement vsync latency UMA to be based on BeginImplFrame rather than BeginMainFrame

cc/scheduler/compositor_timing_history.cc

Index: cc/scheduler/compositor_timing_history.cc
diff --git a/cc/scheduler/compositor_timing_history.cc b/cc/scheduler/compositor_timing_history.cc
index 0e88a8af073c93f361fa0adcf08266e661f4a9c1..c16e76f3730bb46a87e0bb6875a4e33d148d6d6e 100644
--- a/cc/scheduler/compositor_timing_history.cc
+++ b/cc/scheduler/compositor_timing_history.cc
@@ -26,7 +26,7 @@ class CompositorTimingHistory::UMAReporter {
   virtual void AddDrawInterval(base::TimeDelta interval) = 0;
 
   // Latency measurements
-  virtual void AddBeginMainFrameLatency(base::TimeDelta delta) = 0;
+  virtual void AddBeginImplFrameLatency(base::TimeDelta delta) = 0;
   virtual void AddBeginMainFrameQueueDurationCriticalDuration(
       base::TimeDelta duration) = 0;
   virtual void AddBeginMainFrameQueueDurationNotCriticalDuration(
@@ -147,6 +147,11 @@ class RendererUMAReporter : public CompositorTimingHistory::UMAReporter {
                                              interval);
   }
 
+  void AddBeginImplFrameLatency(base::TimeDelta delta) override {
+    UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
+        "Scheduling.Renderer.VsyncToBeginImplFrameLatency", delta);

[brianderson, 2017/03/16 20:53:16]

Using the "Vsync" is true on Windows, but is not quite right on Android. Maybe
just call it BeginImplFrameLatency?

[stanisc, 2017/03/16 21:01:12]

ajuma@ suggested adding VSync in the previous review but I am OK with taking it
off. I'll change it before submitting.

isherman@, this will require histograms.xml to be changed again to reflect the
new name.

[Ilya Sherman, 2017/03/16 21:28:04]

Either name is ok with me.  Thanks.

[stanisc, 2017/03/16 23:17:41]

Done.

+  }
+
   void AddBeginMainFrameQueueDurationCriticalDuration(
       base::TimeDelta duration) override {
     UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
@@ -198,11 +203,6 @@ class RendererUMAReporter : public CompositorTimingHistory::UMAReporter {
     UMA_HISTOGRAM_CUSTOM_TIMES_VSYNC_ALIGNED(
         "Scheduling.Renderer.MainAndImplFrameTimeDelta", delta);
   }
-
-  void AddBeginMainFrameLatency(base::TimeDelta delta) override {
-    UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
-        "Scheduling.Renderer.VsyncToBeginMainFrameLatency", delta);
-  }
 };
 
 class BrowserUMAReporter : public CompositorTimingHistory::UMAReporter {
@@ -229,6 +229,11 @@ class BrowserUMAReporter : public CompositorTimingHistory::UMAReporter {
                                              interval);
   }
 
+  void AddBeginImplFrameLatency(base::TimeDelta delta) override {
+    UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
+        "Scheduling.Browser.VsyncToBeginImplFrameLatency", delta);
+  }
+
   void AddBeginMainFrameQueueDurationCriticalDuration(
       base::TimeDelta duration) override {
     UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
@@ -280,11 +285,6 @@ class BrowserUMAReporter : public CompositorTimingHistory::UMAReporter {
     UMA_HISTOGRAM_CUSTOM_TIMES_VSYNC_ALIGNED(
         "Scheduling.Browser.MainAndImplFrameTimeDelta", delta);
   }
-
-  void AddBeginMainFrameLatency(base::TimeDelta delta) override {
-    UMA_HISTOGRAM_CUSTOM_TIMES_DURATION(
-        "Scheduling.Browser.VsyncToBeginMainFrameLatency", delta);
-  }
 };
 
 class NullUMAReporter : public CompositorTimingHistory::UMAReporter {
@@ -295,6 +295,7 @@ class NullUMAReporter : public CompositorTimingHistory::UMAReporter {
   }
   void AddCommitInterval(base::TimeDelta interval) override {}
   void AddDrawInterval(base::TimeDelta interval) override {}
+  void AddBeginImplFrameLatency(base::TimeDelta delta) override {}
   void AddBeginMainFrameQueueDurationCriticalDuration(
       base::TimeDelta duration) override {}
   void AddBeginMainFrameQueueDurationNotCriticalDuration(
@@ -308,7 +309,6 @@ class NullUMAReporter : public CompositorTimingHistory::UMAReporter {
   void AddSubmitToAckLatency(base::TimeDelta duration) override {}
   void AddSubmitAckWasFast(bool was_fast) override {}
   void AddMainAndImplFrameTimeDelta(base::TimeDelta delta) override {}
-  void AddBeginMainFrameLatency(base::TimeDelta delta) override {}
 };
 
 }  // namespace
@@ -467,7 +467,10 @@ void CompositorTimingHistory::DidCreateAndInitializeCompositorFrameSink() {
 }
 
 void CompositorTimingHistory::WillBeginImplFrame(
-    bool new_active_tree_is_likely) {
+    bool new_active_tree_is_likely,
+    base::TimeTicks frame_time,
+    BeginFrameArgs::BeginFrameArgsType frame_type,
+    base::TimeTicks now) {
   // The check for whether a BeginMainFrame was sent anytime between two
   // BeginImplFrames protects us from not detecting a fast main thread that
   // does all it's work and goes idle in between BeginImplFrames.
@@ -480,7 +483,7 @@ void CompositorTimingHistory::WillBeginImplFrame(
   }
 
   if (submit_ack_watchdog_enabled_) {
-    base::TimeDelta submit_not_acked_time_ = Now() - submit_start_time_;
+    base::TimeDelta submit_not_acked_time_ = now - submit_start_time_;
     if (submit_not_acked_time_ >= kSubmitAckWatchdogTimeout) {
       uma_reporter_->AddSubmitAckWasFast(false);
       // Only record this UMA once per submitted CompositorFrame.
@@ -488,6 +491,9 @@ void CompositorTimingHistory::WillBeginImplFrame(
     }
   }
 
+  if (frame_type == BeginFrameArgs::NORMAL)
+    uma_reporter_->AddBeginImplFrameLatency(now - frame_time);
+
   did_send_begin_main_frame_ = false;
 }
 
@@ -504,8 +510,7 @@ void CompositorTimingHistory::BeginImplFrameNotExpectedSoon() {
 
 void CompositorTimingHistory::WillBeginMainFrame(
     bool on_critical_path,
-    base::TimeTicks main_frame_time,
-    BeginFrameArgs::BeginFrameArgsType frame_type) {
+    base::TimeTicks main_frame_time) {
   DCHECK_EQ(base::TimeTicks(), begin_main_frame_sent_time_);
   DCHECK_EQ(base::TimeTicks(), begin_main_frame_frame_time_);
 
@@ -513,11 +518,6 @@ void CompositorTimingHistory::WillBeginMainFrame(
   begin_main_frame_sent_time_ = Now();
   begin_main_frame_frame_time_ = main_frame_time;
 
-  if (frame_type == BeginFrameArgs::NORMAL) {
-    uma_reporter_->AddBeginMainFrameLatency(begin_main_frame_sent_time_ -
-                                            begin_main_frame_frame_time_);
-  }
-
   did_send_begin_main_frame_ = true;
   SetBeginMainFrameNeededContinuously(true);
 }