Revert "Make sure lifecycle updates proceed enough for IntersectionObserver"

This reverts commit 645c8058.

Reason for revert: Appears to have broken PhysicsBasedFlingCurveBrowserTest.TargetScrollOffsetForFlingAnimation, see crbug.com/1158664

Original change's description:
> 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}

TBR=szager@chromium.org,wangxianzhu@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: Ic75ec8f9d9b3d075c16f26718df781d377676bc7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1156937, 1158664
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593096Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837051}

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

@@ -257,6 +257,7 @@ 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),
@@ -2359,18 +2360,6 @@ 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(
@@ -2417,27 +2406,35 @@ 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. 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().
+  // in this cycle.
   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");
@@ -2672,6 +2669,17 @@ 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();
@@ -4526,20 +4534,7 @@ bool LocalFrameView::ShouldThrottleRendering() const {
   auto* local_frame_root_view = GetFrame().LocalFrameRoot().View();
   if (local_frame_root_view->IsUpdatingLifecycle() &&
       intersection_observation_state_ == kRequired && !IsDisplayLocked()) {
-    // 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 local_frame_root_view->past_layout_lifecycle_update_;
   }
 
   return true;

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

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