[SPv2] Unconditionally allocate compositor element ids.

For LayoutObjects which receive ObjectPaintProperties, always allocate
a compositor element id for it and plumb it into all associated property
tree nodes.

BUG=718564
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2860183002
Cr-Commit-Position: refs/heads/master@{#469747}

third_party/WebKit/Source/core/paint/ObjectPaintProperties.h

@@ -8,6 +8,7 @@
 #include <memory>
 #include "core/CoreExport.h"
 #include "platform/geometry/LayoutPoint.h"
+#include "platform/graphics/CompositorElementId.h"
 #include "platform/graphics/paint/ClipPaintPropertyNode.h"
 #include "platform/graphics/paint/EffectPaintPropertyNode.h"
 #include "platform/graphics/paint/PaintChunkProperties.h"
@@ -274,6 +275,13 @@ class CORE_EXPORT ObjectPaintProperties {
   }
 #endif
 
+  CompositorElementId& GetCompositorElementId() {
+    return compositor_element_id_;
+  }
+  void SetCompositorElementId(const CompositorElementId& id) {
+    compositor_element_id_ = id;
+  }
+
  private:
   ObjectPaintProperties() {}
 
@@ -319,6 +327,7 @@ class CORE_EXPORT ObjectPaintProperties {
   RefPtr<TransformPaintPropertyNode> svg_local_to_border_box_transform_;
   RefPtr<TransformPaintPropertyNode> scroll_translation_;
   RefPtr<TransformPaintPropertyNode> scrollbar_paint_offset_;
+  CompositorElementId compositor_element_id_;
 };
 
 }  // namespace blink

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp

@@ -413,16 +413,12 @@ void PaintPropertyTreeBuilder::UpdateTransform(
       if (style.Preserves3D() && !rendering_context_id)
         rendering_context_id = PtrHash<const LayoutObject>::GetHash(&object);
 
-      CompositorElementId compositor_element_id =
-          style.HasCurrentTransformAnimation()
-              ? CreateDomNodeBasedCompositorElementId(object)
-              : CompositorElementId();
-
       auto& properties = *object.GetMutableForPainting().PaintProperties();
       force_subtree_update |= properties.UpdateTransform(
           context.current.transform, matrix, TransformOrigin(box),
           context.current.should_flatten_inherited_transform,
-          rendering_context_id, compositing_reasons, compositor_element_id);
+          rendering_context_id, compositing_reasons,
+          properties.GetCompositorElementId());
     } else {
       if (auto* properties = object.GetMutableForPainting().PaintProperties())
         force_subtree_update |= properties->ClearTransform();
@@ -583,18 +579,13 @@ void PaintPropertyTreeBuilder::UpdateEffect(
                                                 style.BlendMode())
               : SkBlendMode::kSrcOver;
 
-      CompositorElementId compositor_element_id =
-          style.HasCurrentOpacityAnimation()
-              ? CreateDomNodeBasedCompositorElementId(object)
-              : CompositorElementId();
-
       DCHECK(!style.HasCurrentOpacityAnimation() ||
              compositing_reasons != kCompositingReasonNone);
 
       force_subtree_update |= properties.UpdateEffect(
           context.current_effect, context.current.transform, output_clip,
           kColorFilterNone, CompositorFilterOperations(), style.Opacity(),
-          blend_mode, compositing_reasons, compositor_element_id);
+          blend_mode, compositing_reasons, properties.GetCompositorElementId());
       if (has_mask) {
         // TODO(crbug.com/683425): PaintArtifactCompositor does not handle
         // grouping (i.e. descendant-dependent compositing reason) properly
@@ -675,10 +666,6 @@ void PaintPropertyTreeBuilder::UpdateFilter(
       // We may begin to composite our subtree prior to an animation starts,
       // but a compositor element ID is only needed when an animation is
       // current.
-      CompositorElementId compositor_element_id =
-          style.HasCurrentFilterAnimation()
-              ? CreateDomNodeBasedCompositorElementId(object)
-              : CompositorElementId();
       CompositingReasons compositing_reasons =
           CompositingReasonFinder::RequiresCompositingForFilterAnimation(style)
               ? kCompositingReasonActiveAnimation
@@ -690,7 +677,7 @@ void PaintPropertyTreeBuilder::UpdateFilter(
       force_subtree_update |= properties.UpdateFilter(
           context.current_effect, context.current.transform, output_clip,
           kColorFilterNone, std::move(filter), 1.f, SkBlendMode::kSrcOver,
-          compositing_reasons, compositor_element_id);
+          compositing_reasons, properties.GetCompositorElementId());
     } else {
       if (auto* properties = object.GetMutableForPainting().PaintProperties())
         force_subtree_update |= properties->ClearFilter();
@@ -975,17 +962,15 @@ void PaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation(
           force_subtree_update = true;
       }
 
-      CompositorElementId compositor_element_id =
-          CreateDomNodeBasedCompositorElementId(object);
       TransformationMatrix matrix = TransformationMatrix().Translate(
           -scroll_offset.Width(), -scroll_offset.Height());
       force_subtree_update |= properties.UpdateScrollTranslation(
           context.current.transform, matrix, FloatPoint3D(),
           context.current.should_flatten_inherited_transform,
           context.current.rendering_context_id, kCompositingReasonNone,
-          compositor_element_id, context.current.scroll, scroll_clip,
-          scroll_bounds, user_scrollable_horizontal, user_scrollable_vertical,
-          reasons, scrollable_area);
+          properties.GetCompositorElementId(), context.current.scroll,
+          scroll_clip, scroll_bounds, user_scrollable_horizontal,
+          user_scrollable_vertical, reasons, scrollable_area);
     } else {
       // Ensure pre-existing properties are cleared.
       if (auto* properties = object.GetMutableForPainting().PaintProperties())
@@ -1196,7 +1181,13 @@ void PaintPropertyTreeBuilder::UpdatePaintProperties(
   bool had_paint_properties = object.PaintProperties();
 
   if (needs_paint_properties) {
-    object.GetMutableForPainting().EnsurePaintProperties();
+    ObjectPaintProperties& paint_properties =
+        object.GetMutableForPainting().EnsurePaintProperties();
+    if (!had_paint_properties &&
+        RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
+      paint_properties.SetCompositorElementId(
+          CreateDomNodeBasedCompositorElementId(object));
+    }
   } else {
     object.GetMutableForPainting().ClearPaintProperties();
     if (had_paint_properties)

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp

@@ -3162,20 +3162,20 @@ TEST_P(PaintPropertyTreeBuilderTest, ChangePositionUpdateDescendantProperties) {
 }
 
 TEST_P(PaintPropertyTreeBuilderTest,
-       TransformNodeNotAnimatedHasNoCompositorElementId) {
+       TransformNodeNotAnimatedStillHasCompositorElementId) {
   SetBodyInnerHTML("<div id='target' style='transform: translateX(2em)'></div");
   const ObjectPaintProperties* properties = PaintPropertiesForElement("target");
   EXPECT_TRUE(properties->Transform());
-  EXPECT_EQ(CompositorElementId(),
+  EXPECT_NE(CompositorElementId(),
             properties->Transform()->GetCompositorElementId());
 }
 
 TEST_P(PaintPropertyTreeBuilderTest,
-       EffectNodeNotAnimatedHasNoCompositorElementId) {
+       EffectNodeNotAnimatedStillHasCompositorElementId) {
   SetBodyInnerHTML("<div id='target' style='opacity: 0.5'></div");
   const ObjectPaintProperties* properties = PaintPropertiesForElement("target");
   EXPECT_TRUE(properties->Effect());
-  EXPECT_EQ(CompositorElementId(),
+  EXPECT_NE(CompositorElementId(),
             properties->Effect()->GetCompositorElementId());
 }
 

third_party/WebKit/Source/core/paint/PaintPropertyTreeUpdateTests.cpp

@@ -571,7 +571,7 @@ TEST_P(PaintPropertyTreeUpdateTest,
 }
 
 TEST_P(PaintPropertyTreeUpdateTest,
-       TransformNodeLosesCompositorElementIdWhenAnimationRemoved) {
+       TransformNodeDoesNotLoseCompositorElementIdWhenAnimationRemoved) {
   LoadTestData("transform-animation.html");
 
   Element* target = GetDocument().getElementById("target");
@@ -586,12 +586,12 @@ TEST_P(PaintPropertyTreeUpdateTest,
   // Remove the animation but keep the transform on the element.
   target->removeAttribute(HTMLNames::classAttr);
   GetDocument().View()->UpdateAllLifecyclePhases();
-  EXPECT_EQ(CompositorElementId(),
+  EXPECT_NE(CompositorElementId(),
             properties->Transform()->GetCompositorElementId());
 }
 
 TEST_P(PaintPropertyTreeUpdateTest,
-       EffectNodeLosesCompositorElementIdWhenAnimationRemoved) {
+       EffectNodeDoesNotLoseCompositorElementIdWhenAnimationRemoved) {
   LoadTestData("opacity-animation.html");
 
   Element* target = GetDocument().getElementById("target");
@@ -605,7 +605,7 @@ TEST_P(PaintPropertyTreeUpdateTest,
 
   target->removeAttribute(HTMLNames::classAttr);
   GetDocument().View()->UpdateAllLifecyclePhases();
-  EXPECT_EQ(CompositorElementId(),
+  EXPECT_NE(CompositorElementId(),
             properties->Effect()->GetCompositorElementId());
 }
 

third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.cpp

@@ -40,30 +40,18 @@ bool IsAncestorOf(const PropertyNode* ancestor, const PropertyNode* child) {
 
 const CompositorElementId PropertyTreeState::GetCompositorElementId(
     const CompositorElementIdSet& element_ids) const {
-// The effect or transform nodes could have a compositor element id. The order
-// doesn't matter as the element id should be the same on all that have a
-// non-default CompositorElementId.
-//
-// Note that PropertyTreeState acts as a context that accumulates state as we
-// traverse the tree building layers. This means that we could see a compositor
-// element id 'A' for a parent layer in conjunction with a compositor element id
-// 'B' for a child layer. To preserve uniqueness of element ids, then, we check
-// for presence in the |element_ids| set (which represents element ids already
-// previously attached to a layer). This is an interim step while we pursue
-// broader rework of animation subsystem noted in http://crbug.com/709137.
-#if DCHECK_IS_ON()
-  CompositorElementId expected_element_id;
-  CompositorElementId effect_element_id = Effect()->GetCompositorElementId();
-  if (effect_element_id && !element_ids.Contains(effect_element_id)) {
-    expected_element_id = effect_element_id;
-  }
-  CompositorElementId transform_element_id =
-      Transform()->GetCompositorElementId();
-  if (expected_element_id && transform_element_id &&
-      !element_ids.Contains(transform_element_id)) {
-    DCHECK_EQ(expected_element_id, transform_element_id);
-  }
-#endif
+  // The effect or transform nodes could have a compositor element id. The order
+  // doesn't matter as the element id should be the same on all that have a
+  // non-default CompositorElementId.
+  //
+  // Note that PropertyTreeState acts as a context that accumulates state as we
+  // traverse the tree building layers. This means that we could see a
+  // compositor element id 'A' for a parent layer in conjunction with a
+  // compositor element id 'B' for a child layer. To preserve uniqueness of
+  // element ids, then, we check for presence in the |element_ids| set (which
+  // represents element ids already previously attached to a layer). This is an
+  // interim step while we pursue broader rework of animation subsystem noted in
+  // http://crbug.com/709137.
   if (Effect()->GetCompositorElementId() &&
       !element_ids.Contains(Effect()->GetCompositorElementId()))
     return Effect()->GetCompositorElementId();