[DL] Refactor API between DisplayLockContext and budget.

Instead of computing deadlines and lifecycle counts relative to
last observation, inject the latest lifecycle count of the releveant
frame generator (LocalFrameView) and the actual start time of the
lifecycle.

This simplifies the code somewhat, improves dependencies, and has
two concrete benefits:
1. Uses the same deadline for all locks executed on the same frame.
Previously each lock would compute its own deadline at the time
WillStartLifecycleUpdate was called.
2. Allows skipping lifecycles if desired. e.g. if a lock is updating,
but we decide not to doing anything for it for a given frame, we can
skip it.

This CL achieves benefit #2; benefit #1 is a possible future addition.

Note that WillStartLifecycleUpdate is still required for all locks
which are currently acquiring, updating or committing (but not
quiesecent locks). The reason it's required is that there may be
forced style or layout computations between lifecycles, and those
end up clearing the dirty bits of the lifecycle. There needs to be
a mechanism to re-set those dirty bits, and WillStartLifecycleUpdate
is that mechanism.

Bug: 968928
Change-Id: Ibe7e2c51631e7a93101563db41a444d01ae54162
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1642789
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667273}

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

@@ -38,4 +38,13 @@ bool DisplayLockBudget::IsElementDirtyForPhase(Phase phase) const {
   return false;
 }
 
+void DisplayLockBudget::MarkPhaseAsDirty(Phase marking_phase) {
+  // Mark the next phase we're scheduled to run.
+  for (auto phase = static_cast<unsigned>(marking_phase);
+       phase <= static_cast<unsigned>(Phase::kLast); ++phase) {
+    if (MarkAncestorsDirtyForPhaseIfNeeded(static_cast<Phase>(phase)))
+      break;
+  }
+}
+
 }  // namespace blink

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

@@ -15,6 +15,8 @@ class TickClock;
 namespace blink {
 
 class DisplayLockContext;
+struct LifecycleData;
+
 class CORE_EXPORT DisplayLockBudget {
  public:
   enum class Phase : unsigned {
@@ -30,14 +32,14 @@ class CORE_EXPORT DisplayLockBudget {
 
   // Returns true if the given phase is allowed to proceed under the current
   // budget.
-  virtual bool ShouldPerformPhase(Phase) const = 0;
+  virtual bool ShouldPerformPhase(Phase, const LifecycleData&) = 0;
+
+  // Called just before any calls to ShouldPerformPhase for a new lifecycle.
+  virtual void OnLifecycleChange(const LifecycleData&) = 0;
 
   // Notifies the budget that the given phase was completed.
   virtual void DidPerformPhase(Phase) = 0;
 
-  // Notifies the budget that a new lifecycle update phase is going to start.
-  virtual void WillStartLifecycleUpdate() = 0;
-
   // Returns true if according to this budget, we still need a lifecycle update.
   // For example, if a budget blocked a needed phase, then it this will return
   // true indicating that another frame is needed.
@@ -48,13 +50,15 @@ class CORE_EXPORT DisplayLockBudget {
   void SetTickClockForTesting(const base::TickClock* clock) { clock_ = clock; }
 
  protected:
+  // Returns true if there is likely to be work for the given phase.
+  bool IsElementDirtyForPhase(Phase) const;
+
+  void MarkPhaseAsDirty(Phase marking_phase);
+
   // Marks the ancestor chain dirty for the given phase if it's needed. Returns
   // true if the ancestors were marked dirty and false otherwise.
   bool MarkAncestorsDirtyForPhaseIfNeeded(Phase);
 
-  // Returns true if there is likely to be work for the given phase.
-  bool IsElementDirtyForPhase(Phase) const;
-
   const base::TickClock* clock_;
 
  private:

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

@@ -352,10 +352,20 @@ void DisplayLockContext::FinishResolver(Member<ScriptPromiseResolver>* resolver,
   *resolver = nullptr;
 }
 
+bool DisplayLockContext::ShouldPerformUpdatePhase(
+    DisplayLockBudget::Phase phase) const {
+  DCHECK(document_);
+  if (state_ != kUpdating)
+    return false;
+  auto* view = document_->View();
+  return view && view->InLifecycleUpdate() &&
+         update_budget_->ShouldPerformPhase(phase,
+                                            view->CurrentLifecycleData());
+}
+
 bool DisplayLockContext::ShouldStyle(LifecycleTarget target) const {
   return target == kSelf || update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating && in_lifecycle_update_ &&
-          update_budget_->ShouldPerformPhase(DisplayLockBudget::Phase::kStyle));
+         ShouldPerformUpdatePhase(DisplayLockBudget::Phase::kStyle);
 }
 
 void DisplayLockContext::DidStyle(LifecycleTarget target) {
@@ -387,9 +397,7 @@ void DisplayLockContext::DidStyle(LifecycleTarget target) {
 
 bool DisplayLockContext::ShouldLayout(LifecycleTarget target) const {
   return target == kSelf || update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating && in_lifecycle_update_ &&
-          update_budget_->ShouldPerformPhase(
-              DisplayLockBudget::Phase::kLayout));
+         ShouldPerformUpdatePhase(DisplayLockBudget::Phase::kLayout);
 }
 
 void DisplayLockContext::DidLayout(LifecycleTarget target) {
@@ -402,9 +410,7 @@ void DisplayLockContext::DidLayout(LifecycleTarget target) {
 
 bool DisplayLockContext::ShouldPrePaint() const {
   return update_forced_ || state_ > kUpdating ||
-         (state_ == kUpdating && in_lifecycle_update_ &&
-          update_budget_->ShouldPerformPhase(
-              DisplayLockBudget::Phase::kPrePaint));
+         ShouldPerformUpdatePhase(DisplayLockBudget::Phase::kPrePaint);
 }
 
 void DisplayLockContext::DidPrePaint() {
@@ -692,13 +698,11 @@ void DisplayLockContext::DidMoveToNewDocument(Document& old_document) {
 }
 
 void DisplayLockContext::WillStartLifecycleUpdate(const LocalFrameView& view) {
-  in_lifecycle_update_ = true;
-  if (state_ == kUpdating)
-    update_budget_->WillStartLifecycleUpdate();
+  if (update_budget_)
+    update_budget_->OnLifecycleChange(view.CurrentLifecycleData());
 }
 
 void DisplayLockContext::DidFinishLifecycleUpdate(const LocalFrameView& view) {
-  in_lifecycle_update_ = false;
   if (acquire_resolver_) {
     if (ForceUnlockIfNeeded())
       return;

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

@@ -283,6 +283,8 @@ class CORE_EXPORT DisplayLockContext final
   // when acquiring this lock should immediately resolve the acquire promise.
   bool ConnectedToView() const;
 
+  bool ShouldPerformUpdatePhase(DisplayLockBudget::Phase phase) const;
+
   std::unique_ptr<DisplayLockBudget> update_budget_;
 
   Member<ScriptPromiseResolver> commit_resolver_;
@@ -304,7 +306,6 @@ 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;

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

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/display_lock/strict_yielding_display_lock_budget.h"
 
 #include <algorithm>
+#include "third_party/blink/renderer/core/frame/local_frame_view.h"
 
 namespace blink {
 
@@ -12,7 +13,9 @@ StrictYieldingDisplayLockBudget::StrictYieldingDisplayLockBudget(
     DisplayLockContext* context)
     : DisplayLockBudget(context) {}
 
-bool StrictYieldingDisplayLockBudget::ShouldPerformPhase(Phase phase) const {
+bool StrictYieldingDisplayLockBudget::ShouldPerformPhase(
+    Phase phase,
+    const LifecycleData& lifecycle_data) {
   // We should perform any phase earlier than the one we already completed.
   // Also, we should complete a new phase once per cycle.
   return (last_completed_phase_ && phase <= *last_completed_phase_) ||
@@ -36,7 +39,8 @@ void StrictYieldingDisplayLockBudget::DidPerformPhase(Phase phase) {
 #endif
 }
 
-void StrictYieldingDisplayLockBudget::WillStartLifecycleUpdate() {
+void StrictYieldingDisplayLockBudget::OnLifecycleChange(
+    const LifecycleData& lifecycle_data) {
   // Figure out the next phase we would run. If we had completed a phase before,
   // then we should try to complete the next one, otherwise we'll start with the
   // first phase.
@@ -48,12 +52,7 @@ void StrictYieldingDisplayLockBudget::WillStartLifecycleUpdate() {
           : Phase::kFirst;
 
   // Mark the next phase we're scheduled to run.
-  for (auto phase = static_cast<unsigned>(next_phase);
-       phase <= static_cast<unsigned>(Phase::kLast); ++phase) {
-    if (MarkAncestorsDirtyForPhaseIfNeeded(static_cast<Phase>(phase)))
-      break;
-  }
-
+  MarkPhaseAsDirty(next_phase);
   completed_new_phase_this_cycle_ = false;
 }
 

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

@@ -21,9 +21,9 @@ class CORE_EXPORT StrictYieldingDisplayLockBudget final
   StrictYieldingDisplayLockBudget(DisplayLockContext*);
   ~StrictYieldingDisplayLockBudget() override = default;
 
-  bool ShouldPerformPhase(Phase) const override;
+  bool ShouldPerformPhase(Phase, const LifecycleData&) override;
   void DidPerformPhase(Phase) override;
-  void WillStartLifecycleUpdate() override;
+  void OnLifecycleChange(const LifecycleData&) override;
   // Returns true if any of the lifecycles that have been previously blocked by
   // this budget need updates. Note that this does not check lifecycle phases
   // that have already completed by this budget even if they are now dirty

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

@@ -4,19 +4,24 @@
 
 #include "third_party/blink/renderer/core/display_lock/unyielding_display_lock_budget.h"
 
+#include "third_party/blink/renderer/core/frame/local_frame_view.h"
+
 namespace blink {
 
 UnyieldingDisplayLockBudget::UnyieldingDisplayLockBudget(
     DisplayLockContext* context)
     : DisplayLockBudget(context) {}
 
-bool UnyieldingDisplayLockBudget::ShouldPerformPhase(Phase) const {
+bool UnyieldingDisplayLockBudget::ShouldPerformPhase(
+    Phase,
+    const LifecycleData& lifecycle_data) {
   return true;
 }
 
 void UnyieldingDisplayLockBudget::DidPerformPhase(Phase) {}
 
-void UnyieldingDisplayLockBudget::WillStartLifecycleUpdate() {
+void UnyieldingDisplayLockBudget::OnLifecycleChange(
+    const LifecycleData& lifecycle_data) {
   // Mark all the phases dirty since we have no intention of yielding.
   for (auto phase = static_cast<unsigned>(Phase::kFirst);
        phase <= static_cast<unsigned>(Phase::kLast); ++phase) {

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

@@ -18,9 +18,9 @@ class CORE_EXPORT UnyieldingDisplayLockBudget final : public DisplayLockBudget {
   UnyieldingDisplayLockBudget(DisplayLockContext*);
   ~UnyieldingDisplayLockBudget() override = default;
 
-  bool ShouldPerformPhase(Phase) const override;
+  bool ShouldPerformPhase(Phase, const LifecycleData&) override;
   void DidPerformPhase(Phase) override;
-  void WillStartLifecycleUpdate() override;
+  void OnLifecycleChange(const LifecycleData&) override;
   bool NeedsLifecycleUpdates() const override;
 };
 

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

@@ -5,6 +5,7 @@
 #include "third_party/blink/renderer/core/display_lock/yielding_display_lock_budget.h"
 
 #include "base/time/tick_clock.h"
+#include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/platform/wtf/time.h"
 
 #include <algorithm>
@@ -15,7 +16,9 @@ YieldingDisplayLockBudget::YieldingDisplayLockBudget(
     DisplayLockContext* context)
     : DisplayLockBudget(context) {}
 
-bool YieldingDisplayLockBudget::ShouldPerformPhase(Phase phase) const {
+bool YieldingDisplayLockBudget::ShouldPerformPhase(
+    Phase phase,
+    const LifecycleData& lifecycle_data) {
   // Always perform at least one more phase.
   if (phase <= next_phase_from_start_of_lifecycle_)
     return true;
@@ -33,17 +36,16 @@ void YieldingDisplayLockBudget::DidPerformPhase(Phase phase) {
     last_completed_phase_ = phase;
 
   // Mark the next phase as dirty so that we can reach it if we need to.
-  for (auto phase = static_cast<unsigned>(*last_completed_phase_) + 1;
-       phase <= static_cast<unsigned>(Phase::kLast); ++phase) {
-    if (MarkAncestorsDirtyForPhaseIfNeeded(static_cast<Phase>(phase)))
-      break;
-  }
+  MarkPhaseAsDirty(
+      static_cast<Phase>(static_cast<unsigned>(*last_completed_phase_) + 1));
 }
 
-void YieldingDisplayLockBudget::WillStartLifecycleUpdate() {
-  ++lifecycle_count_;
-  deadline_ =
-      clock_->NowTicks() + TimeDelta::FromMillisecondsD(GetCurrentBudgetMs());
+void YieldingDisplayLockBudget::OnLifecycleChange(
+    const LifecycleData& lifecycle_data) {
+  if (first_lifecycle_count_ == 0)
+    first_lifecycle_count_ = lifecycle_data.count;
+  deadline_ = lifecycle_data.start_time +
+              TimeDelta::FromMillisecondsD(GetCurrentBudgetMs(lifecycle_data));
 
   // Figure out the next phase we would run. If we had completed a phase before,
   // then we should try to complete the next one, otherwise we'll start with the
@@ -54,13 +56,7 @@ void YieldingDisplayLockBudget::WillStartLifecycleUpdate() {
                 std::min(static_cast<unsigned>(*last_completed_phase_) + 1,
                          static_cast<unsigned>(Phase::kLast)))
           : Phase::kFirst;
-
-  // Mark the next phase we're scheduled to run.
-  for (auto phase = static_cast<unsigned>(next_phase_from_start_of_lifecycle_);
-       phase <= static_cast<unsigned>(Phase::kLast); ++phase) {
-    if (MarkAncestorsDirtyForPhaseIfNeeded(static_cast<Phase>(phase)))
-      break;
-  }
+  MarkPhaseAsDirty(next_phase_from_start_of_lifecycle_);
 }
 
 bool YieldingDisplayLockBudget::NeedsLifecycleUpdates() const {
@@ -80,19 +76,21 @@ bool YieldingDisplayLockBudget::NeedsLifecycleUpdates() const {
   return false;
 }
 
-double YieldingDisplayLockBudget::GetCurrentBudgetMs() const {
+double YieldingDisplayLockBudget::GetCurrentBudgetMs(
+    const LifecycleData& lifecycle_data) const {
+  int lifecycle_count = lifecycle_data.count - first_lifecycle_count_ + 1;
   if (TimeTicks::IsHighResolution()) {
-    if (lifecycle_count_ < 3)
+    if (lifecycle_count < 3)
       return 4.;
-    if (lifecycle_count_ < 10)
+    if (lifecycle_count < 10)
       return 8.;
-    if (lifecycle_count_ < 60)
+    if (lifecycle_count < 60)
       return 16.;
   } else {
     // Without a high resolution clock, the resolution can be as bad as 15ms, so
     // increase the budgets accordingly to ensure we don't abort before doing
     // any phases.
-    if (lifecycle_count_ < 60)
+    if (lifecycle_count < 60)
       return 16.;
   }
   return 1e9;

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

@@ -21,9 +21,9 @@ class CORE_EXPORT YieldingDisplayLockBudget final : public DisplayLockBudget {
   YieldingDisplayLockBudget(DisplayLockContext*);
   ~YieldingDisplayLockBudget() override = default;
 
-  bool ShouldPerformPhase(Phase) const override;
+  bool ShouldPerformPhase(Phase, const LifecycleData& lifecycle_data) override;
   void DidPerformPhase(Phase) override;
-  void WillStartLifecycleUpdate() override;
+  void OnLifecycleChange(const LifecycleData& lifecycle_data) override;
   // Returns true if any of the lifecycles that have been previously blocked by
   // this budget need updates. Note that this does not check lifecycle phases
   // that have already completed by this budget even if they are now dirty
@@ -35,9 +35,10 @@ class CORE_EXPORT YieldingDisplayLockBudget final : public DisplayLockBudget {
  protected:
   friend class DisplayLockBudgetTest;
 
-  double GetCurrentBudgetMs() const;
+  double GetCurrentBudgetMs(const LifecycleData& lifecycle_data) const;
 
-  int lifecycle_count_ = 0;
+ private:
+  unsigned first_lifecycle_count_ = 0;
   TimeTicks deadline_;
   base::Optional<Phase> last_completed_phase_;
   Phase next_phase_from_start_of_lifecycle_ = Phase::kFirst;

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

@@ -270,6 +270,7 @@ LocalFrameView::LocalFrameView(LocalFrame& frame, IntRect frame_rect)
       intersection_observation_state_(kNotNeeded),
       needs_forced_compositing_update_(false),
       needs_focus_on_fragment_(false),
+      in_lifecycle_update_(false),
       tracked_object_paint_invalidations_(
           base::WrapUnique(g_initial_track_all_paint_invalidations
                                ? new Vector<ObjectPaintInvalidation>
@@ -2194,6 +2195,10 @@ bool LocalFrameView::UpdateLifecyclePhases(
     return Lifecycle().GetState() == target_state;
   }
 
+  base::AutoReset<bool> in_lifecycle_scope(&in_lifecycle_update_, true);
+  lifecycle_data_.start_time = base::TimeTicks::Now();
+  ++lifecycle_data_.count;
+
   {
     TRACE_EVENT0("blink,benchmark", "LocalFrameView::WillStartLifecycleUpdate");
 

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

@@ -103,6 +103,16 @@ typedef uint64_t DOMTimeStamp;
 using LayerTreeFlags = unsigned;
 using MainThreadScrollingReasons = uint32_t;
 
+struct LifecycleData {
+  LifecycleData() {}
+  LifecycleData(TimeTicks start_time_arg, int count_arg)
+      : start_time(start_time_arg), count(count_arg) {}
+  TimeTicks start_time;
+  // The number of lifecycles that have occcurred since the first one,
+  // inclusive, on a given LocalFrameRoot.
+  unsigned count = 0;
+};
+
 class CORE_EXPORT LocalFrameView final
     : public GarbageCollectedFinalized<LocalFrameView>,
       public FrameView {
@@ -358,6 +368,14 @@ class CORE_EXPORT LocalFrameView final
   // desired state.
   bool UpdateLifecycleToLayoutClean();
 
+  bool InLifecycleUpdate() { return in_lifecycle_update_; }
+  void SetInLifecycleUpdateForTest(bool val) { in_lifecycle_update_ = val; }
+  void SetLifecycleDataForTesting(const LifecycleData& lifecycle_data) {
+    lifecycle_data_ = lifecycle_data;
+  }
+
+  const LifecycleData& CurrentLifecycleData() const { return lifecycle_data_; }
+
   // This for doing work that needs to run synchronously at the end of lifecyle
   // updates, but needs to happen outside of the lifecycle code. It's OK to
   // schedule another animation frame here, but the layout tree should not be
@@ -937,6 +955,9 @@ class CORE_EXPORT LocalFrameView final
   bool needs_forced_compositing_update_;
 
   bool needs_focus_on_fragment_;
+  bool in_lifecycle_update_;
+
+  LifecycleData lifecycle_data_;
 
   IntRect remote_viewport_intersection_;