[cc/metrics] Fix canvas and JSAnimation tracker not reporting UMA

Currently the canvas and JSAnimation tracker isn't reporting to UMA. The problem is that after we set HasCanvasInvalidation and HasJSAnimation on the main thread, we didn't push to the impl thread, and thus there is no tracker started. This CL fixes the problem, also added to UKM so that we will also receive data for UKM. The original doc for collection UKM is here: https://docs.google.com/document/d/1SDxdua997Gm7ihJFVjinHz9qTdBzTJrEzC7GEeQHs-8/edit#heading=h.k5jx6iluw4yt Bug: 1141901 Change-Id: I0e1b0aaad8e35e2e8ab77356a81487782ad9ebde Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2497464 Reviewed-by: Jesse Doherty <jwd@chromium.org> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#824612}
Denger-Network · Nov 5, 2020 · 9d689d8 · 9d689d8
1 parent 44f4eb8
commit 9d689d8
Show file tree

Hide file tree

Showing 10 changed files with 62 additions and 14 deletions.
diff --git a/cc/animation/animation_host.cc b/cc/animation/animation_host.cc
@@ -258,6 +258,8 @@ void AnimationHost::PushPropertiesTo(MutatorHost* mutator_host_impl) {
   host_impl->main_thread_animations_count_ = main_thread_animations_count_;
   host_impl->current_frame_had_raf_ = current_frame_had_raf_;
   host_impl->next_frame_has_pending_raf_ = next_frame_has_pending_raf_;
+  host_impl->has_canvas_invalidation_ = has_canvas_invalidation_;
+  host_impl->has_inline_style_mutation_ = has_inline_style_mutation_;
 
   if (needs_push_properties_) {
     needs_push_properties_ = false;

diff --git a/cc/animation/animation_host_unittest.cc b/cc/animation/animation_host_unittest.cc
@@ -412,6 +412,23 @@ TEST_F(AnimationHostTest, TickScrollLinkedAnimation) {
                                           property_trees.scroll_tree, false));
 }
 
+TEST_F(AnimationHostTest, PushPropertiesToImpl) {
+  std::unique_ptr<AnimationHost> host(
+      AnimationHost::CreateForTesting(ThreadInstance::MAIN));
+  std::unique_ptr<AnimationHost> host_impl(
+      AnimationHost::CreateForTesting(ThreadInstance::IMPL));
+
+  host->SetHasCanvasInvalidation(true);
+  host->SetHasInlineStyleMutation(true);
+
+  EXPECT_FALSE(host_impl->HasCanvasInvalidation());
+  EXPECT_FALSE(host_impl->HasJSAnimation());
+
+  host->PushPropertiesTo(host_impl.get());
+  EXPECT_TRUE(host_impl->HasCanvasInvalidation());
+  EXPECT_TRUE(host_impl->HasJSAnimation());
+}
+
 TEST_F(AnimationHostTest, ScrollTimelineOffsetUpdatedByScrollAnimation) {
   client_.RegisterElementId(element_id_, ElementListType::ACTIVE);
   client_impl_.RegisterElementId(element_id_, ElementListType::PENDING);

diff --git a/cc/metrics/compositor_frame_reporter.cc b/cc/metrics/compositor_frame_reporter.cc
@@ -568,8 +568,8 @@ void CompositorFrameReporter::ReportCompositorLatencyHistograms() const {
           UMA_HISTOGRAM_ENUMERATION("CompositorLatency.Type.ScrollbarScroll",
                                     report_type);
           break;
-        case FrameSequenceTrackerType::kCanvas:
-          UMA_HISTOGRAM_ENUMERATION("CompositorLatency.Type.Canvas",
+        case FrameSequenceTrackerType::kCanvasAnimation:
+          UMA_HISTOGRAM_ENUMERATION("CompositorLatency.Type.CanvasAnimation",
                                     report_type);
           break;
         case FrameSequenceTrackerType::kJSAnimation:

diff --git a/cc/metrics/frame_sequence_metrics.cc b/cc/metrics/frame_sequence_metrics.cc
@@ -150,7 +150,7 @@ FrameSequenceMetrics::ThreadType FrameSequenceMetrics::GetEffectiveThread()
 
     case FrameSequenceTrackerType::kMainThreadAnimation:
     case FrameSequenceTrackerType::kRAF:
-    case FrameSequenceTrackerType::kCanvas:
+    case FrameSequenceTrackerType::kCanvasAnimation:
     case FrameSequenceTrackerType::kJSAnimation:
       return ThreadType::kMain;
 
@@ -257,7 +257,8 @@ void FrameSequenceMetrics::ReportMetrics() {
                               type_),
             impl_throughput_);
   }
-  if (main_report) {
+  if (main_report || type_ == FrameSequenceTrackerType::kCanvasAnimation ||
+      type_ == FrameSequenceTrackerType::kJSAnimation) {
     main_throughput_percent_dropped =
         ThroughputData::ReportDroppedFramePercentHistogram(
             this, ThreadType::kMain,
@@ -421,7 +422,9 @@ int FrameSequenceMetrics::ThroughputData::ReportDroppedFramePercentHistogram(
     const ThroughputData& data) {
   const auto sequence_type = metrics->type();
   DCHECK_LT(sequence_type, FrameSequenceTrackerType::kMaxType);
-  DCHECK(CanReportHistogram(metrics, thread_type, data));
+  DCHECK(CanReportHistogram(metrics, thread_type, data) ||
+         sequence_type == FrameSequenceTrackerType::kCanvasAnimation ||
+         sequence_type == FrameSequenceTrackerType::kJSAnimation);
 
   if (metrics->GetEffectiveThread() == thread_type) {
     STATIC_HISTOGRAM_POINTER_GROUP(

diff --git a/cc/metrics/frame_sequence_metrics.h b/cc/metrics/frame_sequence_metrics.h
@@ -29,7 +29,7 @@ enum class FrameSequenceTrackerType {
   kScrollbarScroll = 8,
   kCustom = 9,  // Note that the metrics for kCustom are not reported on UMA,
                 // and instead are dispatched back to the LayerTreeHostClient.
-  kCanvas = 10,
+  kCanvasAnimation = 10,
   kJSAnimation = 11,
   kMaxType
 };

diff --git a/cc/metrics/frame_sequence_tracker.cc b/cc/metrics/frame_sequence_tracker.cc
@@ -60,8 +60,8 @@ const char* FrameSequenceTracker::GetFrameSequenceTrackerTypeName(
       return "ScrollbarScroll";
     case FrameSequenceTrackerType::kCustom:
       return "Custom";
-    case FrameSequenceTrackerType::kCanvas:
-      return "Canvas";
+    case FrameSequenceTrackerType::kCanvasAnimation:
+      return "CanvasAnimation";
     case FrameSequenceTrackerType::kJSAnimation:
       return "JSAnimation";
     case FrameSequenceTrackerType::kMaxType:

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -613,7 +613,7 @@ void LayerTreeHostImpl::CommitComplete() {
   if (mutator_host_->CurrentFrameHadRAF())
     frame_trackers_.StartSequence(FrameSequenceTrackerType::kRAF);
   if (mutator_host_->HasCanvasInvalidation())
-    frame_trackers_.StartSequence(FrameSequenceTrackerType::kCanvas);
+    frame_trackers_.StartSequence(FrameSequenceTrackerType::kCanvasAnimation);
   if (mutator_host_->HasJSAnimation())
     frame_trackers_.StartSequence(FrameSequenceTrackerType::kJSAnimation);
 
@@ -2358,7 +2358,7 @@ bool LayerTreeHostImpl::DrawLayers(FrameData* frame) {
   if (!mutator_host_->NextFrameHasPendingRAF())
     frame_trackers_.StopSequence(FrameSequenceTrackerType::kRAF);
   if (!mutator_host_->HasCanvasInvalidation())
-    frame_trackers_.StopSequence(FrameSequenceTrackerType::kCanvas);
+    frame_trackers_.StopSequence(FrameSequenceTrackerType::kCanvasAnimation);
   if (!mutator_host_->HasJSAnimation())
     frame_trackers_.StopSequence(FrameSequenceTrackerType::kJSAnimation);
 

diff --git a/cc/trees/ukm_manager.cc b/cc/trees/ukm_manager.cc
@@ -116,6 +116,8 @@ void UkmManager::RecordThroughputUKM(
         CASE_FOR_MAIN_THREAD_TRACKER(TouchScroll);
         CASE_FOR_MAIN_THREAD_TRACKER(Video);
         CASE_FOR_MAIN_THREAD_TRACKER(WheelScroll);
+        CASE_FOR_MAIN_THREAD_TRACKER(CanvasAnimation);
+        CASE_FOR_MAIN_THREAD_TRACKER(JSAnimation);
 #undef CASE_FOR_MAIN_THREAD_TRACKER
         default:
           NOTREACHED();
@@ -261,6 +263,8 @@ void UkmManager::RecordCompositorLatencyUKM(
       CASE_FOR_TRACKER(TouchScroll);
       CASE_FOR_TRACKER(Video);
       CASE_FOR_TRACKER(WheelScroll);
+      CASE_FOR_TRACKER(CanvasAnimation);
+      CASE_FOR_TRACKER(JSAnimation);
 #undef CASE_FOR_TRACKER
       default:
         NOTREACHED();

diff --git a/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml b/tools/metrics/histograms/histograms_xml/histogram_suffixes_list.xml
@@ -2874,7 +2874,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <affected-histogram name="CompositorLatency"/>
   <affected-histogram name="CompositorLatency.CompositorAnimation"/>
   <affected-histogram name="CompositorLatency.DroppedFrame"/>
-  <affected-histogram name="CompositorLatency.DroppedFrame.Canvas"/>
+  <affected-histogram name="CompositorLatency.DroppedFrame.CanvasAnimation"/>
   <affected-histogram
       name="CompositorLatency.DroppedFrame.CompositorAnimation"/>
   <affected-histogram name="CompositorLatency.DroppedFrame.JSAnimation"/>
@@ -2887,7 +2887,8 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <affected-histogram name="CompositorLatency.DroppedFrame.WheelScroll"/>
   <affected-histogram name="CompositorLatency.MainThreadAnimation"/>
   <affected-histogram name="CompositorLatency.MissedDeadlineFrame"/>
-  <affected-histogram name="CompositorLatency.MissedDeadlineFrame.Canvas"/>
+  <affected-histogram
+      name="CompositorLatency.MissedDeadlineFrame.CanvasAnimation"/>
   <affected-histogram
       name="CompositorLatency.MissedDeadlineFrame.CompositorAnimation"/>
   <affected-histogram name="CompositorLatency.MissedDeadlineFrame.JSAnimation"/>
@@ -3153,7 +3154,8 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
       label="The total time starting from BeginImplFrame to when
              CompositorFramePresentation is done."/>
   <affected-histogram name="CompositorLatency.CompositorOnlyFrame"/>
-  <affected-histogram name="CompositorLatency.CompositorOnlyFrame.Canvas"/>
+  <affected-histogram
+      name="CompositorLatency.CompositorOnlyFrame.CanvasAnimation"/>
   <affected-histogram
       name="CompositorLatency.CompositorOnlyFrame.CompositorAnimation"/>
   <affected-histogram name="CompositorLatency.CompositorOnlyFrame.JSAnimation"/>
@@ -17538,7 +17540,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 </histogram_suffixes>
 
 <histogram_suffixes name="SmoothnessSequenceTypes" separator=".">
-  <suffix name="Canvas" label="Main-thread canvas"/>
+  <suffix name="CanvasAnimation" label="Main-thread canvas animation"/>
   <suffix name="CompositorAnimation" label="Compositor-driven animation"/>
   <suffix name="JSAnimation" label="JS-driven animation"/>
   <suffix name="MainThreadAnimation" label="Main-thread driven animation"/>

diff --git a/tools/metrics/ukm/ukm.xml b/tools/metrics/ukm/ukm.xml
@@ -4548,6 +4548,11 @@ be describing additional metrics about the same event.
       </history>
     </aggregation>
   </metric>
+  <metric name="CanvasAnimation" enum="Boolean">
+    <summary>
+      True when a canvas animation was active this frame.
+    </summary>
+  </metric>
   <metric name="Commit">
     <summary>
       The duration of the commit stage.&quot;
@@ -4594,6 +4599,11 @@ be describing additional metrics about the same event.
       </history>
     </aggregation>
   </metric>
+  <metric name="JSAnimation" enum="Boolean">
+    <summary>
+      True when a inline style animation was active this frame.
+    </summary>
+  </metric>
   <metric name="MainThreadAnimation" enum="Boolean">
     <summary>
       True when a main-thread-driven animation was active this frame.
@@ -4973,11 +4983,21 @@ be describing additional metrics about the same event.
       interactions.
     </summary>
   </metric>
+  <metric name="MainThread.CanvasAnimation">
+    <summary>
+      The throughput of the main thread during canvas animation.
+    </summary>
+  </metric>
   <metric name="MainThread.CompositorAnimation">
     <summary>
       The throughput of the main thread during a compositor animation.
     </summary>
   </metric>
+  <metric name="MainThread.JSAnimation">
+    <summary>
+      The throughput of the main thread during inline style animation.
+    </summary>
+  </metric>
   <metric name="MainThread.MainThreadAnimation">
     <summary>
       The throughput of the main thread during a main thread animation.