Don't skip fragment clip if there are other clips for composited layers

Composited layer under multicol is still not fully correctly
supported. Currently we don't allow fragmentation of composited layers,
and skip fragment clip for them. However, if there are other clips below
the fragment clip, we should not skip fragment clip to ensure the other
clips are correctly applied. This is still not fully correct (see the
comment in the code for details), but the result is less broken in many
cases. Hopefully we'll fully fix the issue in
CompositeAfterPaint+LayoutNG-box-fragments.

Bug: 976187
Change-Id: Ica9acc71f0d1204d6380f1bcac3e2159631d63e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670413
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671480}

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

@@ -2491,38 +2491,48 @@ void PaintPropertyTreeBuilder::InitSingleFragmentFromParent(
   // should also skip any fragment clip created by the skipped pagination
   // container. We also need to skip fragment clip if the object is a paint
   // invalidation container which doesn't allow fragmentation.
-  // TODO(crbug.com/803649): This may also skip necessary clips under the
-  // skipped fragment clip.
-  if (object_.IsColumnSpanAll() ||
-      (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
-       object_.IsPaintInvalidationContainer() &&
-       ToLayoutBoxModelObject(object_).Layer()->EnclosingPaginationLayer())) {
-    if (const auto* pagination_layer_in_tree_hierarchy =
-            object_.Parent()->EnclosingLayer()->EnclosingPaginationLayer()) {
-      const auto& clip_container =
-          pagination_layer_in_tree_hierarchy->GetLayoutObject();
-      const auto* properties = clip_container.FirstFragment().PaintProperties();
-      if (properties && properties->FragmentClip()) {
-        // However, because we don't allow an object's clip to escape the
-        // output clip of the object's effect, we can't skip fragment clip if
-        // between this object and the container there is any effect that has
-        // an output clip. TODO(crbug.com/803649): Fix this workaround.
-        const auto& clip_container_effect = clip_container.FirstFragment()
-                                                .LocalBorderBoxProperties()
-                                                .Effect()
-                                                .Unalias();
-        for (const auto* effect =
-                 &context_.fragments[0].current_effect->Unalias();
-             effect && effect != &clip_container_effect;
-             effect = SafeUnalias(effect->Parent())) {
-          if (effect->OutputClip())
-            return;
-        }
-        context_.fragments[0].current.clip =
-            properties->FragmentClip()->Parent();
-      }
-    }
+  bool skip_fragment_clip_for_composited_layer =
+      !RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
+      object_.IsPaintInvalidationContainer() &&
+      ToLayoutBoxModelObject(object_).Layer()->EnclosingPaginationLayer();
+  if (!skip_fragment_clip_for_composited_layer && !object_.IsColumnSpanAll())
+    return;
+
+  const auto* pagination_layer_in_tree_hierarchy =
+      object_.Parent()->EnclosingLayer()->EnclosingPaginationLayer();
+  if (!pagination_layer_in_tree_hierarchy)
+    return;
+
+  const auto& clip_container =
+      pagination_layer_in_tree_hierarchy->GetLayoutObject();
+  const auto* properties = clip_container.FirstFragment().PaintProperties();
+  if (!properties || !properties->FragmentClip())
+    return;
+
+  // Skip fragment clip for composited layer only when there are no other clips.
+  // TODO(crbug.com/803649): This is still incorrect if this object first
+  // appear in the second or later fragment of its parent.
+  if (skip_fragment_clip_for_composited_layer &&
+      properties->FragmentClip() != context_.fragments[0].current.clip)
+    return;
+
+  // However, because we don't allow an object's clip to escape the
+  // output clip of the object's effect, we can't skip fragment clip if
+  // between this object and the container there is any effect that has
+  // an output clip. TODO(crbug.com/803649): Fix this workaround.
+  const auto& clip_container_effect = clip_container.FirstFragment()
+                                          .LocalBorderBoxProperties()
+                                          .Effect()
+                                          .Unalias();
+  for (const auto* effect = &context_.fragments[0].current_effect->Unalias();
+       effect && effect != &clip_container_effect;
+       effect = SafeUnalias(effect->Parent())) {
+    if (effect->OutputClip())
+      return;
   }
+
+  // Skip the fragment clip.
+  context_.fragments[0].current.clip = properties->FragmentClip()->Parent();
 }
 
 void PaintPropertyTreeBuilder::UpdateCompositedLayerPaginationOffset() {

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

@@ -5841,6 +5841,28 @@ TEST_P(PaintPropertyTreeBuilderTest, CompositedLayerSkipsFragmentClip) {
                                    .Clip());
 }
 
+TEST_P(PaintPropertyTreeBuilderTest, CompositedLayerUnderClipUnerMulticol) {
+  SetBodyInnerHTML(R"HTML(
+    <div id="multicol" style="columns: 2">
+      <div id="clip" style="height: 100px; overflow: hidden">
+        <div id="composited"
+             style="width: 200px; height: 200px; will-change: transform">
+        </div>
+      </div>
+    </div>
+  )HTML");
+
+  const auto* flow_thread =
+      GetLayoutObjectByElementId("multicol")->SlowFirstChild();
+  const auto* fragment_clip =
+      flow_thread->FirstFragment().PaintProperties()->FragmentClip();
+  const auto* clip_properties = PaintPropertiesForElement("clip");
+  const auto* composited = GetLayoutObjectByElementId("composited");
+  EXPECT_EQ(clip_properties->OverflowClip(),
+            &composited->FirstFragment().LocalBorderBoxProperties().Clip());
+  EXPECT_EQ(fragment_clip, clip_properties->OverflowClip()->Parent());
+}
+
 TEST_P(PaintPropertyTreeBuilderTest, RepeatingFixedPositionInPagedMedia) {
   SetBodyInnerHTML(R"HTML(
     <div id="fixed" style="position: fixed; top: 20px; left: 20px">

third_party/blink/web_tests/external/wpt/css/css-multicol/composited-under-clip-under-multicol-ref.html

+<!DOCTYPE html>
+<div style="width: 100px; height: 100px; background: green"></div>

third_party/blink/web_tests/external/wpt/css/css-multicol/composited-under-clip-under-multicol.html

+<!DOCTYPE html>
+<link rel="help" href="https://drafts.csswg.org/css-multicol">
+<link rel="match" href="composited-under-clip-under-multicol-ref.html">
+<meta name="assert" content="Test that clip under multicol is correctly applied on composited child">
+<style>
+.columns {
+  columns: 2;
+  column-gap: 20px;
+  width: 220px;
+  height: 100px;
+}
+.clip {
+  height: 100px;
+  overflow: hidden;
+}
+.composited {
+  will-change: transform;
+  margin-top: -20px;
+  margin-left: -20px;
+  border: 20px solid red;
+  width: 200px;
+  height: 200px;
+  background: green;
+}
+</style>
+<div class="columns">
+  <div class="clip">
+    <div class="composited"></div>
+  </div>
+  <div class="clip"></div>
+</div>