Revert "[CompositeAfterPaint] Allow non-atomic effects in multicol"

This reverts commit 117ea7fd.

Reason for revert: crbug.com/1067845

Original change's description:
> [CompositeAfterPaint] Allow non-atomic effects in multicol
> 
> This is not fully correct, but we have to accept it in our current
> multicol painting based on FragmentClips.
> 
> Actually we have already allowed non-atomic effects in multicol within
> composited layers since SlimmingPaintV175. A fragmented element with
> effect applies the effect on each fragment separately. We have to do
> this because each fragment has its own fragment clip, and its own paint
> properties because they may depend on the fragment clip.
> 
> This CL enables non-atomic composited effects for CompositeAfterPaint.
> This is not an issue in pre-CompositeAfterPaint which doesn't allow
> fragmentation of composited layers (which is incorrect).
> 
> The root issue will be fully fixed by LayoutNG block fragmentation
> which doesn't need FragmentClips.
> 
> Bug: 1064341
> Change-Id: Id6ae4c263381f24d037f3c2bc493d73cd050598e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2124340
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#754166}

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1064341,1067845
Change-Id: I14da5cd66b644c39404446be61fc16a02dabe6f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135834Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756775}

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

@@ -260,12 +260,6 @@ class FragmentPaintPropertyTreeBuilder {
     full_context_.clip_changed |= cleared;
   }
 
-  CompositorElementId GetCompositorElementId(
-      CompositorElementIdNamespace namespace_id) const {
-    return CompositorElementIdFromUniqueObjectId(fragment_data_.UniqueId(),
-                                                 namespace_id);
-  }
-
   const LayoutObject& object_;
   NGPrePaintInfo* pre_paint_info_;
   // The tree builder context for the whole object.
@@ -540,7 +534,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateStickyTranslation() {
       const auto& box_model = ToLayoutBoxModelObject(object_);
       TransformPaintPropertyNode::State state{
           FloatSize(box_model.StickyPositionOffset())};
-      state.compositor_element_id = GetCompositorElementId(
+      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+          box_model.UniqueId(),
           CompositorElementIdNamespace::kStickyTranslation);
 
       auto* layer = box_model.Layer();
@@ -806,8 +801,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
           object_.HasHiddenBackface()
               ? TransformPaintPropertyNode::BackfaceVisibility::kHidden
               : TransformPaintPropertyNode::BackfaceVisibility::kVisible;
-      state.compositor_element_id = GetCompositorElementId(
-          CompositorElementIdNamespace::kPrimaryTransform);
+      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+          object_.UniqueId(), CompositorElementIdNamespace::kPrimaryTransform);
 
       TransformPaintPropertyNode::AnimationState animation_state;
       animation_state.is_running_animation_on_compositor =
@@ -1050,8 +1045,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
 
       CompositorElementId mask_compositor_element_id;
       if (mask_clip || has_spv1_composited_clip_path) {
-        mask_compositor_element_id =
-            GetCompositorElementId(CompositorElementIdNamespace::kEffectMask);
+        mask_compositor_element_id = CompositorElementIdFromUniqueObjectId(
+            object_.UniqueId(), CompositorElementIdNamespace::kEffectMask);
       }
 
       EffectPaintPropertyNode::State state;
@@ -1088,15 +1083,15 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
           full_context_.direct_compositing_reasons &
           CompositingReasonsForEffectProperty();
       if (state.direct_compositing_reasons) {
-        state.compositor_element_id = GetCompositorElementId(
-            CompositorElementIdNamespace::kPrimaryEffect);
+        state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+            object_.UniqueId(), CompositorElementIdNamespace::kPrimaryEffect);
       } else {
         // The effect node CompositorElementId is used to uniquely identify
         // renderpasses so even if we don't need one for animations we still
         // need to set an id. Using kPrimary avoids confusing cc::Animation
         // into thinking the element has been composited for animations.
-        state.compositor_element_id =
-            GetCompositorElementId(CompositorElementIdNamespace::kPrimary);
+        state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+            object_.UniqueId(), CompositorElementIdNamespace::kPrimary);
       }
 
       // TODO(crbug.com/900241): Remove these setters when we can use
@@ -1153,8 +1148,10 @@ void FragmentPaintPropertyTreeBuilder::UpdateEffect() {
         clip_path_state.local_transform_space = context_.current.transform;
         clip_path_state.output_clip = output_clip;
         clip_path_state.blend_mode = SkBlendMode::kDstIn;
-        clip_path_state.compositor_element_id = GetCompositorElementId(
-            CompositorElementIdNamespace::kEffectClipPath);
+        clip_path_state.compositor_element_id =
+            CompositorElementIdFromUniqueObjectId(
+                object_.UniqueId(),
+                CompositorElementIdNamespace::kEffectClipPath);
         OnUpdate(
             properties_->UpdateClipPath(parent, std::move(clip_path_state)));
       } else {
@@ -1268,8 +1265,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateFilter() {
       state.direct_compositing_reasons =
           full_context_.direct_compositing_reasons &
           CompositingReasonsForFilterProperty();
-      state.compositor_element_id =
-          GetCompositorElementId(CompositorElementIdNamespace::kEffectFilter);
+      state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
+          object_.UniqueId(), CompositorElementIdNamespace::kEffectFilter);
 
       // TODO(crbug.com/900241): Remove the setter when we can use
       // state.direct_compositing_reasons to check for active animations.

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

@@ -4350,6 +4350,10 @@ TEST_P(PaintPropertyTreeBuilderTest, OverflowClipUnderMultiColumn) {
 }
 
 TEST_P(PaintPropertyTreeBuilderTest, CompositedUnderMultiColumn) {
+  // TODO(crbug.com/1064341): This test crashes in CompositeAfterPaint. Fix it.
+  if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+    return;
+
   SetBodyInnerHTML(R"HTML(
     <style>body { margin: 0; }</style>
     <div id='multicol' style='columns:3; column-fill:auto; column-gap: 0;
@@ -4498,6 +4502,10 @@ TEST_P(PaintPropertyTreeBuilderTest, FrameUnderMulticol) {
 }
 
 TEST_P(PaintPropertyTreeBuilderTest, CompositedMulticolFrameUnderMulticol) {
+  // TODO(crbug.com/1064341): This test crashes in CompositeAfterPaint. Fix it.
+  if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+    return;
+
   SetBodyInnerHTML(R"HTML(
     <style>body { margin: 0 }</style>
     <div style='columns: 3; column-gap: 0; column-fill: auto;
@@ -5646,6 +5654,10 @@ TEST_P(PaintPropertyTreeBuilderTest, CompositedLayerSkipsFragmentClip) {
 }
 
 TEST_P(PaintPropertyTreeBuilderTest, CompositedLayerUnderClipUnderMulticol) {
+  // TODO(crbug.com/1064341): This test crashes in CompositeAfterPaint. Fix it.
+  if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+    return;
+
   SetBodyInnerHTML(R"HTML(
     <div id="multicol" style="columns: 2">
       <div id="clip" style="height: 100px; overflow: hidden">

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

@@ -1049,20 +1049,11 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
   BuildEffectNodesRecursively(*next_effect.Parent());
   DCHECK_EQ(&next_effect.Parent()->Unalias(), current_.effect);
 
-  bool has_multiple_groups = false;
-  if (GetEffectTree().Node(next_effect.CcNodeId(new_sequence_number_))) {
-    if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
-      // TODO(crbug.com/1064341): We have to allow one blink effect node to
-      // apply to multiple groups in block fragments (multicol, etc.) due to
-      // the current FragmentClip implementation. This can only be fixed by
-      // LayoutNG block fragments. For now we'll create multiple cc effect
-      // nodes in the case.
-      has_multiple_groups = true;
-    } else {
-      NOTREACHED() << "Malformed paint artifact. Paint chunks under the same"
-                      " effect should be contiguous.";
-    }
-  }
+#if DCHECK_IS_ON()
+  DCHECK(!GetEffectTree().Node(next_effect.CcNodeId(new_sequence_number_)))
+      << "Malformed paint artifact. Paint chunks under the same effect should "
+         "be contiguous.";
+#endif
 
   auto backdrop_effect_state = kNoBackdropEffect;
   int output_clip_id = 0;
@@ -1089,16 +1080,14 @@ void PropertyTreeManager::BuildEffectNodesRecursively(
   int effect_node_id =
       GetEffectTree().Insert(cc::EffectNode(), current_.effect_id);
   auto& effect_node = *GetEffectTree().Node(effect_node_id);
-
-  if (!has_multiple_groups)
-    next_effect.SetCcNodeId(new_sequence_number_, effect_node_id);
+  next_effect.SetCcNodeId(new_sequence_number_, effect_node_id);
 
   PopulateCcEffectNode(effect_node, next_effect, output_clip_id,
                        backdrop_effect_state);
 
   CompositorElementId compositor_element_id =
       next_effect.GetCompositorElementId();
-  if (compositor_element_id && !has_multiple_groups) {
+  if (compositor_element_id) {
     DCHECK(!property_trees_.element_id_to_effect_node_index.contains(
         compositor_element_id));
     property_trees_.element_id_to_effect_node_index[compositor_element_id] =

third_party/blink/web_tests/FlagExpectations/composite-after-paint

@@ -100,8 +100,16 @@ http/tests/devtools/layers/layer-compositing-reasons.js [ Failure ]
 # Missing WheelEventHandler
 http/tests/devtools/layers/layer-scroll-rects-get.js [ Failure ]
 
-# Outline paints incorrectly with columns.
-crbug.com/1047358 paint/pagination/composited-paginated-outlined-box.html [ Failure ]
+# Crash on non-contiguous effect on multiple columns
+crbug.com/1064341 fast/multicol/composited-layer-will-change.html [ Crash ]
+crbug.com/1064341 paint/clipath/change-mask-clip-path-multicol-crash.html [ Crash ]
+crbug.com/1064341 fast/multicol/composited-layer-multiple-fragments-translated.html [ Crash ]
+crbug.com/1064341 fast/multicol/composited-layer-multiple-fragments.html [ Crash ]
+crbug.com/1064341 fast/multicol/composited-layer-nested.html [ Crash ]
+
+# Outline paints incorrectly with columns. For now this is hidden by crbug.com/1064341.
+# crbug.com/1047358 paint/pagination/composited-paginated-outlined-box.html [ Failure ]
+crbug.com/1064341 paint/pagination/composited-paginated-outlined-box.html [ Crash ]
 
 # Crash on weird clip hierarchy in multiple columns
 compositing/geometry/composited-in-columns.html [ Crash ]