Let PrePaintTreeWalk handle layer invalidation of subtree of clip change

This avoids unnecessary visual rect recalculation when ancestor clip
changes when we called SetSubtreeShouldCheckForPaintInvalidation()
in the case.

Bug: 927903
Change-Id: Id55440689e9208b2c7b9590a0c2b06e70ae78160
Reviewed-on: https://chromium-review.googlesource.com/c/1484150Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636697}

third_party/blink/renderer/core/layout/layout_block.cc

@@ -292,7 +292,6 @@ void LayoutBlock::UpdateFromStyle() {
   if (should_clip_overflow != HasOverflowClip()) {
     if (!should_clip_overflow)
       GetScrollableArea()->InvalidateAllStickyConstraints();
-    SetSubtreeShouldCheckForPaintInvalidation();
     // The overflow clip paint property depends on whether overflow clip is
     // present so we need to update paint properties if this changes.
     SetNeedsPaintPropertyUpdate();

third_party/blink/renderer/core/layout/ng/ng_block_node.cc

@@ -339,13 +339,6 @@ void NGBlockNode::FinishLayout(
     }
   }
 
-  // If we have an overflow clip present, we need to check for paint
-  // invalidation for the subtree in case that the overflow clip changes.
-  // TODO(crbug.com/927903): Probably PrePaintTreeWalk is the better place to
-  // do this by invalidating descendant PaintLayers only.
-  if (box_->HasOverflowClip())
-    box_->SetSubtreeShouldCheckForPaintInvalidation();
-
   CopyFragmentDataToLayoutBox(constraint_space, *layout_result);
 }
 

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

@@ -240,20 +240,6 @@ void PrePaintTreeWalk::UpdateAuxiliaryObjectProperties(
     context.ancestor_overflow_paint_layer = paint_layer;
 }
 
-void PrePaintTreeWalk::InvalidatePaintLayerOptimizationsIfNeeded(
-    const LayoutObject& object,
-    PrePaintTreeWalkContext& context) {
-  if (!object.HasLayer())
-    return;
-
-  PaintLayer& paint_layer = *ToLayoutBoxModelObject(object).Layer();
-
-  if (!context.tree_builder_context->clip_changed)
-    return;
-
-  paint_layer.SetNeedsRepaint();
-}
-
 bool PrePaintTreeWalk::NeedsTreeBuilderContextUpdate(
     const LocalFrameView& frame_view,
     const PrePaintTreeWalkContext& context) {
@@ -342,7 +328,11 @@ void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
   if (context.tree_builder_context) {
     property_changed =
         std::max(property_changed, property_tree_builder->UpdateForChildren());
-    InvalidatePaintLayerOptimizationsIfNeeded(object, context);
+
+    // Save clip_changed flag in |context| so that all descendants will see it
+    // even if we don't creae tree_builder_context.
+    if (context.tree_builder_context->clip_changed)
+      context.clip_changed = true;
 
     if (property_changed > PaintPropertyChangedState::kUnchanged) {
       if (property_changed >
@@ -372,6 +362,11 @@ void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
     }
   }
 
+  // When this or ancestor clip changed, the layer needs repaint because it
+  // may paint more or less results according to the changed clip.
+  if (context.clip_changed && object.HasLayer())
+    ToLayoutBoxModelObject(object).Layer()->SetNeedsRepaint();
+
   CompositingLayerPropertyUpdater::Update(object);
 
   if (origin_trials::JankTrackingEnabled(&object.GetDocument())) {
@@ -405,7 +400,8 @@ void PrePaintTreeWalk::Walk(const LayoutObject& object) {
       !object.ShouldCheckForPaintInvalidation() &&
       !parent_context().paint_invalidator_context.NeedsSubtreeWalk() &&
       !NeedsEffectiveWhitelistedTouchActionUpdate(object, parent_context()) &&
-      !NeedsHitTestingPaintInvalidation(object, parent_context())) {
+      !NeedsHitTestingPaintInvalidation(object, parent_context()) &&
+      !parent_context().clip_changed) {
     return;
   }
 
@@ -424,8 +420,11 @@ void PrePaintTreeWalk::Walk(const LayoutObject& object) {
   };
 
   // Ignore clip changes from ancestor across transform boundaries.
-  if (context().tree_builder_context && object.StyleRef().HasTransform())
-    context().tree_builder_context->clip_changed = false;
+  if (object.StyleRef().HasTransform()) {
+    context().clip_changed = false;
+    if (context().tree_builder_context)
+      context().tree_builder_context->clip_changed = false;
+  }
 
   WalkInternal(object, context());
 

third_party/blink/renderer/core/paint/pre_paint_tree_walk.h

@@ -71,6 +71,11 @@ class CORE_EXPORT PrePaintTreeWalk {
     // When the effective whitelisted touch action changes on an ancestor, the
     // entire subtree may need to update.
     bool effective_whitelisted_touch_action_changed = false;
+
+    // This is set to true once we see tree_builder_context->clip_changed is
+    // true. It will be propagated to descendant contexts even if we don't
+    // create tree_builder_context.
+    bool clip_changed = false;
   };
 
   const PrePaintTreeWalkContext& ContextAt(wtf_size_t index) {
@@ -88,11 +93,6 @@ class CORE_EXPORT PrePaintTreeWalk {
   NOINLINE void WalkInternal(const LayoutObject&, PrePaintTreeWalkContext&);
   void Walk(const LayoutObject&);
 
-  // Invalidates paint-layer painting optimizations, such as subsequence caching
-  // and empty paint phase optimizations if clips from the context have changed.
-  void InvalidatePaintLayerOptimizationsIfNeeded(const LayoutObject&,
-                                                 PrePaintTreeWalkContext&);
-
   bool NeedsTreeBuilderContextUpdate(const LocalFrameView&,
                                      const PrePaintTreeWalkContext&);
   bool NeedsTreeBuilderContextUpdate(const LayoutObject&,