DL: Don't process update phases if we're not in a lifecycle run.

This patch ensures that we don't process any update() lifecycle phases
when layout is forced outside of the lifecycle phase. This can happen
if the budget yields, and then script runs which forces something
to be forced. We don't want to process update phases in that case,
since we want to wait until proper lifecycle runs to do that.

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

Bug: 964827
Change-Id: I05848c9dd3dcd4fcedc209a2f66e324d2ba6f0b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626172Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662612}

third_party/blink/renderer/core/display_lock/display_lock_budget_test.cc

@@ -14,6 +14,7 @@
 #include "third_party/blink/renderer/core/html_names.h"
 #include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
 #include "third_party/blink/renderer/platform/runtime_enabled_features.h"
+#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
 
 namespace blink {
 
@@ -427,7 +428,9 @@ TEST_F(DisplayLockBudgetTest, YieldingBudgetMarksNextPhase) {
   auto* parent = GetDocument().getElementById("parent");
   EXPECT_TRUE(budget->NeedsLifecycleUpdates());
 
-  budget->WillStartLifecycleUpdate();
+  element->GetDisplayLockContext()->WillStartLifecycleUpdate(
+      *GetDocument().GetFrame()->View());
+
   // Initially all of the phase checks should return true, since we don't know
   // which phase the system wants to process next.
   EXPECT_TRUE(budget->ShouldPerformPhase(DisplayLockBudget::Phase::kStyle));
@@ -459,4 +462,69 @@ TEST_F(DisplayLockBudgetTest, YieldingBudgetMarksNextPhase) {
   EXPECT_TRUE(parent->GetLayoutObject()->NeedsLayout());
   EXPECT_TRUE(element->GetLayoutObject()->NeedsLayout());
 }
+
+TEST_F(DisplayLockBudgetTest, UpdateHappensInLifecycleOnly) {
+  // Note that we're not testing the display lock here, just the budget so we
+  // can do minimal work to ensure we have a context, ignoring containment and
+  // other requirements.
+  SetHtmlInnerHTML(R"HTML(
+    <style>
+      #container {
+        contain: style layout;
+      }
+    </style>
+    <div id="parent"><div id="container"><div id="child"></div></div></div>
+  )HTML");
+
+  auto* element = GetDocument().getElementById("container");
+  {
+    auto* script_state = ToScriptStateForMainWorld(GetDocument().GetFrame());
+    ScriptState::Scope scope(script_state);
+    element->getDisplayLockForBindings()->acquire(script_state, nullptr);
+  }
+  UpdateAllLifecyclePhasesForTest();
+
+  ASSERT_TRUE(element->GetDisplayLockContext());
+  ASSERT_TRUE(element->GetDisplayLockContext()->IsLocked());
+
+  auto budget_owned = base::WrapUnique(
+      new UnyieldingDisplayLockBudget(element->GetDisplayLockContext()));
+  auto* budget = budget_owned.get();
+  {
+    auto* script_state = ToScriptStateForMainWorld(GetDocument().GetFrame());
+    ScriptState::Scope scope(script_state);
+    element->getDisplayLockForBindings()->update(script_state);
+    ResetBudget(std::move(budget_owned), element->GetDisplayLockContext());
+  }
+
+  // When acquiring, we need to update the layout with the locked size, so we
+  // need an update.
+  EXPECT_TRUE(budget->NeedsLifecycleUpdates());
+
+  auto* context = element->GetDisplayLockContext();
+  EXPECT_TRUE(budget->ShouldPerformPhase(DisplayLockBudget::Phase::kStyle));
+  EXPECT_TRUE(budget->ShouldPerformPhase(DisplayLockBudget::Phase::kLayout));
+  EXPECT_TRUE(budget->ShouldPerformPhase(DisplayLockBudget::Phase::kPrePaint));
+
+  // Since we're not in a lifecycle, the budget itself should not want to do any
+  // phases, even though the budget allows it.
+  EXPECT_FALSE(context->ShouldStyle(DisplayLockContext::kChildren));
+  EXPECT_FALSE(context->ShouldLayout(DisplayLockContext::kChildren));
+  EXPECT_FALSE(context->ShouldPrePaint());
+
+  context->WillStartLifecycleUpdate(*GetDocument().GetFrame()->View());
+
+  EXPECT_TRUE(context->ShouldStyle(DisplayLockContext::kChildren));
+  EXPECT_TRUE(context->ShouldLayout(DisplayLockContext::kChildren));
+  EXPECT_TRUE(context->ShouldPrePaint());
+
+  context->DidFinishLifecycleUpdate(*GetDocument().GetFrame()->View());
+
+  EXPECT_FALSE(context->ShouldStyle(DisplayLockContext::kChildren));
+  EXPECT_FALSE(context->ShouldLayout(DisplayLockContext::kChildren));
+  EXPECT_FALSE(context->ShouldPrePaint());
+
+  // Ensure to flush any tasks scheduled by context calls.
+  test::RunPendingTasks();
+}
 }  // namespace blink

third_party/blink/renderer/core/display_lock/display_lock_context.cc

@@ -352,7 +352,7 @@ void DisplayLockContext::FinishResolver(Member<ScriptPromiseResolver>* resolver,
 
 bool DisplayLockContext::ShouldStyle(LifecycleTarget target) const {
   return target == kSelf || update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating &&
+         (state_ == kUpdating && in_lifecycle_update_ &&
           update_budget_->ShouldPerformPhase(DisplayLockBudget::Phase::kStyle));
 }
 
@@ -397,7 +397,8 @@ void DisplayLockContext::DidStyle(LifecycleTarget target) {
 
 bool DisplayLockContext::ShouldLayout(LifecycleTarget target) const {
   return target == kSelf || update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating && update_budget_->ShouldPerformPhase(
+         (state_ == kUpdating && in_lifecycle_update_ &&
+          update_budget_->ShouldPerformPhase(
               DisplayLockBudget::Phase::kLayout));
 }
 
@@ -411,7 +412,8 @@ void DisplayLockContext::DidLayout(LifecycleTarget target) {
 
 bool DisplayLockContext::ShouldPrePaint() const {
   return update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating && update_budget_->ShouldPerformPhase(
+         (state_ == kUpdating && in_lifecycle_update_ &&
+          update_budget_->ShouldPerformPhase(
               DisplayLockBudget::Phase::kPrePaint));
 }
 
@@ -702,11 +704,13 @@ void DisplayLockContext::DidMoveToNewDocument(Document& old_document) {
 }
 
 void DisplayLockContext::WillStartLifecycleUpdate(const LocalFrameView& view) {
+  in_lifecycle_update_ = true;
   if (state_ == kUpdating)
     update_budget_->WillStartLifecycleUpdate();
 }
 
 void DisplayLockContext::DidFinishLifecycleUpdate(const LocalFrameView& view) {
+  in_lifecycle_update_ = false;
   if (acquire_resolver_) {
     if (!ElementSupportsDisplayLocking()) {
       FinishAcquireResolver(kReject, rejection_names::kContainmentNotSatisfied);

third_party/blink/renderer/core/display_lock/display_lock_context.h

@@ -292,6 +292,7 @@ class CORE_EXPORT DisplayLockContext final
   base::Optional<LayoutSize> locked_content_logical_size_;
 
   bool update_forced_ = false;
+  bool in_lifecycle_update_ = false;
   bool activatable_ = false;
 
   bool is_locked_after_connect_ = false;