ContentVisibility: Make the lifecycle tasks run only on full lifecycle.

This patch changes the behavior of willstart- an didfinish- lifecycle
callbacks, as well as when the start of lifecycle tasks are run.

Previously, they would run on any call to the lifecycle update.
However, the intent of this is to document when the main frame
lifecycle runs (ie to PaintClean).

R=szager@chromium.org, chrishtr@chromium.org

Fixed: 1121965
Change-Id: I83bfa7142ca3fc9a94adbd0e5dfbe0245da3c9be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386222Reviewed-by: Stefan Zager <szager@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803637}

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

@@ -2395,34 +2395,39 @@ bool LocalFrameView::UpdateLifecyclePhases(
   lifecycle_data_.start_time = base::TimeTicks::Now();
   ++lifecycle_data_.count;
 
-  {
-    TRACE_EVENT0("blink", "LocalFrameView::WillStartLifecycleUpdate");
+  ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
+    frame_view.Lifecycle().EnsureStateAtMost(
+        DocumentLifecycle::kVisualUpdatePending);
+  });
 
-    ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
-      frame_view.Lifecycle().EnsureStateAtMost(
-          DocumentLifecycle::kVisualUpdatePending);
-      auto lifecycle_observers = frame_view.lifecycle_observers_;
-      for (auto& observer : lifecycle_observers)
-        observer->WillStartLifecycleUpdate(frame_view);
-    });
-  }
+  if (target_state == DocumentLifecycle::kPaintClean) {
+    {
+      TRACE_EVENT0("blink", "LocalFrameView::WillStartLifecycleUpdate");
 
-  {
-    TRACE_EVENT0(
-        "blink",
-        "LocalFrameView::UpdateLifecyclePhases - start of lifecycle tasks");
-    ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
-      WTF::Vector<base::OnceClosure> tasks;
-      frame_view.start_of_lifecycle_tasks_.swap(tasks);
-      for (auto& task : tasks)
-        std::move(task).Run();
-    });
+      ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
+        auto lifecycle_observers = frame_view.lifecycle_observers_;
+        for (auto& observer : lifecycle_observers)
+          observer->WillStartLifecycleUpdate(frame_view);
+      });
+    }
+
+    {
+      TRACE_EVENT0(
+          "blink",
+          "LocalFrameView::UpdateLifecyclePhases - start of lifecycle tasks");
+      ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
+        WTF::Vector<base::OnceClosure> tasks;
+        frame_view.start_of_lifecycle_tasks_.swap(tasks);
+        for (auto& task : tasks)
+          std::move(task).Run();
+      });
+    }
   }
 
   // Run the lifecycle updates.
   UpdateLifecyclePhasesInternal(target_state);
 
-  {
+  if (target_state == DocumentLifecycle::kPaintClean) {
     TRACE_EVENT0("blink", "LocalFrameView::DidFinishLifecycleUpdate");
 
     ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {

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

@@ -557,5 +557,88 @@ TEST_F(SimTest, NestedCrossOriginPaintEligibility) {
   EXPECT_TRUE(inner_frame_timing.FirstEligibleToPaint().is_null());
 }
 
+class TestLifecycleObserver
+    : public GarbageCollected<TestLifecycleObserver>,
+      public LocalFrameView::LifecycleNotificationObserver {
+ public:
+  TestLifecycleObserver() = default;
+
+  void WillStartLifecycleUpdate(const LocalFrameView&) override {
+    ++will_start_lifecycle_count_;
+  }
+  void DidFinishLifecycleUpdate(const LocalFrameView&) override {
+    ++did_finish_lifecycle_count_;
+  }
+
+  int will_start_lifecycle_count() const { return will_start_lifecycle_count_; }
+  int did_finish_lifecycle_count() const { return did_finish_lifecycle_count_; }
+
+  // GC functions.
+  void Trace(Visitor*) const override {}
+
+ private:
+  int will_start_lifecycle_count_ = 0;
+  int did_finish_lifecycle_count_ = 0;
+};
+
+TEST_F(LocalFrameViewTest, LifecycleNotificationsOnlyOnFullLifecycle) {
+  SetBodyInnerHTML("<div></div>");
+  auto* frame_view = GetDocument().View();
+
+  auto* observer = MakeGarbageCollected<TestLifecycleObserver>();
+  frame_view->RegisterForLifecycleNotifications(observer);
+
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 0);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 0);
+
+  frame_view->UpdateAllLifecyclePhasesExceptPaint(DocumentUpdateReason::kTest);
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 0);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 0);
+
+  frame_view->UpdateLifecyclePhasesForPrinting();
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 0);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 0);
+
+  frame_view->UpdateAllLifecyclePhases(DocumentUpdateReason::kTest);
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 1);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 1);
+
+  frame_view->UpdateAllLifecyclePhases(DocumentUpdateReason::kTest);
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 2);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 2);
+
+  frame_view->UnregisterFromLifecycleNotifications(observer);
+  frame_view->UpdateAllLifecyclePhases(DocumentUpdateReason::kTest);
+  EXPECT_EQ(observer->will_start_lifecycle_count(), 2);
+  EXPECT_EQ(observer->did_finish_lifecycle_count(), 2);
+}
+
+TEST_F(LocalFrameViewTest, StartOfLifecycleTaskRunsOnFullLifecycle) {
+  SetBodyInnerHTML("<div></div>");
+  auto* frame_view = GetDocument().View();
+
+  struct TestCallback {
+    void Increment() { ++calls; }
+    int calls = 0;
+  };
+
+  TestCallback callback;
+
+  frame_view->EnqueueStartOfLifecycleTask(
+      base::BindOnce(&TestCallback::Increment, base::Unretained(&callback)));
+  EXPECT_EQ(callback.calls, 0);
+
+  frame_view->UpdateAllLifecyclePhasesExceptPaint(DocumentUpdateReason::kTest);
+  EXPECT_EQ(callback.calls, 0);
+
+  frame_view->UpdateLifecyclePhasesForPrinting();
+  EXPECT_EQ(callback.calls, 0);
+
+  frame_view->UpdateAllLifecyclePhases(DocumentUpdateReason::kTest);
+  EXPECT_EQ(callback.calls, 1);
+
+  frame_view->UpdateAllLifecyclePhases(DocumentUpdateReason::kTest);
+  EXPECT_EQ(callback.calls, 1);
+}
 }  // namespace
 }  // namespace blink

third_party/blink/web_tests/TestExpectations

@@ -1025,7 +1025,6 @@ crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-break/widows-
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-contain/contain-size-breaks-001.html [ Crash Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-contain/contain-size-monolithic-001.html [ Crash Failure ]
 crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-contain/contain-size-multicol-001.html [ Failure ]
-crbug.com/1121965 virtual/layout_ng_block_frag/external/wpt/css/css-contain/content-visibility/content-visibility-026.html [ Failure Pass ]
 crbug.com/1079031 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/change-out-of-flow-type-and-remove-inner-multicol-crash.html [ Crash Failure ]
 crbug.com/1058792 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/composited-under-clip-under-multicol.html [ Failure Crash ]
 crbug.com/996655 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/going-out-of-flow-after-spanner.html [ Crash Failure Pass ]

third_party/blink/web_tests/external/wpt/css/css-contain/content-visibility/content-visibility-026.html

@@ -21,6 +21,7 @@
 }
 </style>
 <div id="container"></div>
+<div id="onscreen_container"></div>
 <div class=spacer></div>
 <div id="offscreen_container"></div>
 
@@ -41,49 +42,53 @@ test(() => {
 }, "content-visibility: hidden adds contain: size layout style;");
 
 async_test((t) => {
-  setUp(container);
+  setUp(onscreen_container);
   setUp(offscreen_container);
 
-  container.classList.add("auto");
-  offscreen_container.classList.add("auto");
-  // This will be determined to be on-screen immediately.
-  t.step(() => assert_equals(getComputedStyle(container).contain, "size layout style paint", "initial onscreen"));
-  t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "initial offscreen"));
-  // Considering this as frame 1:
-  // At frame 2, intersection observation will happen and determine that the container
-  //   is not visible.
-  // At frame 3, the container has size-containment, and intersection
-  //   observations will mark this container as visible at the end of the frame
-  // At frame 4, the container still has size-containment, because visiblility
-  //   switch happens after rAF-callbacks have run.
-  // At frame 5, the container is no longer size-contained since it is visible.
-  requestAnimationFrame(() => {
-    // Frame 2 checks:
-    // The onscreen container should be visible and the switch happens immediately after layout.
-    t.step(() => assert_equals(getComputedStyle(container).contain, "size layout style paint", "frame 2 onscreen"));
-    t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 2 offscreen"));
+  function runTest() {
+    onscreen_container.classList.add("auto");
+    offscreen_container.classList.add("auto");
 
-    requestAnimationFrame(() => {
-      offscreen_container.scrollIntoView();
+    t.step(() => assert_equals(getComputedStyle(onscreen_container).contain, "size layout style paint", "initial onscreen"));
+    t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "initial offscreen"));
+    // Considering this as frame 1, at the end of the frame intersection observation will happen
+    // and determine that on-screen is visible and off-screen is not. The on-screen switch will
+    // happen synchronously since this is the first time we check, so for the remainder of the
+    // frames, on-screen should not have a size containment.
 
-      // Frame 3 checks:
-      // At the beginning of the frame, the onscreen container should already be visible because
-      // the visibility switch should have happened after layout of the previous frame.
-      t.step(() => assert_equals(getComputedStyle(container).contain, "layout style paint", "frame 3 onscreen"));
-      t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 3 offscreen"));
+    // For off-screen container:
+    // At frame 2, we verify that it has size containment.
+    // At frame 3, we scroll it into view and verify it has size containment. At the end of the frame
+    //    we will determine that it is visible and schedule a visibility switch to happen at the next
+    //    frame (at post-rAF timing).
+    // At frame 4, we still have size containment until after rAF callbacks.
+    // At frame 5, the container is no longer size-contained since it is visible.
+    requestAnimationFrame(() => {
+      // Frame 2 checks:
+      t.step(() => assert_equals(getComputedStyle(onscreen_container).contain, "layout style paint", "frame 2 onscreen"));
+      t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 2 offscreen"));
 
       requestAnimationFrame(() => {
-        // Frame 4 checks:
-        t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 4"));
+        offscreen_container.scrollIntoView();
+
+        // Frame 3 checks:
+        t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 3"));
 
         requestAnimationFrame(() => {
-          // Frame 5 checks:
-          t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "layout style paint", "frame 4"));
-          t.done();
+          // Frame 4 checks:
+          t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "size layout style paint", "frame 4"));
+
+          requestAnimationFrame(() => {
+            // Frame 5 checks:
+            t.step(() => assert_equals(getComputedStyle(offscreen_container).contain, "layout style paint", "frame 5"));
+            t.done();
+          });
         });
       });
     });
-  });
+  }
+  // rAF to ensure we know where we are in the lifecycle.
+  requestAnimationFrame(runTest);
 }, "content-visibility: auto adds contain: size layout style paint;");
 
 test(() => {