[cc/metrics] Create main-thread animation tracker for SVG animations

Right now the svg animation isn't tracked by throughput metrics. This CL addresses that. In particular, we pass a bit from Blink to cc::AnimationHost to track whether or not there is active svg animation. This doc contains some notes: https://docs.google.com/document/d/1cUGU36XZ0iWC0hK5EjHHgl1KMuZGselLfEo2wxDbxOg/edit Bug: 1200924 Change-Id: I4257e7d5767bbb76fd60ad96e38bbe7322067445 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2863204 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Xida Chen <xidachen@chromium.org> Cr-Commit-Position: refs/heads/master@{#879658}
chromium · May 6, 2021 · 0950b5b · 0950b5b
1 parent 6120982
commit 0950b5b
Show file tree

Hide file tree

Showing 16 changed files with 95 additions and 34 deletions.
diff --git a/cc/animation/animation_host.cc b/cc/animation/animation_host.cc
@@ -152,6 +152,14 @@ void AnimationHost::SetHasInlineStyleMutation(bool has_inline_style_mutation) {
   has_inline_style_mutation_ = has_inline_style_mutation;
 }
 
+bool AnimationHost::HasSmilAnimation() const {
+  return has_smil_animation_;
+}
+
+void AnimationHost::SetHasSmilAnimation(bool has_smil_animation) {
+  has_smil_animation_ = has_smil_animation;
+}
+
 void AnimationHost::UpdateRegisteredElementIds(ElementListType changed_list) {
   for (auto map_entry : element_to_animations_map_) {
     // kReservedElementId is reserved for a paint worklet element that animates
@@ -268,6 +276,7 @@ void AnimationHost::PushPropertiesTo(MutatorHost* mutator_host_impl) {
   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_;
+  host_impl->has_smil_animation_ = has_smil_animation_;
 
   if (needs_push_properties_) {
     needs_push_properties_ = false;
@@ -776,10 +785,9 @@ void AnimationHost::SetMutationUpdate(
   }
 }
 
-void AnimationHost::SetAnimationCounts(
-    size_t total_animations_count,
-    bool current_frame_had_raf,
-    bool next_frame_has_pending_raf) {
+void AnimationHost::SetAnimationCounts(size_t total_animations_count,
+                                       bool current_frame_had_raf,
+                                       bool next_frame_has_pending_raf) {
   // Though these changes are pushed as part of AnimationHost::PushPropertiesTo
   // we don't SetNeedsPushProperties as pushing the values requires a commit.
   // Instead we allow them to be pushed whenever the next required commit

diff --git a/cc/animation/animation_host.h b/cc/animation/animation_host.h
@@ -209,6 +209,7 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
   PendingThroughputTrackerInfos TakePendingThroughputTrackerInfos() override;
   bool HasCanvasInvalidation() const override;
   bool HasJSAnimation() const override;
+  bool HasSmilAnimation() const override;
 
   // Starts/stops throughput tracking represented by |sequence_id|.
   void StartThroughputTracking(TrackedAnimationSequenceId sequence_id);
@@ -220,6 +221,7 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
 
   void SetHasCanvasInvalidation(bool has_canvas_invalidation);
   void SetHasInlineStyleMutation(bool has_inline_style_mutation);
+  void SetHasSmilAnimation(bool has_svg_smil_animation);
 
  private:
   explicit AnimationHost(ThreadInstance thread_instance);
@@ -273,6 +275,7 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
   bool next_frame_has_pending_raf_ = false;
   bool has_canvas_invalidation_ = false;
   bool has_inline_style_mutation_ = false;
+  bool has_smil_animation_ = false;
 
   PendingThroughputTrackerInfos pending_throughput_tracker_infos_;
 

diff --git a/cc/layers/layer_unittest.cc b/cc/layers/layer_unittest.cc
@@ -1565,10 +1565,13 @@ TEST_F(LayerTest, PushAnimationCountsLazily) {
   EXPECT_CALL(*layer_tree_host_, SetNeedsCommit()).Times(0);
   animation_host_->SetAnimationCounts(0, /* current_frame_had_raf = */ true,
                                       /* next_frame_has_pending_raf = */ true);
+  animation_host_->SetHasSmilAnimation(true);
   EXPECT_FALSE(host_impl_.animation_host()->CurrentFrameHadRAF());
+  EXPECT_FALSE(host_impl_.animation_host()->HasSmilAnimation());
   EXPECT_FALSE(animation_host_->needs_push_properties());
   animation_host_->PushPropertiesTo(host_impl_.animation_host());
   EXPECT_TRUE(host_impl_.animation_host()->CurrentFrameHadRAF());
+  EXPECT_TRUE(host_impl_.animation_host()->HasSmilAnimation());
 }
 
 TEST_F(LayerTest, SetElementIdNotUsingLayerLists) {

diff --git a/cc/test/mock_mutator_host.h b/cc/test/mock_mutator_host.h
@@ -95,6 +95,7 @@ class MockMutatorHost : public MutatorHost {
   MOCK_CONST_METHOD0(MainThreadAnimationsCount, size_t());
   MOCK_CONST_METHOD0(HasCustomPropertyAnimations, bool());
   MOCK_CONST_METHOD0(CurrentFrameHadRAF, bool());
+  MOCK_CONST_METHOD0(HasSmilAnimation, bool());
   MOCK_CONST_METHOD0(NextFrameHasPendingRAF, bool());
   MOCK_METHOD0(TakePendingThroughputTrackerInfos,
                PendingThroughputTrackerInfos());

diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
@@ -638,7 +638,8 @@ void LayerTreeHostImpl::CommitComplete() {
   if (mutator_host_->HasJSAnimation())
     frame_trackers_.StartSequence(FrameSequenceTrackerType::kJSAnimation);
 
-  if (mutator_host_->MainThreadAnimationsCount() > 0) {
+  if (mutator_host_->MainThreadAnimationsCount() > 0 ||
+      mutator_host_->HasSmilAnimation()) {
     frame_trackers_.StartSequence(
         FrameSequenceTrackerType::kMainThreadAnimation);
   }
@@ -2402,7 +2403,8 @@ bool LayerTreeHostImpl::DrawLayers(FrameData* frame) {
   if (!mutator_host_->HasJSAnimation())
     frame_trackers_.StopSequence(FrameSequenceTrackerType::kJSAnimation);
 
-  if (mutator_host_->MainThreadAnimationsCount() == 0) {
+  if (mutator_host_->MainThreadAnimationsCount() == 0 &&
+      !mutator_host_->HasSmilAnimation()) {
     frame_trackers_.StopSequence(
         FrameSequenceTrackerType::kMainThreadAnimation);
   }
@@ -2528,6 +2530,7 @@ viz::CompositorFrame LayerTreeHostImpl::GenerateCompositorFrame(
   if (enable_frame_rate_throttling_) {
     metadata.preferred_frame_interval = viz::BeginFrameArgs::MaxInterval();
   } else if (mutator_host_->MainThreadAnimationsCount() == 0 &&
+             !mutator_host_->HasSmilAnimation() &&
              mutator_host_->MinimumTickInterval() > kMinDelta) {
     // All animations are impl-thread animations that tick at no more than
     // half the default display compositing fps.

diff --git a/cc/trees/mutator_host.h b/cc/trees/mutator_host.h
@@ -158,6 +158,7 @@ class MutatorHost {
   virtual bool NextFrameHasPendingRAF() const = 0;
   virtual bool HasCanvasInvalidation() const = 0;
   virtual bool HasJSAnimation() const = 0;
+  virtual bool HasSmilAnimation() const = 0;
 
   // Iterates through all animations and returns the minimum tick interval.
   // Returns 0 if there is a continuous animation which should be ticked

diff --git a/third_party/blink/renderer/core/animation/compositor_animations_test.cc b/third_party/blink/renderer/core/animation/compositor_animations_test.cc
@@ -2008,6 +2008,15 @@ TEST_P(AnimationCompositorAnimationsTest, TrackRafAnimationTimeout) {
   EXPECT_FALSE(host->NextFrameHasPendingRAF());
 }
 
+TEST_P(AnimationCompositorAnimationsTest, TrackSVGAnimation) {
+  LoadTestData("svg-smil-animation.html");
+
+  cc::AnimationHost* host = GetFrame()->View()->GetCompositorAnimationHost();
+
+  BeginFrame();
+  EXPECT_TRUE(host->HasSmilAnimation());
+}
+
 TEST_P(AnimationCompositorAnimationsTest, TrackRafAnimationNoneRegistered) {
   SetBodyInnerHTML("<div id='box'></div>");
 

diff --git a/third_party/blink/renderer/core/animation/test_data/svg-smil-animation.html b/third_party/blink/renderer/core/animation/test_data/svg-smil-animation.html
@@ -0,0 +1,5 @@
+<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
+  <rect width="10" height="10">
+    <animate attributeName="rx" values="0;5;0" dur="1s"/>
+  </rect>
+</svg>
diff --git a/third_party/blink/renderer/core/frame/local_frame_view.cc b/third_party/blink/renderer/core/frame/local_frame_view.cc
@@ -2768,14 +2768,10 @@ void LocalFrameView::RunPaintLifecyclePhase(PaintBenchmarkMode benchmark_mode) {
         next_frame_has_pending_raf |= document.NextFrameHasPendingRAF();
       });
 
-  if (GetLayoutView()->GetDocument().View() &&
-      GetLayoutView()->GetDocument().View()->GetCompositorAnimationHost()) {
-    GetLayoutView()
-        ->GetDocument()
-        .View()
-        ->GetCompositorAnimationHost()
-        ->SetAnimationCounts(total_animations_count, current_frame_had_raf,
-                             next_frame_has_pending_raf);
+  if (auto* animation_host = GetCompositorAnimationHost()) {
+    animation_host->SetAnimationCounts(total_animations_count,
+                                       current_frame_had_raf,
+                                       next_frame_has_pending_raf);
   }
 
   // Initialize animation properties in the newly created paint property
@@ -3658,7 +3654,9 @@ void LocalFrameView::ServiceScriptedAnimations(base::TimeTicks start_time) {
   GetFrame().AnimateSnapFling(start_time);
   Document* document = GetFrame().GetDocument();
   DCHECK(document);
-  SVGDocumentExtensions::ServiceOnAnimationFrame(*document);
+  if (SVGDocumentExtensions::ServiceSmilOnAnimationFrame(*document))
+    GetPage()->Animator().SetHasSmilAnimation();
+  SVGDocumentExtensions::ServiceWebAnimationsOnAnimationFrame(*document);
   document->GetDocumentAnimations().UpdateAnimationTimingForAnimationFrame();
   document->ServiceScriptedAnimations(start_time);
 }

diff --git a/third_party/blink/renderer/core/page/page_animator.cc b/third_party/blink/renderer/core/page/page_animator.cc
@@ -105,9 +105,11 @@ void PageAnimator::ReportFrameAnimations(cc::AnimationHost* animation_host) {
   if (animation_host) {
     animation_host->SetHasCanvasInvalidation(has_canvas_invalidation_);
     animation_host->SetHasInlineStyleMutation(has_inline_style_mutation_);
+    animation_host->SetHasSmilAnimation(has_smil_animation_);
   }
   has_canvas_invalidation_ = false;
   has_inline_style_mutation_ = false;
+  has_smil_animation_ = false;
 }
 
 void PageAnimator::SetSuppressFrameRequestsWorkaroundFor704763Only(
@@ -123,6 +125,10 @@ void PageAnimator::SetHasInlineStyleMutation() {
   has_inline_style_mutation_ = true;
 }
 
+void PageAnimator::SetHasSmilAnimation() {
+  has_smil_animation_ = true;
+}
+
 DISABLE_CFI_PERF
 void PageAnimator::ScheduleVisualUpdate(LocalFrame* frame) {
   if (servicing_animations_ || updating_layout_and_style_for_painting_ ||

diff --git a/third_party/blink/renderer/core/page/page_animator.h b/third_party/blink/renderer/core/page/page_animator.h
@@ -57,6 +57,7 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> {
   bool has_inline_style_mutation_for_test() const {
     return has_inline_style_mutation_;
   }
+  void SetHasSmilAnimation();
   void ReportFrameAnimations(cc::AnimationHost* animation_host);
 
  private:
@@ -70,6 +71,8 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> {
   bool has_inline_style_mutation_ = false;
   // True if the current main frame has canvas invalidation.
   bool has_canvas_invalidation_ = false;
+  // True if the current main frame has svg smil animation.
+  bool has_smil_animation_ = false;
 };
 
 }  // namespace blink

diff --git a/third_party/blink/renderer/core/svg/animation/smil_time_container.cc b/third_party/blink/renderer/core/svg/animation/smil_time_container.cc
@@ -447,47 +447,48 @@ void SMILTimeContainer::ServiceOnNextFrame() {
   }
 }
 
-void SMILTimeContainer::ServiceAnimations() {
+bool SMILTimeContainer::ServiceAnimations() {
   // If a synchronization is pending, we can flush it now.
   if (frame_scheduling_state_ == kSynchronizeAnimations) {
     DCHECK(wakeup_timer_.IsActive());
     wakeup_timer_.Stop();
     frame_scheduling_state_ = kAnimationFrame;
   }
   if (frame_scheduling_state_ != kAnimationFrame)
-    return;
+    return false;
   frame_scheduling_state_ = kIdle;
   // TODO(fs): The timeline should not be running if we're in an inactive
   // document, so this should be turned into a DCHECK.
   if (!GetDocument().IsActive())
-    return;
+    return false;
   TimingUpdate update(*this, Elapsed(), TimingUpdate::kNormal);
-  UpdateAnimationsAndScheduleFrameIfNeeded(update);
+  return UpdateAnimationsAndScheduleFrameIfNeeded(update);
 }
 
-void SMILTimeContainer::UpdateAnimationsAndScheduleFrameIfNeeded(
+bool SMILTimeContainer::UpdateAnimationsAndScheduleFrameIfNeeded(
     TimingUpdate& update) {
   DCHECK(GetDocument().IsActive());
   DCHECK(!wakeup_timer_.IsActive());
   // If the priority queue is empty, there are no timed elements to process and
   // no animations to apply, so we are done.
   if (priority_queue_.IsEmpty())
-    return;
+    return false;
   AnimationTargetsMutationsForbidden scope(this);
   UpdateTimedElements(update);
   ApplyTimedEffects(update.TargetTime());
   DCHECK(!wakeup_timer_.IsActive());
   DCHECK(!HasPendingSynchronization());
 
   if (!IsTimelineRunning())
-    return;
+    return false;
   SMILTime next_progress_time = NextProgressTime(update.TargetTime());
   if (!next_progress_time.IsFinite())
-    return;
+    return false;
   SMILTime delay_time = next_progress_time - update.TargetTime();
   DCHECK(delay_time.IsFinite());
   ScheduleAnimationFrame(
       base::TimeDelta::FromMicroseconds(delay_time.InMicroseconds()));
+  return true;
 }
 
 SMILTime SMILTimeContainer::NextProgressTime(SMILTime presentation_time) const {

diff --git a/third_party/blink/renderer/core/svg/animation/smil_time_container.h b/third_party/blink/renderer/core/svg/animation/smil_time_container.h
@@ -67,7 +67,8 @@ class CORE_EXPORT SMILTimeContainer final
   void Unpause();
   void SetElapsed(SMILTime);
 
-  void ServiceAnimations();
+  // True if an animation frame is successfully scheduled.
+  bool ServiceAnimations();
   bool HasAnimations() const;
 
   void ResetDocumentTime();
@@ -99,7 +100,7 @@ class CORE_EXPORT SMILTimeContainer final
   mojom::blink::ImageAnimationPolicy AnimationPolicy() const;
   bool AnimationsDisabled() const;
   class TimingUpdate;
-  void UpdateAnimationsAndScheduleFrameIfNeeded(TimingUpdate&);
+  bool UpdateAnimationsAndScheduleFrameIfNeeded(TimingUpdate&);
   void PrepareSeek(TimingUpdate&);
   void ResetIntervals();
   void UpdateIntervals(TimingUpdate&);

diff --git a/third_party/blink/renderer/core/svg/animation/smil_time_container_test.cc b/third_party/blink/renderer/core/svg/animation/smil_time_container_test.cc
@@ -80,7 +80,8 @@ TEST_F(SMILTimeContainerTest, ServiceAnimationsFlushesPendingSynchronizations) {
   rect->appendChild(animation);
 
   // Frame callback before the synchronization timer fires.
-  SVGDocumentExtensions::ServiceOnAnimationFrame(GetDocument());
+  SVGDocumentExtensions::ServiceSmilOnAnimationFrame(GetDocument());
+  SVGDocumentExtensions::ServiceWebAnimationsOnAnimationFrame(GetDocument());
 
   // The frame callback should have flushed any pending updates.
   EXPECT_EQ(100, rect->height()->CurrentValue()->Value(length_context));
@@ -127,7 +128,8 @@ class SMILTimeContainerAnimationPolicyOnceTest : public PageTestBase {
     platform()->RunForPeriod(delta);
     current_time_ += delta;
     GetAnimationClock().UpdateTime(current_time_);
-    SVGDocumentExtensions::ServiceOnAnimationFrame(GetDocument());
+    SVGDocumentExtensions::ServiceSmilOnAnimationFrame(GetDocument());
+    SVGDocumentExtensions::ServiceWebAnimationsOnAnimationFrame(GetDocument());
   }
 
   void OnContentLoaded(base::OnceCallback<void(Document&)> callback) {

diff --git a/third_party/blink/renderer/core/svg/svg_document_extensions.cc b/third_party/blink/renderer/core/svg/svg_document_extensions.cc
@@ -46,18 +46,31 @@ void SVGDocumentExtensions::AddWebAnimationsPendingSVGElement(
   web_animations_pending_svg_elements_.insert(&element);
 }
 
-void SVGDocumentExtensions::ServiceOnAnimationFrame(Document& document) {
+bool SVGDocumentExtensions::ServiceSmilOnAnimationFrame(Document& document) {
+  if (!document.SvgExtensions())
+    return false;
+  return document.AccessSVGExtensions().ServiceSmilAnimations();
+}
+
+void SVGDocumentExtensions::ServiceWebAnimationsOnAnimationFrame(
+    Document& document) {
   if (!document.SvgExtensions())
     return;
-  document.AccessSVGExtensions().ServiceAnimations();
+  document.AccessSVGExtensions().ServiceWebAnimations();
 }
 
-void SVGDocumentExtensions::ServiceAnimations() {
+bool SVGDocumentExtensions::ServiceSmilAnimations() {
+  bool did_schedule_animation_frame = false;
   HeapVector<Member<SVGSVGElement>> time_containers;
   CopyToVector(time_containers_, time_containers);
-  for (const auto& container : time_containers)
-    container->TimeContainer()->ServiceAnimations();
+  for (const auto& container : time_containers) {
+    did_schedule_animation_frame |=
+        container->TimeContainer()->ServiceAnimations();
+  }
+  return did_schedule_animation_frame;
+}
 
+void SVGDocumentExtensions::ServiceWebAnimations() {
   SVGElementSet web_animations_pending_svg_elements;
   web_animations_pending_svg_elements.swap(
       web_animations_pending_svg_elements_);

diff --git a/third_party/blink/renderer/core/svg/svg_document_extensions.h b/third_party/blink/renderer/core/svg/svg_document_extensions.h
@@ -49,11 +49,15 @@ class CORE_EXPORT SVGDocumentExtensions final
   // needs applying.
   void AddWebAnimationsPendingSVGElement(SVGElement&);
 
-  static void ServiceOnAnimationFrame(Document&);
+  // True if a SMIL animation frame is successfully scheduled.
+  static bool ServiceSmilOnAnimationFrame(Document&);
+  static void ServiceWebAnimationsOnAnimationFrame(Document&);
 
   void StartAnimations();
   void PauseAnimations();
-  void ServiceAnimations();
+  // True if a SMIL animation frame is successfully scheduled.
+  bool ServiceSmilAnimations();
+  void ServiceWebAnimations();
 
   void DispatchSVGLoadEventToOutermostSVGElements();