Improve paint invalidation handling around throttling

This avoids over-invalidation when render throttling status changes.
Rather than unconditionally invalidating the frame contents, with
this CL we simply postpone propagating invalidation bits up to the
parent document until the frame becomes unthrottled.

Change-Id: Id510939429f40547a4e8a7e6df1c66a0f0869fad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506298Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Stefan Zager <szager@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829105}

third_party/blink/renderer/core/frame/local_frame_view.cc

@@ -2742,17 +2742,26 @@ bool LocalFrameView::RunPrePaintLifecyclePhase(
 
   ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
     frame_view.Lifecycle().AdvanceTo(DocumentLifecycle::kInPrePaint);
-    if (frame_view.CanThrottleRendering()) {
-      // This frame can be throttled but not throttled, meaning we are not in an
-      // AllowThrottlingScope. Now this frame may contain dirty paint flags, and
-      // we need to propagate the flags into the ancestor chain so that
-      // PrePaintTreeWalk can reach this frame.
-      frame_view.SetNeedsPaintPropertyUpdate();
-      // We may record more pre-composited layers under the frame.
-      if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
-        frame_view.SetPaintArtifactCompositorNeedsUpdate();
-      if (auto* owner = frame_view.GetLayoutEmbeddedContent())
-        owner->SetShouldCheckForPaintInvalidation();
+    if (frame_view.pre_paint_skipped_while_throttled_ ||
+        frame_view.lifecycle_updates_throttled_) {
+      // We skipped pre-paint for this frame while it was throttled, or we
+      // have never run pre-paint for this frame. Either way, we're
+      // unthrottled now, so we must propagate our dirty bits into our
+      // parent frame so that pre-paint reaches into this frame.
+      if (LayoutView* layout_view = frame_view.GetLayoutView()) {
+        if (auto* owner = frame_view.GetFrame().OwnerLayoutObject()) {
+          if (layout_view->NeedsPaintPropertyUpdate() ||
+              layout_view->DescendantNeedsPaintPropertyUpdate()) {
+            owner->SetDescendantNeedsPaintPropertyUpdate();
+            // We may record more pre-composited layers under the frame.
+            if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
+              frame_view.SetPaintArtifactCompositorNeedsUpdate();
+          }
+          if (layout_view->ShouldCheckForPaintInvalidation())
+            owner->SetShouldCheckForPaintInvalidation();
+        }
+      }
+      frame_view.pre_paint_skipped_while_throttled_ = false;
     }
   });
 
@@ -4313,10 +4322,6 @@ void LocalFrameView::CrossOriginToMainFrameChanged() {
     return;
   }
   RenderThrottlingStatusChanged();
-  // We need to invalidate unconditionally, so if it didn't happen during
-  // RenderThrottlingStatusChanged, do it now.
-  if (CanThrottleRendering())
-    InvalidateForThrottlingChange();
   // Immediately propagate changes to children.
   UpdateRenderThrottlingStatus(IsHiddenForThrottling(), IsSubtreeThrottled(),
                                true);
@@ -4355,7 +4360,12 @@ void LocalFrameView::RenderThrottlingStatusChanged() {
     SetPaintArtifactCompositorNeedsUpdate();
 
   if (!CanThrottleRendering()) {
-    InvalidateForThrottlingChange();
+    // Start ticking animation frames again if necessary.
+    if (GetPage())
+      GetPage()->Animator().ScheduleVisualUpdate(frame_.Get());
+    // Ensure we'll recompute viewport intersection for the frame subtree during
+    // the scheduled visual update.
+    SetIntersectionObservationState(kRequired);
   } else if (GetFrame().IsLocalRoot()) {
     // By this point, every frame in the local frame tree has become throttled,
     // so painting the tree should just clear the previous painted output.
@@ -4374,26 +4384,6 @@ void LocalFrameView::RenderThrottlingStatusChanged() {
 #endif
 }
 
-void LocalFrameView::InvalidateForThrottlingChange() {
-  // Start ticking animation frames again if necessary.
-  if (GetPage())
-    GetPage()->Animator().ScheduleVisualUpdate(frame_.Get());
-  // Force a full repaint of this frame to ensure we are not left with a
-  // partially painted version of this frame's contents if we skipped
-  // painting them while the frame was throttled.
-  LayoutView* layout_view = GetLayoutView();
-  if (layout_view) {
-    layout_view->InvalidatePaintForViewAndDescendants();
-    // Also need to update all paint properties that might be skipped while
-    // the frame was throttled.
-    layout_view->AddSubtreePaintPropertyUpdateReason(
-        SubtreePaintPropertyUpdateReason::kPreviouslySkipped);
-  }
-  // Ensure we'll recompute viewport intersection for the frame subtree during
-  // the scheduled visual update.
-  SetIntersectionObservationState(kRequired);
-}
-
 void LocalFrameView::SetNeedsForcedCompositingUpdate() {
   needs_forced_compositing_update_ = true;
   if (LocalFrameView* parent = ParentFrameView())

third_party/blink/renderer/core/frame/local_frame_view.h

@@ -618,6 +618,10 @@ class CORE_EXPORT LocalFrameView final
                                     bool subtree_throttled,
                                     bool recurse = false) override;
 
+  void SetPrePaintSkippedWhileThrottled() {
+    pre_paint_skipped_while_throttled_ = true;
+  }
+
   void BeginLifecycleUpdates();
 
   // Records a timestamp in PaintTiming when the frame is first not
@@ -907,8 +911,6 @@ class CORE_EXPORT LocalFrameView final
   DoublePoint ConvertFromContainingEmbeddedContentView(
       const DoublePoint&) const;
 
-  void InvalidateForThrottlingChange();
-
   void UpdateGeometriesIfNeeded();
   bool WasViewportResized();
   void SendResizeEventIfNeeded();
@@ -1138,6 +1140,8 @@ class CORE_EXPORT LocalFrameView final
 
   HeapHashSet<WeakMember<HTMLVideoElement>> fullscreen_video_elements_;
 
+  bool pre_paint_skipped_while_throttled_ = false;
+
   std::unique_ptr<OverlayInterstitialAdDetector>
       overlay_interstitial_ad_detector_;
 

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

@@ -3615,7 +3615,13 @@ void LayoutObject::SetNeedsPaintPropertyUpdatePreservingCachedRects() {
   GetFrameView()->SetIntersectionObservationState(LocalFrameView::kDesired);
 
   bitfields_.SetNeedsPaintPropertyUpdate(true);
-  for (auto* ancestor = ParentCrossingFrames();
+  if (auto* ancestor = ParentCrossingFrames())
+    ancestor->SetDescendantNeedsPaintPropertyUpdate();
+}
+
+void LayoutObject::SetDescendantNeedsPaintPropertyUpdate() {
+  NOT_DESTROYED();
+  for (auto* ancestor = this;
        ancestor && !ancestor->DescendantNeedsPaintPropertyUpdate();
        ancestor = ancestor->ParentCrossingFrames()) {
     ancestor->bitfields_.SetDescendantNeedsPaintPropertyUpdate(true);

third_party/blink/renderer/core/layout/layout_object.h

@@ -3139,6 +3139,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
   // descendant needing a paint property update too.
   void SetNeedsPaintPropertyUpdate();
   void SetNeedsPaintPropertyUpdatePreservingCachedRects();
+  void SetDescendantNeedsPaintPropertyUpdate();
   bool NeedsPaintPropertyUpdate() const {
     NOT_DESTROYED();
     return bitfields_.NeedsPaintPropertyUpdate();

third_party/blink/renderer/core/page/page_animator.cc

@@ -140,13 +140,12 @@ void PageAnimator::UpdateAllLifecyclePhases(LocalFrame& root_frame,
   view->UpdateAllLifecyclePhases(reason);
 }
 
-void PageAnimator::UpdateAllLifecyclePhasesExceptPaint(
-    LocalFrame& root_frame,
-    DocumentUpdateReason reason) {
+void PageAnimator::UpdateLifecycleToPrePaintClean(LocalFrame& root_frame,
+                                                  DocumentUpdateReason reason) {
   LocalFrameView* view = root_frame.View();
   base::AutoReset<bool> servicing(&updating_layout_and_style_for_painting_,
                                   true);
-  view->UpdateAllLifecyclePhasesExceptPaint(reason);
+  view->UpdateLifecycleToPrePaintClean(reason);
 }
 
 void PageAnimator::UpdateLifecycleToLayoutClean(LocalFrame& root_frame,

third_party/blink/renderer/core/page/page_animator.h

@@ -43,8 +43,8 @@ class CORE_EXPORT PageAnimator final : public GarbageCollected<PageAnimator> {
   // See documents of methods with the same names in LocalFrameView class.
   void UpdateAllLifecyclePhases(LocalFrame& root_frame,
                                 DocumentUpdateReason reason);
-  void UpdateAllLifecyclePhasesExceptPaint(LocalFrame& root_frame,
-                                           DocumentUpdateReason reason);
+  void UpdateLifecycleToPrePaintClean(LocalFrame& root_frame,
+                                      DocumentUpdateReason reason);
   void UpdateLifecycleToLayoutClean(LocalFrame& root_frame,
                                     DocumentUpdateReason reason);
   AnimationClock& Clock() { return animation_clock_; }

third_party/blink/renderer/core/page/page_widget_delegate.cc

@@ -67,7 +67,7 @@ void PageWidgetDelegate::UpdateLifecycle(Page& page,
   if (requested_update == WebLifecycleUpdate::kLayout) {
     page.Animator().UpdateLifecycleToLayoutClean(root, reason);
   } else if (requested_update == WebLifecycleUpdate::kPrePaint) {
-    page.Animator().UpdateAllLifecyclePhasesExceptPaint(root, reason);
+    page.Animator().UpdateLifecycleToPrePaintClean(root, reason);
   } else {
     page.Animator().UpdateAllLifecyclePhases(root, reason);
   }

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

@@ -401,62 +401,71 @@ TEST_P(PaintPropertyTreeUpdateTest, BuildingStopsAtThrottledFrames) {
   auto* iframe = To<HTMLIFrameElement>(GetDocument().getElementById("iframe"));
   iframe->setAttribute(html_names::kStyleAttr, "transform: translateY(5555px)");
   UpdateAllLifecyclePhasesForTest();
-  // Ensure intersection observer notifications get delivered.
-  test::RunPendingTasks();
   EXPECT_FALSE(GetDocument().View()->IsHiddenForThrottling());
+  EXPECT_FALSE(GetDocument().View()->ShouldThrottleRenderingForTest());
   EXPECT_TRUE(ChildDocument().View()->IsHiddenForThrottling());
+  EXPECT_TRUE(ChildDocument().View()->ShouldThrottleRenderingForTest());
 
   auto* transform = GetLayoutObjectByElementId("transform");
   auto* iframe_layout_view = ChildDocument().GetLayoutView();
   auto* iframe_transform =
       ChildDocument().getElementById("iframeTransform")->GetLayoutObject();
 
-  // Invalidate properties in the iframe and ensure ancestors are marked.
+  // Invalidate properties in the iframe; invalidations will be propagated from
+  // the throttled frame into the embedding document.
   iframe_transform->SetNeedsPaintPropertyUpdate();
+  iframe_transform->SetShouldCheckForPaintInvalidation();
   EXPECT_FALSE(GetDocument().GetLayoutView()->NeedsPaintPropertyUpdate());
   EXPECT_TRUE(
       GetDocument().GetLayoutView()->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_TRUE(GetDocument().GetLayoutView()->ShouldCheckForPaintInvalidation());
   EXPECT_FALSE(transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(transform->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_FALSE(transform->ShouldCheckForPaintInvalidation());
   EXPECT_FALSE(iframe_layout_view->NeedsPaintPropertyUpdate());
   EXPECT_TRUE(iframe_layout_view->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_TRUE(iframe_layout_view->ShouldCheckForPaintInvalidation());
   EXPECT_TRUE(iframe_transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(iframe_transform->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_TRUE(iframe_transform->ShouldCheckForPaintInvalidation());
 
+  // Invalidate properties in the top document.
   transform->SetNeedsPaintPropertyUpdate();
+  EXPECT_FALSE(GetDocument().GetLayoutView()->NeedsPaintPropertyUpdate());
+  EXPECT_TRUE(
+      GetDocument().GetLayoutView()->DescendantNeedsPaintPropertyUpdate());
   EXPECT_TRUE(transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(transform->DescendantNeedsPaintPropertyUpdate());
 
-  EXPECT_FALSE(GetDocument().View()->ShouldThrottleRenderingForTest());
-  EXPECT_TRUE(ChildDocument().View()->ShouldThrottleRenderingForTest());
-
-  // A lifecycle update should update all properties except those with
-  // actively throttled descendants.
+  // A full lifecycle update with the iframe throttled will clear flags in the
+  // top document, but not in the throttled iframe. The iframe's LayoutView
+  // will be marked for paint property update because it was skipped while
+  // paint properties were updated in the embedding document.
   UpdateAllLifecyclePhasesForTest();
   EXPECT_FALSE(GetDocument().GetLayoutView()->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(
       GetDocument().GetLayoutView()->DescendantNeedsPaintPropertyUpdate());
   EXPECT_FALSE(transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(transform->DescendantNeedsPaintPropertyUpdate());
-  EXPECT_FALSE(iframe_layout_view->NeedsPaintPropertyUpdate());
+  EXPECT_TRUE(iframe_layout_view->NeedsPaintPropertyUpdate());
   EXPECT_TRUE(iframe_layout_view->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_TRUE(iframe_layout_view->ShouldCheckForPaintInvalidation());
   EXPECT_TRUE(iframe_transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(iframe_transform->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_TRUE(iframe_transform->ShouldCheckForPaintInvalidation());
 
-  EXPECT_FALSE(GetDocument().View()->ShouldThrottleRendering());
-  EXPECT_FALSE(ChildDocument().View()->ShouldThrottleRendering());
-  // Once unthrottled, a lifecycel update should update all properties.
-  GetDocument().View()->UpdateLifecycleToCompositingCleanPlusScrolling(
+  // Run a force-unthrottled lifecycle update. All flags should be cleared.
+  GetDocument().View()->UpdateLifecycleToPrePaintClean(
       DocumentUpdateReason::kTest);
-  EXPECT_FALSE(GetDocument().GetLayoutView()->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(
       GetDocument().GetLayoutView()->DescendantNeedsPaintPropertyUpdate());
-  EXPECT_FALSE(transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(transform->DescendantNeedsPaintPropertyUpdate());
   EXPECT_FALSE(iframe_layout_view->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(iframe_layout_view->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_FALSE(iframe_layout_view->ShouldCheckForPaintInvalidation());
   EXPECT_FALSE(iframe_transform->NeedsPaintPropertyUpdate());
   EXPECT_FALSE(iframe_transform->DescendantNeedsPaintPropertyUpdate());
+  EXPECT_FALSE(iframe_transform->ShouldCheckForPaintInvalidation());
 }
 
 TEST_P(PaintPropertyTreeUpdateTest, ClipChangesUpdateOverflowClip) {

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

@@ -233,11 +233,6 @@ void PrePaintTreeWalk::WalkTree(LocalFrameView& root_frame_view) {
 }
 
 void PrePaintTreeWalk::Walk(LocalFrameView& frame_view) {
-  if (frame_view.ShouldThrottleRendering()) {
-    // Skip the throttled frame. Will update it when it becomes unthrottled.
-    return;
-  }
-
   // We need to be careful not to have a reference to the parent context, since
   // this reference will be to the context_storage_ memory which may be
   // reallocated during this function call.
@@ -250,6 +245,25 @@ void PrePaintTreeWalk::Walk(LocalFrameView& frame_view) {
   bool needs_tree_builder_context_update =
       NeedsTreeBuilderContextUpdate(frame_view, parent_context());
 
+  if (frame_view.ShouldThrottleRendering()) {
+    // Skip the throttled frame, and set dirty bits that will be applied when it
+    // becomes unthrottled.
+    frame_view.SetPrePaintSkippedWhileThrottled();
+    if (LayoutView* layout_view = frame_view.GetLayoutView()) {
+      if (needs_tree_builder_context_update) {
+        layout_view->AddSubtreePaintPropertyUpdateReason(
+            SubtreePaintPropertyUpdateReason::kPreviouslySkipped);
+      }
+      if (parent_context().paint_invalidator_context.NeedsSubtreeWalk())
+        layout_view->SetSubtreeShouldDoFullPaintInvalidation();
+      if (parent_context().effective_allowed_touch_action_changed)
+        layout_view->MarkEffectiveAllowedTouchActionChanged();
+      if (parent_context().blocking_wheel_event_handler_changed)
+        layout_view->MarkBlockingWheelEventHandlerChanged();
+    }
+    return;
+  }
+
   // Note that because we're emplacing an object constructed from
   // parent_context() (which is a reference to the vector itself), it's
   // important to first ensure that there's sufficient capacity in the vector.