Make sure lifecycle updates proceed enough for IntersectionObserver

IntersectionObserver may require sticky position information, so if
a lifecycle update is forced by IntersectionObserver, make sure it
proceeds far enough to update sticky position.

As noted in the code comments, even if sticky position update is
moved up to the layout phase (which is planned, I think), if the
document is servicing any IntersectionObservers with
track_visibility=true (i.e. V2 features), then we will still need to
force the lifecycle update to continue far enough to enable hit
testing. That's the root cause of the bug.

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

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

@@ -257,7 +257,6 @@ LocalFrameView::LocalFrameView(LocalFrame& frame, IntRect frame_rect)
       // doesn't get setup properly.
       lifecycle_updates_throttled_(!GetFrame().IsMainFrame()),
       target_state_(DocumentLifecycle::kUninitialized),
-      past_layout_lifecycle_update_(false),
       suppress_adjust_view_size_(false),
       intersection_observation_state_(kNotNeeded),
       needs_forced_compositing_update_(false),
@@ -2360,6 +2359,18 @@ bool LocalFrameView::LocalFrameTreeAllowsThrottling() const {
   return false;
 }
 
+void LocalFrameView::PrepareForLifecycleUpdateRecursive() {
+  // We will run lifecycle phases for LocalFrameViews that are unthrottled; or
+  // are throttled but require IntersectionObserver steps to run.
+  if (!ShouldThrottleRendering() ||
+      intersection_observation_state_ == kRequired) {
+    Lifecycle().EnsureStateAtMost(DocumentLifecycle::kVisualUpdatePending);
+    ForAllChildLocalFrameViews([](LocalFrameView& child) {
+      child.PrepareForLifecycleUpdateRecursive();
+    });
+  }
+}
+
 // TODO(leviw): We don't assert lifecycle information from documents in child
 // WebPluginContainerImpls.
 bool LocalFrameView::UpdateLifecyclePhases(
@@ -2406,35 +2417,27 @@ bool LocalFrameView::UpdateLifecyclePhases(
   if (frame_->IsLocalRoot())
     UpdateLayerDebugInfoEnabled();
 
+  // If we're throttling and we aren't required to run the IntersectionObserver
+  // steps, then we don't need to update lifecycle phases. The throttling status
+  // will get updated in RunPostLifecycleSteps().
+  if (ShouldThrottleRendering() &&
+      intersection_observation_state_ < kRequired) {
+    return Lifecycle().GetState() == target_state;
+  }
+
+  PrepareForLifecycleUpdateRecursive();
+
   // This is used to guard against reentrance. It is also used in conjunction
   // with the current lifecycle state to determine which phases are yet to run
-  // in this cycle.
+  // in this cycle. Note that this may change the return value of
+  // ShouldThrottleRendering(), hence it cannot be moved before the preceeding
+  // code, which relies on the prior value of ShouldThrottleRendering().
   base::AutoReset<DocumentLifecycle::LifecycleState> target_state_scope(
       &target_state_, target_state);
-  // This is used to check if we're within a lifecycle update but have passed
-  // the layout update phase. Note there is a bit of a subtlety here: it's not
-  // sufficient for us to check the current lifecycle state, since it can be
-  // past kLayoutClean but the function to run style and layout phase has not
-  // actually been run yet. Since this bool affects throttling, and throttling,
-  // in turn, determines whether style and layout function will run, we need a
-  // separate bool.
-  base::AutoReset<bool> past_layout_lifecycle_resetter(
-      &past_layout_lifecycle_update_, false);
-
-  // If we're throttling, then we don't need to update lifecycle phases. The
-  // throttling status will get updated in RunPostLifecycleSteps().
-  if (ShouldThrottleRendering()) {
-    return Lifecycle().GetState() == target_state;
-  }
 
   lifecycle_data_.start_time = base::TimeTicks::Now();
   ++lifecycle_data_.count;
 
-  ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
-    frame_view.Lifecycle().EnsureStateAtMost(
-        DocumentLifecycle::kVisualUpdatePending);
-  });
-
   if (target_state == DocumentLifecycle::kPaintClean) {
     {
       TRACE_EVENT0("blink", "LocalFrameView::WillStartLifecycleUpdate");
@@ -2669,17 +2672,6 @@ bool LocalFrameView::RunStyleAndLayoutLifecyclePhases(
   if (target_state == DocumentLifecycle::kLayoutClean)
     return false;
 
-  // This will be reset by AutoReset in the calling function
-  past_layout_lifecycle_update_ = true;
-
-  // After layout and the |past_layout_lifecycle_update_| update, the value of
-  // ShouldThrottleRendering() can change. OOPIF local frame roots that are
-  // throttled can return now that layout is clean. This situation happens if
-  // the throttling was disabled due to required intersection observation, which
-  // can now be run.
-  if (ShouldThrottleRendering())
-    return false;
-
   // Now we can run post layout steps in preparation for further phases.
   ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
     frame_view.PerformScrollAnchoringAdjustments();
@@ -4534,7 +4526,20 @@ bool LocalFrameView::ShouldThrottleRendering() const {
   auto* local_frame_root_view = GetFrame().LocalFrameRoot().View();
   if (local_frame_root_view->IsUpdatingLifecycle() &&
       intersection_observation_state_ == kRequired && !IsDisplayLocked()) {
-    return local_frame_root_view->past_layout_lifecycle_update_;
+    // IntersectionObserver may rely on sticky positioning, which is not
+    // resolved until compositing assignments are done. If the resolution of
+    // sticky positioning is refactored to occur during layout, then the
+    // required minimum lifecycle state will depend on whether this frame
+    // requires occlusion tracking, since determining occlusion state requires a
+    // hit test, which requires the document to be pre-paint clean.  So the code
+    // here would change to something like:
+    //
+    //   auto min_state = GetFrame().NeedsOcclusionTracking() ?
+    //       DocumentLifecycle::kPrePaintClean :
+    //       DocumentLifecycle::kLayoutClean;
+    //   return Lifecycle().GetState() >= min_state;
+    return Lifecycle().GetState() >=
+           DocumentLifecycle::kCompositingAssignmentsClean;
   }
 
   return true;

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

@@ -854,6 +854,8 @@ class CORE_EXPORT LocalFrameView final
 
   LayoutSVGRoot* EmbeddedReplacedContent() const;
 
+  void PrepareForLifecycleUpdateRecursive();
+
   // Returns whether the lifecycle was successfully updated to the
   // target state.
   bool UpdateLifecyclePhases(DocumentLifecycle::LifecycleState target_state,
@@ -1075,7 +1077,6 @@ class CORE_EXPORT LocalFrameView final
 
   // This is set on the local root frame view only.
   DocumentLifecycle::LifecycleState target_state_;
-  bool past_layout_lifecycle_update_;
 
   using AnchoringAdjustmentQueue =
       HeapLinkedHashSet<WeakMember<ScrollableArea>>;

third_party/blink/renderer/core/scheduler_integration_tests/frame_throttling_test.cc

@@ -199,6 +199,8 @@ TEST_P(FrameThrottlingTest, IntersectionObservationOverridesThrottling) {
   {
     GetDocument().GetFrame()->View()->SetTargetStateForTest(
         DocumentLifecycle::kPaintClean);
+    inner_frame_document->Lifecycle().EnsureStateAtMost(
+        DocumentLifecycle::kVisualUpdatePending);
     EXPECT_FALSE(
         inner_frame_document->View()->ShouldThrottleRenderingForTest());
     GetDocument().GetFrame()->View()->SetTargetStateForTest(
@@ -233,11 +235,12 @@ TEST_P(FrameThrottlingTest, IntersectionObservationOverridesThrottling) {
   EXPECT_TRUE(inner_frame_document->View()->ShouldThrottleRenderingForTest());
 
   EXPECT_FALSE(inner_view->NeedsLayout());
-  EXPECT_TRUE(inner_frame_document->View()
-                  ->GetLayoutView()
-                  ->ShouldDoFullPaintInvalidation());
+  EXPECT_LT(inner_frame_document->Lifecycle().GetState(),
+            DocumentLifecycle::kPaintClean);
   if (!RuntimeEnabledFeatures::CompositeAfterPaintEnabled()) {
-    EXPECT_EQ(kCompositingUpdateRebuildTree,
+    // If IntersectionObserver is required to run, lifecycle will be updated
+    // through compositing.
+    EXPECT_EQ(kCompositingUpdateNone,
               inner_view->Compositor()->pending_update_type_);
   }
   EXPECT_TRUE(inner_view->Layer()->SelfNeedsRepaint());
@@ -357,6 +360,8 @@ TEST_P(FrameThrottlingTest,
   {
     GetDocument().GetFrame()->View()->SetTargetStateForTest(
         DocumentLifecycle::kPaintClean);
+    frame_document->Lifecycle().EnsureStateAtMost(
+        DocumentLifecycle::kVisualUpdatePending);
     EXPECT_FALSE(frame_document->View()->ShouldThrottleRenderingForTest());
     GetDocument().GetFrame()->View()->SetTargetStateForTest(
         DocumentLifecycle::kUninitialized);
@@ -369,7 +374,7 @@ TEST_P(FrameThrottlingTest,
   frame_document->View()->ScheduleAnimation();
   frame_document->View()->GetLayoutView()->Layer()->SetNeedsRepaint();
   CompositeFrame();
-  EXPECT_EQ(DocumentLifecycle::kLayoutClean,
+  EXPECT_EQ(DocumentLifecycle::kCompositingAssignmentsClean,
             frame_document->Lifecycle().GetState());
   EXPECT_TRUE(frame_document->View()->ShouldThrottleRenderingForTest());
 }