Reland "[BGPT] Let cc initialize animation states in property nodes"

This is a reland of 54b17c7e

The original CL called MutatorHost::InitClientAnimationState()
before updating animations. The new CL calls it after updating
animations. Not sure why the original CL caused flakiness of
virtual/threaded/animations/hit-testing/composited-with-hit-testing.html,
but the test is no longer flaky with this new CL.

Original change's description:
> [BGPT] Let cc initialize animation states in property nodes
>
> In this way, blink::PropertyTreeManager no longer needs to care about
> the animation states. Now we initialize and change the states from a
> single source to avoid duplicate code and inconsistency.
>
> Bug: 935770
> Change-Id: I7f98d66677f557213fd4559895e9c2069de196e7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1513922
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Robert Flack <flackr@chromium.org>
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#640892}

TBR=flackr@chromium.org, pdr@chromium.org

Bug: 935770
Change-Id: Icd7ec59b9e5b95f2c268baf88732788cde5da9c6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1525449Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641098}

cc/animation/animation_host.cc

@@ -129,6 +129,11 @@ void AnimationHost::RemoveAnimationTimeline(
   SetNeedsPushProperties();
 }
 
+void AnimationHost::InitClientAnimationState() {
+  for (auto map_entry : element_to_animations_map_)
+    map_entry.second->InitClientAnimationState();
+}
+
 void AnimationHost::RegisterElement(ElementId element_id,
                                     ElementListType list_type) {
   scoped_refptr<ElementAnimations> element_animations =

cc/animation/animation_host.h

@@ -90,6 +90,8 @@ class CC_ANIMATION_EXPORT AnimationHost : public MutatorHost,
       bool supports_impl_scrolling) const override;
   void ClearMutators() override;
 
+  void InitClientAnimationState() override;
+
   void RegisterElement(ElementId element_id,
                        ElementListType list_type) override;
   void UnregisterElement(ElementId element_id,

cc/animation/element_animations.cc

@@ -312,6 +312,14 @@ void ElementAnimations::NotifyClientScrollOffsetAnimated(
                            keyframe_model);
 }
 
+void ElementAnimations::InitClientAnimationState() {
+  // Clear current states so that UpdateClientAnimationState() will send all
+  // (instead of only changed) recalculated current states to the client.
+  pending_state_.Clear();
+  active_state_.Clear();
+  UpdateClientAnimationState();
+}
+
 void ElementAnimations::UpdateClientAnimationState() {
   if (!element_id())
     return;

cc/animation/element_animations.h

@@ -124,6 +124,13 @@ class CC_ANIMATION_EXPORT ElementAnimations
   bool ScrollOffsetAnimationWasInterrupted() const;
 
   void SetNeedsPushProperties();
+
+  // Initializes client animation state by calling client's
+  // ElementIsAnimatingChanged() method with the current animation state.
+  void InitClientAnimationState();
+  // Updates client animation state by calling client's
+  // ElementIsAnimatingChanged() method with the state containing properties
+  // that have changed since the last update.
   void UpdateClientAnimationState();
 
   void NotifyClientFloatAnimated(float opacity,

cc/trees/mutator_host.h

@@ -42,6 +42,8 @@ class MutatorHost {
 
   virtual void ClearMutators() = 0;
 
+  virtual void InitClientAnimationState() = 0;
+
   virtual void RegisterElement(ElementId element_id,
                                ElementListType list_type) = 0;
   virtual void UnregisterElement(ElementId element_id,

third_party/blink/renderer/core/animation/compositor_animations.h

@@ -148,18 +148,6 @@ class CORE_EXPORT CompositorAnimations {
   static FailureCode CheckCanStartElementOnCompositor(const Element&);
 
   friend class AnimationCompositorAnimationsTest;
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           CanStartElementOnCompositorTransformCAP);
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           CanStartElementOnCompositorEffectCAP);
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           CanStartElementOnCompositorEffect);
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           CannotStartElementOnCompositorEffectSVG);
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           CancelIncompatibleCompositorAnimations);
-  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
-                           NonAnimatedTransformPropertyChangeGetsUpdated);
 };
 
 }  // namespace blink

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -2458,6 +2458,8 @@ void LocalFrameView::RunPaintLifecyclePhase() {
       RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
     if (!print_mode_enabled) {
       auto& composited_element_ids = composited_element_ids_;
+      bool needed_update = !paint_artifact_compositor_ ||
+                           paint_artifact_compositor_->NeedsUpdate();
       PushPaintArtifactToCompositor(composited_element_ids.value());
       ForAllNonThrottledLocalFrameViews(
           [&composited_element_ids](LocalFrameView& frame_view) {
@@ -2466,6 +2468,19 @@ void LocalFrameView::RunPaintLifecyclePhase() {
                 DocumentLifecycle::kPaintClean, composited_element_ids);
           });
 
+      // Initialize animation properties in the newly created paint property
+      // nodes according to the current animation state. This is mainly for
+      // the running composited animations which didn't change state during
+      // above UpdateAnimations() but associated with new paint property nodes.
+      if (needed_update) {
+        auto* root_layer = RootCcLayer();
+        if (root_layer && root_layer->layer_tree_host()) {
+          root_layer->layer_tree_host()
+              ->mutator_host()
+              ->InitClientAnimationState();
+        }
+      }
+
       // Notify the controller that the artifact has been pushed and some
       // lifecycle state can be freed (such as raster invalidations).
       if (paint_controller_)

third_party/blink/renderer/core/paint/link_highlight_impl.cc

@@ -102,13 +102,7 @@ LinkHighlightImpl::LinkHighlightImpl(Node* node)
   geometry_needs_update_ = true;
 
   EffectPaintPropertyNode::State state;
-  // In theory this value doesn't matter because the actual opacity during
-  // composited animation is controlled by cc. However, this value could prevent
-  // potential glitches at the end of the animation when opacity should be 0.
-  // For web tests we don't fade out.
-  // TODO(crbug.com/935770): Investigate the root cause that seems a timing
-  // issue at the end of a composited animation in BlinkGenPropertyTree mode.
-  state.opacity = WebTestSupport::IsRunningWebTest() ? kStartOpacity : 0;
+  state.opacity = kStartOpacity;
   state.local_transform_space = &TransformPaintPropertyNode::Root();
   state.compositor_element_id = element_id_;
   state.direct_compositing_reasons = CompositingReason::kActiveOpacityAnimation;

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc

@@ -12,6 +12,7 @@
 #include "cc/paint/display_item_list.h"
 #include "cc/trees/effect_node.h"
 #include "cc/trees/layer_tree_host.h"
+#include "cc/trees/mutator_host.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/renderer/platform/graphics/compositing/content_layer_client_impl.h"
 #include "third_party/blink/renderer/platform/graphics/graphics_context.h"

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc

@@ -3355,8 +3355,6 @@ TEST_P(PaintArtifactCompositorTest, CreatesViewportNodes) {
 enum {
   kNoRenderSurface = 0,
   kHasRenderSurface = 1 << 0,
-  kHasOpacityAnimation = 1 << 1,
-  kHasFilterAnimation = 1 << 2,
 };
 
 #define EXPECT_OPACITY(effect_id, expected_opacity, expected_flags)      \
@@ -3365,10 +3363,6 @@ enum {
     EXPECT_EQ(expected_opacity, effect->opacity);                        \
     EXPECT_EQ(!!((expected_flags)&kHasRenderSurface),                    \
               effect->has_render_surface);                               \
-    EXPECT_EQ(!!((expected_flags)&kHasOpacityAnimation),                 \
-              effect->has_potential_opacity_animation);                  \
-    EXPECT_EQ(!!((expected_flags)&kHasFilterAnimation),                  \
-              effect->has_potential_filter_animation);                   \
   } while (false)
 
 TEST_P(PaintArtifactCompositorTest, OpacityRenderSurfaces) {
@@ -3475,14 +3469,14 @@ TEST_P(PaintArtifactCompositorTest, OpacityAnimationRenderSurfaces) {
 
   // Effects of layer 0, 1, 5 each has one compositing layer, so don't have
   // render surface.
-  EXPECT_OPACITY(effect_ids[0], 1.f, kHasOpacityAnimation);
-  EXPECT_OPACITY(effect_ids[1], 1.f, kHasOpacityAnimation);
-  EXPECT_OPACITY(effect_ids[5], 1.f, kHasOpacityAnimation);
+  EXPECT_OPACITY(effect_ids[0], 1.f, kNoRenderSurface);
+  EXPECT_OPACITY(effect_ids[1], 1.f, kNoRenderSurface);
+  EXPECT_OPACITY(effect_ids[5], 1.f, kNoRenderSurface);
 
   // Layer 2 and 3 have the same effect state. The effect has render surface
   // because it has two compositing layers.
   EXPECT_EQ(effect_ids[2], effect_ids[3]);
-  EXPECT_OPACITY(effect_ids[2], 1.f, kHasRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(effect_ids[2], 1.f, kHasRenderSurface);
 
   // TODO(crbug.com/937573): It's an invalid case that an animating effect
   // doesn't have a layer, but we still keep the case in this test case because
@@ -3490,15 +3484,15 @@ TEST_P(PaintArtifactCompositorTest, OpacityAnimationRenderSurfaces) {
   const auto& effect_tree = GetPropertyTrees().effect_tree;
   int id_a = effect_tree.Node(effect_ids[0])->parent_id;
   EXPECT_EQ(id_a, effect_tree.Node(effect_ids[1])->parent_id);
-  EXPECT_OPACITY(id_a, 1.f, kNoRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(id_a, 1.f, kNoRenderSurface);
 
   // Effect |c| has one direct and one indirect compositing layers, so has
   // render surface.
-  EXPECT_OPACITY(effect_ids[4], 1.f, kHasRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(effect_ids[4], 1.f, kHasRenderSurface);
 
   // TODO(crbug.com/937573): Same as |a|.
   EXPECT_OPACITY(effect_tree.Node(effect_ids[4])->parent_id, 1.f,
-                 kNoRenderSurface | kHasOpacityAnimation);
+                 kNoRenderSurface);
 }
 
 TEST_P(PaintArtifactCompositorTest, OpacityRenderSurfacesWithBackdropChildren) {
@@ -3593,17 +3587,15 @@ TEST_P(PaintArtifactCompositorTest,
   // layer0's opacity animation needs a render surfafce because it affects
   // both layer0 and layer1.
   int layer0_effect_id = ContentLayerAt(0)->effect_tree_index();
-  EXPECT_OPACITY(layer0_effect_id, 1.f,
-                 kHasRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(layer0_effect_id, 1.f, kHasRenderSurface);
   // layer1's opacity animation doesn't need a render surface because it
   // affects layer1 only.
   int layer1_effect_id = ContentLayerAt(1)->effect_tree_index();
-  EXPECT_OPACITY(layer1_effect_id, 1.f,
-                 kNoRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(layer1_effect_id, 1.f, kNoRenderSurface);
   // Though |opacity| affects both layer0 and layer1, layer0's effect has
   // render surface, so |opacity| doesn't need a render surface.
   int opacity_id = effect_tree.Node(layer0_effect_id)->parent_id;
-  EXPECT_OPACITY(opacity_id, 1.f, kNoRenderSurface | kHasOpacityAnimation);
+  EXPECT_OPACITY(opacity_id, 1.f, kNoRenderSurface);
 }
 
 TEST_P(PaintArtifactCompositorTest, FilterCreatesRenderSurface) {
@@ -3633,7 +3625,7 @@ TEST_P(PaintArtifactCompositorTest,
              .Build());
   ASSERT_EQ(1u, ContentLayerCount());
   EXPECT_OPACITY(ContentLayerAt(0)->effect_tree_index(), 1.f,
-                 kHasRenderSurface | kHasFilterAnimation);
+                 kHasRenderSurface);
 }
 
 TEST_P(PaintArtifactCompositorTest,

third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc

@@ -276,11 +276,6 @@ int PropertyTreeManager::EnsureCompositorTransformNode(
     compositor_node.element_id = compositor_element_id;
   }
 
-  // Set has_potential_animation in case we push property tree during an ongoing
-  // animation. This condition should be kept consistent with cc.
-  if (transform_node.IsRunningAnimationOnCompositor())
-    compositor_node.has_potential_animation = true;
-
   // If this transform is a scroll offset translation, create the associated
   // compositor scroll property node and adjust the compositor transform node's
   // scroll offset.
@@ -773,17 +768,6 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
         effect_node.id;
   }
 
-  // Set has_potential_xxx_animation in case we push property tree during
-  // ongoing animations. The conditions should be kept consistent with cc.
-  if (next_effect.IsRunningOpacityAnimationOnCompositor())
-    effect_node.has_potential_opacity_animation = true;
-  if (next_effect.IsRunningFilterAnimationOnCompositor())
-    effect_node.has_potential_filter_animation = true;
-  // TODO(crbug.com/938679): Set effect_node
-  // .has_potential_backdrop_filter_animation when we have it.
-  // if (next_effect.IsRunningBackdropAnimationOnCompositor())
-  //   effect_node.has_potential_backdrop_filter_animation = true;
-
   effect_stack_.emplace_back(current_);
   SetCurrentEffectState(effect_node, CcEffectType::kEffect, next_effect,
                         *output_clip);