SubtreeVisibility: optimize nested lock case.

This patch fixes the "double render" problem that occurs when we have
auto locks inside a container that becomes hidden.

Previously:
- When outer element becomes hidden, intersection observer tells inner
  locks that they are not visible, causing them to lock and invalidate
  their own layout (which propagates up to the outer element).
- When outer element becomes visible, we process the dirty tree
  (render 1)
- If inner elements are visible:
  - Then we run intersection observer which notifies the auto locks that
    they are visible, causing them to unlock and invalidate their own
    layout
  - Next lifecycle, we clean the layout state (render 2)
- If not:
  - Nothing happens since intersection observer does not run on already
    hidden elements.

Overall, we have at least 1 render. In the worst case, 2 renders.

Now:
- When outer element becomes hidden, intersection observer tells inner
  locks that they are not visible. The inner locks discover they are
  nested, and register themselves for lifecycle notifications but don't
  invalidate layout.
- When outer element becomes visible, we don't do anything (I'm not
  counting the element itself updating its layout; since the important
  thing is the subtree)
- If inner elements are visible:
  - Nothing happens; the next intersection observer that lets them know
    they are visible will be ignored since they are unlocked.
- If inner elements are not visible:
  - At the start of the next lifecycle, the element determines that it
    should be not visible and that its not nested; it locks itself
    causing layout invalidations. This same lifecycle is render 1.

Overall, we can have no renders. In the worst case, 1 render.

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

Change-Id: Id9a75a7a68d5208c49839f9f2adb9ee8ae6734e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2092196Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749229}

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

@@ -148,6 +148,8 @@ void DisplayLockContext::ContextDestroyed() {
 void DisplayLockContext::UpdateActivationObservationIfNeeded() {
   if (!document_) {
     is_observed_ = false;
+    is_registered_for_lifecycle_notifications_ = false;
+    needs_intersection_lock_check_ = false;
     return;
   }
 
@@ -165,10 +167,36 @@ void DisplayLockContext::UpdateActivationObservationIfNeeded() {
     document_->RegisterDisplayLockActivationObservation(element_);
   } else if (!should_observe && is_observed_) {
     document_->UnregisterDisplayLockActivationObservation(element_);
+    // We don't need intersection lock checks if we are not observing
+    // intersections anymore.
+    needs_intersection_lock_check_ = false;
+    UpdateLifecycleNotificationRegistration();
   }
   is_observed_ = should_observe;
 }
 
+bool DisplayLockContext::NeedsLifecycleNotifications() const {
+  return HasResolver() || needs_intersection_lock_check_;
+}
+
+void DisplayLockContext::UpdateLifecycleNotificationRegistration() {
+  if (!document_ || !document_->View()) {
+    is_registered_for_lifecycle_notifications_ = false;
+    return;
+  }
+
+  bool needs_notifications = NeedsLifecycleNotifications();
+  if (needs_notifications == is_registered_for_lifecycle_notifications_)
+    return;
+
+  is_registered_for_lifecycle_notifications_ = needs_notifications;
+  if (needs_notifications) {
+    document_->View()->RegisterForLifecycleNotifications(this);
+  } else {
+    document_->View()->UnregisterFromLifecycleNotifications(this);
+  }
+}
+
 void DisplayLockContext::SetActivatable(uint16_t activatable_mask) {
   if (IsLocked()) {
     // If we're locked, the activatable mask might change the activation
@@ -193,6 +221,9 @@ void DisplayLockContext::StartAcquire() {
   update_budget_.reset();
   state_ = kLocked;
 
+  needs_intersection_lock_check_ = false;
+  UpdateLifecycleNotificationRegistration();
+
   // We're no longer activated, so if the signal didn't run yet, we should
   // cancel it.
   weak_factory_.InvalidateWeakPtrs();
@@ -283,11 +314,11 @@ bool DisplayLockContext::CleanupAndRejectCommitIfNotConnected() {
 void DisplayLockContext::MakeResolver(ScriptState* script_state,
                                       Member<ScriptPromiseResolver>* resolver) {
   DCHECK(ConnectedToView());
-  document_->View()->RegisterForLifecycleNotifications(this);
   *resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
+  UpdateLifecycleNotificationRegistration();
 }
 
-bool DisplayLockContext::HasResolver() {
+bool DisplayLockContext::HasResolver() const {
   return update_resolver_;
 }
 
@@ -323,8 +354,7 @@ void DisplayLockContext::FinishResolver(Member<ScriptPromiseResolver>* resolver,
       break;
   }
   *resolver = nullptr;
-  if (!HasResolver() && ConnectedToView())
-    document_->View()->UnregisterFromLifecycleNotifications(this);
+  UpdateLifecycleNotificationRegistration();
 }
 
 bool DisplayLockContext::ShouldPerformUpdatePhase(
@@ -487,6 +517,57 @@ bool DisplayLockContext::IsActivated() const {
 
 void DisplayLockContext::ClearActivated() {
   css_is_activated_ = false;
+  // If we are no longer activated, then we're either committing or acquiring a
+  // lock. In either case, we don't need to rely on lifecycle observations to
+  // become hidden.
+  // TODO(vmpstr): This needs refactoring.
+  needs_intersection_lock_check_ = false;
+  UpdateLifecycleNotificationRegistration();
+}
+
+void DisplayLockContext::NotifyIsIntersectingViewport() {
+  // If we are now intersecting, then we are definitely not nested in a locked
+  // subtree and we don't need to lock as a result.
+  needs_intersection_lock_check_ = false;
+  UpdateLifecycleNotificationRegistration();
+
+  if (!IsLocked())
+    return;
+
+  DCHECK(IsActivatable(DisplayLockActivationReason::kViewportIntersection));
+  CommitForActivationWithSignal(
+      element_, DisplayLockActivationReason::kViewportIntersection);
+}
+
+void DisplayLockContext::NotifyIsNotIntersectingViewport() {
+  if (IsLocked()) {
+    DCHECK(!needs_intersection_lock_check_);
+    return;
+  }
+
+  // There are two situations we need to consider here:
+  // 1. We are off-screen but not nested in any other lock. This means we should
+  //    re-lock (also verify that the reason we're in this state is that we're
+  //    activated).
+  // 2. We are in a nested locked context. This means we don't actually know
+  //    whether we should lock or not. In order to avoid needless dirty of the
+  //    layout and style trees up to the nested context, we remain unlocked.
+  //    However, we also need to ensure that we relock if we become unnested.
+  //    So, we simply delay this check to the next frame (via LocalFrameView),
+  //    which will call this function again and so we can perform the check
+  //    again.
+  DCHECK(ConnectedToView());
+  auto* locked_ancestor =
+      DisplayLockUtilities::NearestLockedExclusiveAncestor(*element_);
+  if (locked_ancestor) {
+    needs_intersection_lock_check_ = true;
+    UpdateLifecycleNotificationRegistration();
+  } else {
+    DCHECK(IsActivated());
+    ClearActivated();
+    StartAcquire();
+    DCHECK(!needs_intersection_lock_check_);
+  }
 }
 
 bool DisplayLockContext::ShouldCommitForActivation(
@@ -814,7 +895,7 @@ void DisplayLockContext::DidMoveToNewDocument(Document& old_document) {
 
   // Since we're observing the lifecycle updates, ensure that we listen to the
   // right document's view.
-  if (HasResolver()) {
+  if (is_registered_for_lifecycle_notifications_) {
     if (old_document.View())
       old_document.View()->UnregisterFromLifecycleNotifications(this);
     if (document_->View())
@@ -832,11 +913,28 @@ void DisplayLockContext::DidMoveToNewDocument(Document& old_document) {
 }
 
 void DisplayLockContext::WillStartLifecycleUpdate(const LocalFrameView& view) {
+  DCHECK(NeedsLifecycleNotifications());
+  // If we have an update budget, then forward the call to it, so that it can
+  // prepare for the lifecycle by propagating the next phase's dirty bits.
   if (update_budget_)
     update_budget_->OnLifecycleChange(view.CurrentLifecycleData());
+
+  // We might have delayed processing intersection observation update (signal
+  // that we were not intersecting) because this context was nested in another
+  // locked context. At the start of the lifecycle, we should check whether
+  // that is still true. In other words, this call will check if we're still
+  // nested. If we are, we won't do anything. If we're not, then we will lock
+  // this context.
+  //
+  // Note that when we are no longer nested and and we have not received any
+  // notifications from the intersection observer, it means that we are not
+  // visible.
+  if (needs_intersection_lock_check_)
+    NotifyIsNotIntersectingViewport();
 }
 
 void DisplayLockContext::DidFinishLifecycleUpdate(const LocalFrameView& view) {
+  DCHECK(NeedsLifecycleNotifications());
   if (state_ == kCommitting) {
     FinishUpdateResolver(kResolve);
     state_ = kUnlocked;

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

@@ -144,6 +144,11 @@ class CORE_EXPORT DisplayLockContext final
   // Clear the activated flag.
   void ClearActivated();
 
+  // Is called by the intersection observer callback to inform us of the
+  // intersection state.
+  void NotifyIsIntersectingViewport();
+  void NotifyIsNotIntersectingViewport();
+
   // Acquire the lock, should only be called when unlocked.
   void StartAcquire();
   // Initiate a commit.
@@ -282,10 +287,14 @@ class CORE_EXPORT DisplayLockContext final
   }
 
  private:
-  friend class DisplayLockContextTest;
+  // Test friends.
   friend class DisplayLockBudgetTest;
-  friend class DisplayLockSuspendedHandle;
+  friend class DisplayLockContextRenderingTest;
+  friend class DisplayLockContextTest;
+
+  // Production friends.
   friend class DisplayLockBudget;
+  friend class DisplayLockSuspendedHandle;
 
   class StateChangeHelper {
     DISALLOW_NEW();
@@ -342,7 +351,7 @@ class CORE_EXPORT DisplayLockContext final
   // Helper functions to resolve the update/commit promises.
   enum ResolverState { kResolve, kReject, kDetach };
   void MakeResolver(ScriptState*, Member<ScriptPromiseResolver>*);
-  bool HasResolver();
+  bool HasResolver() const;
   void FinishUpdateResolver(ResolverState, const char* reject_reason = nullptr);
   void FinishResolver(Member<ScriptPromiseResolver>*,
                       ResolverState,
@@ -381,6 +390,12 @@ class CORE_EXPORT DisplayLockContext final
   // Scheduled by CommitForActivationWithSignal.
   void FireActivationEvent(Element* activated_element);
 
+  // Determines whether or not we need lifecycle notifications.
+  bool NeedsLifecycleNotifications() const;
+  // Updates the lifecycle notification registration based on whether we need
+  // the notifications.
+  void UpdateLifecycleNotificationRegistration();
+
   std::unique_ptr<DisplayLockBudget> update_budget_;
 
   Member<ScriptPromiseResolver> update_resolver_;
@@ -429,6 +444,17 @@ class CORE_EXPORT DisplayLockContext final
   // valid for CSS version of subtree-visibility.
   bool css_is_activated_ = false;
 
+  // Is set to true if we are registered for lifecycle notifications.
+  bool is_registered_for_lifecycle_notifications_ = false;
+
+  // This is set to true when we have delayed locking ourselves due to viewport
+  // intersection (or lack thereof) because we were nested in a locked subtree.
+  // In that case, we register for lifecycle notifications and check every time
+  // if we are still nested.
+  bool needs_intersection_lock_check_ = false;
+
+  // TODO(vmpstr): This is only needed while we're still sending activation
+  // events.
   base::WeakPtrFactory<DisplayLockContext> weak_factory_{this};
 };
 

third_party/blink/renderer/core/dom/document.cc

@@ -8501,21 +8501,9 @@ void Document::ProcessDisplayLockActivationObservation(
     auto* context = entry->target()->GetDisplayLockContext();
     DCHECK(context);
     if (entry->isIntersecting()) {
-      if (!context->IsLocked())
-        continue;
-      DCHECK(context->ShouldCommitForActivation(
-          DisplayLockActivationReason::kViewportIntersection));
-      context->CommitForActivationWithSignal(
-          entry->target(), DisplayLockActivationReason::kViewportIntersection);
+      context->NotifyIsIntersectingViewport();
     } else {
-      // If we're not visible, but are observing viewport intersections, it
-      // means that we're either locked (in which case we should remain locked),
-      // or we've been activated (in which case we should relock).
-      DCHECK(context->IsLocked() || context->IsActivated());
-      if (context->IsLocked())
-        continue;
-      context->ClearActivated();
-      context->StartAcquire();
+      context->NotifyIsNotIntersectingViewport();
     }
   }
 }