Reland "[CompositeAfterPaint] Invalidate chrome client when needed"

This is a reland of crrev.com/618316 which was reverted in
crrev.com/619326 because of performance regression. This patch
changed the condition in PrePaintTreeWalk::WalkTree() from
 if (root_frame_view.GetLayoutView()->Layer()->NeedsRepaint())
to
 if (needs_invalidate_chrome_client_).
needs_invalidate_chrome_client_ is set by PaintInvalidator.

The problem of previous code was that NeedsRepaint() is always
true if the document doesn't update lifecycle to paint which is
the case for the subdocument of SVGImage, causing infinite
invalidation of the chrome client.

Original change's description:
> Revert "[CompositeAfterPaint] Invalidate chrome client when needed"
>
> This reverts commit 29021ede.
>
> Reason for revert: Suspect performance regression
>
> Bug: 918276
>
> Original change's description:
> > [CompositeAfterPaint] Invalidate chrome client when needed
> >
> > For pre-CompositeAfterPaint, we call InvalidateChromeClient()
> > when an object is invalidated in a view referenced from a plugin or
> > an svg-image.
> >
> > Now let the path also work for CompositeAfterPaint.
> >
> > This fixes several web plugin tests for CompositeAfterPaint.
> >
> > Bug: 524134
> > Change-Id: Ia4c3ccba4632ddcdfdfdfec299fb7a68440a1419
> > Reviewed-on: https://chromium-review.googlesource.com/c/1385112
> > Reviewed-by: Philip Rogers <pdr@chromium.org>
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#618316}
>
> TBR=wangxianzhu@chromium.org,pdr@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 524134
> Change-Id: I9b4f67a74732919ba23acc00fd501d45baa498e6
> Reviewed-on: https://chromium-review.googlesource.com/c/1392439
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#619326}

Bug: 918276, 524134
Change-Id: Ifad1c2eb8bdddcceea26a286c3309665f6d94c1a
Reviewed-on: https://chromium-review.googlesource.com/c/1393558Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619733}

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

@@ -1075,9 +1075,7 @@ TEST_F(PaintInvalidatorCustomClientTest,
        NonCompositedInvalidationChangeOpacity) {
   // This test runs in a non-composited mode, so invalidations should
   // be issued via InvalidateChromeClient.
-  SetBodyInnerHTML(R"HTML(
-    <div id=target style="opacity: 0.99"></div>
-    )HTML");
+  SetBodyInnerHTML("<div id=target style='opacity: 0.99'></div>");
 
   auto* target = GetDocument().getElementById("target");
   ASSERT_TRUE(target);
@@ -1090,4 +1088,26 @@ TEST_F(PaintInvalidatorCustomClientTest,
   EXPECT_TRUE(InvalidationRecorded());
 }
 
+TEST_F(PaintInvalidatorCustomClientTest,
+       NoInvalidationRepeatedUpdateLifecyleExceptPaint) {
+  SetBodyInnerHTML("<div id=target style='opacity: 0.99'></div>");
+
+  auto* target = GetDocument().getElementById("target");
+  ASSERT_TRUE(target);
+  ResetInvalidationRecorded();
+
+  target->setAttribute(html_names::kStyleAttr, "opacity: 0.98");
+  GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint();
+  EXPECT_TRUE(GetDocument().View()->GetLayoutView()->Layer()->NeedsRepaint());
+  EXPECT_TRUE(InvalidationRecorded());
+
+  ResetInvalidationRecorded();
+  // Let PrePaintTreeWalk do something instead of no-op.
+  GetDocument().View()->SetNeedsPaintPropertyUpdate();
+  GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint();
+  // The layer NeedsRepaint flag is only cleared after paint.
+  EXPECT_TRUE(GetDocument().View()->GetLayoutView()->Layer()->NeedsRepaint());
+  EXPECT_FALSE(InvalidationRecorded());
+}
+
 }  // namespace blink

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

@@ -16,7 +16,6 @@
 #include "third_party/blink/renderer/core/layout/ng/geometry/ng_physical_offset_rect.h"
 #include "third_party/blink/renderer/core/layout/ng/legacy_layout_tree_walking.h"
 #include "third_party/blink/renderer/core/layout/svg/svg_layout_support.h"
-#include "third_party/blink/renderer/core/page/chrome_client.h"
 #include "third_party/blink/renderer/core/paint/clip_path_clipper.h"
 #include "third_party/blink/renderer/core/paint/find_paint_offset_and_visual_rect_needing_update.h"
 #include "third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h"
@@ -390,22 +389,6 @@ void PaintInvalidator::InvalidatePaint(
   }
 }
 
-static void InvalidateChromeClient(
-    const LayoutBoxModelObject& paint_invalidation_container) {
-  if (paint_invalidation_container.GetDocument().Printing() &&
-      !RuntimeEnabledFeatures::PrintBrowserEnabled())
-    return;
-
-  DCHECK(paint_invalidation_container.IsLayoutView());
-  DCHECK(!paint_invalidation_container.IsPaintInvalidationContainer());
-
-  auto* frame_view = paint_invalidation_container.GetFrameView();
-  DCHECK(!frame_view->GetFrame().OwnerLayoutObject());
-  if (auto* client = frame_view->GetChromeClient()) {
-    client->InvalidateRect(IntRect(IntPoint(), frame_view->Size()));
-  }
-}
-
 void PaintInvalidator::UpdateEmptyVisualRectFlag(
     const LayoutObject& object,
     PaintInvalidatorContext& context) {
@@ -428,7 +411,7 @@ void PaintInvalidator::UpdateEmptyVisualRectFlag(
   }
 }
 
-void PaintInvalidator::InvalidatePaint(
+bool PaintInvalidator::InvalidatePaint(
     const LayoutObject& object,
     const PaintPropertyTreeBuilderContext* tree_builder_context,
     PaintInvalidatorContext& context) {
@@ -436,11 +419,11 @@ void PaintInvalidator::InvalidatePaint(
                "PaintInvalidator::InvalidatePaint()", "object",
                object.DebugName().Ascii());
 
-  if (object.IsSVGHiddenContainer()) {
+  if (object.IsSVGHiddenContainer())
     context.subtree_flags |= PaintInvalidatorContext::kSubtreeNoInvalidation;
-  }
+
   if (context.subtree_flags & PaintInvalidatorContext::kSubtreeNoInvalidation)
-    return;
+    return false;
 
   object.GetMutableForPainting().EnsureIsReadyForPaintInvalidation();
 
@@ -460,7 +443,7 @@ void PaintInvalidator::InvalidatePaint(
   UpdateEmptyVisualRectFlag(object, context);
 
   if (!object.ShouldCheckForPaintInvalidation() && !context.NeedsSubtreeWalk())
-    return;
+    return false;
 
   unsigned tree_builder_index = 0;
 
@@ -532,14 +515,7 @@ void PaintInvalidator::InvalidatePaint(
         PaintInvalidatorContext::kSubtreeInvalidationChecking;
   }
 
-  // The object is under a frame for WebViewPlugin, SVG images etc. Need to
-  // inform the chrome client of the invalidation so that the client will
-  // initiate painting of the contents.
-  // TODO(wangxianzhu): Do we need this for CAP?
-  if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled() &&
-      !context.paint_invalidation_container->IsPaintInvalidationContainer() &&
-      reason != PaintInvalidationReason::kNone)
-    InvalidateChromeClient(*context.paint_invalidation_container);
+  return reason != PaintInvalidationReason::kNone;
 }
 
 void PaintInvalidator::ProcessPendingDelayedPaintInvalidations() {

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

@@ -141,7 +141,8 @@ class PaintInvalidator {
   void InvalidatePaint(LocalFrameView&,
                        const PaintPropertyTreeBuilderContext*,
                        PaintInvalidatorContext&);
-  void InvalidatePaint(const LayoutObject&,
+  // Returns true if the object is invalidated.
+  bool InvalidatePaint(const LayoutObject&,
                        const PaintPropertyTreeBuilderContext*,
                        PaintInvalidatorContext&);
 

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

@@ -16,6 +16,7 @@
 #include "third_party/blink/renderer/core/layout/layout_embedded_content.h"
 #include "third_party/blink/renderer/core/layout/layout_multi_column_spanner_placeholder.h"
 #include "third_party/blink/renderer/core/layout/layout_view.h"
+#include "third_party/blink/renderer/core/page/chrome_client.h"
 #include "third_party/blink/renderer/core/page/page.h"
 #include "third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.h"
 #include "third_party/blink/renderer/core/paint/compositing/compositing_layer_property_updater.h"
@@ -68,6 +69,13 @@ void PrePaintTreeWalk::WalkTree(LocalFrameView& root_frame_view) {
   if (VLOG_IS_ON(1))
     showAllPropertyTrees(root_frame_view);
 #endif
+
+  // If the frame is invalidated, we need to inform the frame's chrome client
+  // so that the client will initiate repaint of the contents.
+  if (needs_invalidate_chrome_client_) {
+    if (auto* client = root_frame_view.GetChromeClient())
+      client->InvalidateRect(IntRect(IntPoint(), root_frame_view.Size()));
+  }
 }
 
 void PrePaintTreeWalk::Walk(LocalFrameView& frame_view) {
@@ -339,9 +347,10 @@ void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
   // depends on the effective whitelisted touch action.
   UpdateEffectiveWhitelistedTouchAction(object, context);
 
-  paint_invalidator_.InvalidatePaint(
-      object, base::OptionalOrNullptr(context.tree_builder_context),
-      paint_invalidator_context);
+  if (paint_invalidator_.InvalidatePaint(
+          object, base::OptionalOrNullptr(context.tree_builder_context),
+          paint_invalidator_context))
+    needs_invalidate_chrome_client_ = true;
 
   InvalidatePaintForHitTesting(object, context);
 

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

@@ -115,6 +115,8 @@ class CORE_EXPORT PrePaintTreeWalk {
   PaintInvalidator paint_invalidator_;
   Vector<PrePaintTreeWalkContext> context_storage_;
 
+  bool needs_invalidate_chrome_client_ = false;
+
   FRIEND_TEST_ALL_PREFIXES(PrePaintTreeWalkTest, ClipRects);
 };
 

third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint

@@ -7,7 +7,6 @@ Bug(none) editing/input [ Skip ]
 Bug(none) external/wpt [ Skip ]
 Bug(none) http/tests/devtools/layers/ [ Skip ]
 Bug(none) http/tests/devtools/tracing/ [ Skip ]
-Bug(none) plugins/ [ Skip ]
 Bug(none) printing/ [ Skip ]
 Bug(none) scrollbars/ [ Skip ]
 Bug(none) scrollingcoordinator/ [ Skip ]