Bug 1237454 - Throttle animations on visibility:hidden element. r=birtles,boris,emilio

This patch does basically throttle animations on visibility:hidden element
and unthrottle it once the animating element became visible or a child of the
animating element became visible.  But still there are some cases that we don't
throttle such animations perfectly.  For example;

  div.style.visibility = 'hidden'; // the 'div' has no children at this moment
  div.animate(..);
  // The animation is throttled

  div.appendChild(visibleChild);
  // The animation isn't throttled

  visibleChild.style.visibility = 'hidden';
  // Now the animation should be throttled again, but actually it's not.

To throttle this case properly, when the |visibleChild|'s visibility changed
to hidden, we would need to do either

 1) Check all siblings of the |visibleChild| have no visible children

or

 2) The parent element stores visible children count somewhere and decrease it
    and check whether the count is zero

To achieve 1) we need to walk up ancestors and their siblings, actually it's
inefficient.

2) is somewhat similar to what we already do for animating images but it's hard
to reuse it for CSS animations since it does not take into account that
descendants' visibilities.

Another example that this patch does not optimize is the the case where
animating element has children whose visibility is inherited and the element
itself initially visible something like this;

  let child = document.createElement('div'); // child visibility is 'inherit'
  div.appendChild(child);

  div.animate(..); // the 'div' is visible
  // The animation isn't throttled since the animating element is visible

  div.style.visiblily = 'hidden';
  // Now the animation should be throttled, but it's not since this patch does
  // not descend down all descendants to check they are invisible or not when the
  // animating element visibility changed to hidden.

This patch adds a test case for this case introduced with todo_is().

Another test case added in this patch fails if we don't use
nsPlaceholderFrame::GetRealFrameFor() in HasNoVisibleDescendants().

MozReview-Commit-ID: BJwzQvP9Yc4

--HG--
extra : rebase_source : ceb95bdce1042cbfc16751d6d023fc6feee5845e

dom/animation/KeyframeEffectReadOnly.cpp

@@ -1457,7 +1457,9 @@ KeyframeEffectReadOnly::CanThrottle() const
     if (presShell && !presShell->IsActive()) {
       return true;
     }
-    if (frame->IsScrolledOutOfView()) {
+
+    if (!frame->IsVisibleOrMayHaveVisibleDescendants() ||
+        frame->IsScrolledOutOfView()) {
       // If there are transform change hints, unthrottle the animation
       // periodically since it might affect the overflow region.
       if (mCumulativeChangeHint & (nsChangeHint_UpdatePostTransformOverflow |

dom/animation/test/mozilla/file_restyles.html

@@ -668,9 +668,9 @@ waitForAllPaints(() => {
 
     var markers = await observeStyling(5);
 
-    todo_is(markers.length, 0,
-            'Bug 1237454: Animations running on the compositor in ' +
-            'visibility hidden element should never cause restyles');
+    is(markers.length, 0,
+       'Animations running on the compositor in visibility hidden element ' +
+       'should never cause restyles');
     await ensureElementRemoval(div);
   });
 
@@ -727,6 +727,33 @@ waitForAllPaints(() => {
     await ensureElementRemoval(parentElement);
   });
 
+  add_task(
+    async function restyling_animations_on_visibility_changed_element_having_child() {
+      var div = addDiv(null,
+       { style: 'animation: background-color 100s;' });
+      var childElement = addDiv(null);
+      div.appendChild(childElement);
+
+      var animation = div.getAnimations()[0];
+
+      await animation.ready;
+
+      // We don't check the animation causes restyles here since we already
+      // check it in the first test case.
+
+      div.style.visibility = 'hidden';
+      await waitForNextFrame();
+
+      var markers = await observeStyling(5);
+      todo_is(markers.length, 0,
+              'Animations running on visibility hidden element which ' +
+              'has a child whose visiblity is inherited from the element and ' +
+              'the element was initially visible');
+
+      await ensureElementRemoval(div);
+    }
+  );
+
   add_task(
     async function restyling_animations_on_visibility_hidden_element_which_gets_visible() {
       var div = addDiv(null,
@@ -737,9 +764,9 @@ waitForAllPaints(() => {
       await animation.ready;
       var markers = await observeStyling(5);
 
-      todo_is(markers.length, 0,
-              'Animations running on visibility hidden element should never ' +
-              'cause restyles');
+      is(markers.length, 0,
+         'Animations running on visibility hidden element should never ' +
+         'cause restyles');
 
       div.style.visibility = 'visible';
       await waitForNextFrame();
@@ -763,9 +790,9 @@ waitForAllPaints(() => {
     await animation.ready;
     var markers = await observeStyling(5);
 
-    todo_is(markers.length, 0,
-            'Animations running in visibility hidden parent should never cause ' +
-            'restyles');
+    is(markers.length, 0,
+       'Animations running in visibility hidden parent should never cause ' +
+       'restyles');
 
     parentDiv.style.visibility = 'visible';
     await waitForNextFrame();
@@ -779,9 +806,9 @@ waitForAllPaints(() => {
     await waitForNextFrame();
 
     var markers = await observeStyling(5);
-    todo_is(markers.length, 0,
-            'Animations that the parent element became visible should throttle ' +
-            'restyling again');
+    is(markers.length, 0,
+       'Animations that the parent element became visible should throttle ' +
+       'restyling again');
 
     await ensureElementRemoval(parentDiv);
   });
@@ -795,9 +822,9 @@ waitForAllPaints(() => {
       await animation.ready;
       var markers = await observeStyling(5);
 
-      todo_is(markers.length, 0,
-              'Animations on visibility hidden element having no visible children ' +
-              'should never cause restyles');
+      is(markers.length, 0,
+         'Animations on visibility hidden element having no visible children ' +
+         'should never cause restyles');
 
       var childElement = addDiv(null, { style: 'visibility: visible' });
       div.appendChild(childElement);
@@ -836,6 +863,33 @@ waitForAllPaints(() => {
     }
   );
 
+  add_task(
+    async function restyling_animations_on_visiblity_hidden_element_having_oof_child() {
+      var div = addDiv(null,
+        { style: 'animation: background-color 100s; position: absolute' });
+      var childElement = addDiv(null,
+        { style: 'float: left; visibility: hidden' });
+      div.appendChild(childElement);
+
+      var animation = div.getAnimations()[0];
+
+      await animation.ready;
+
+      // We don't check the animation causes restyles here since we already
+      // check it in the first test case.
+
+      div.style.visibility = 'hidden';
+      await waitForNextFrame();
+
+      var markers = await observeStyling(5);
+      is(markers.length, 0,
+         'Animations running on visibility hidden element which has an ' +
+         'out-of-flow child should throttle restyling');
+
+      await ensureElementRemoval(div);
+    }
+  );
+
   add_task(
     async function restyling_animations_on_visibility_hidden_element_having_grandchild() {
       // element tree:
@@ -868,9 +922,9 @@ waitForAllPaints(() => {
 
       await animation.ready;
       var markers = await observeStyling(5);
-      todo_is(markers.length, 0,
-              'Animations on visibility hidden element having no visible ' +
-              'descendants should never cause restyles');
+      is(markers.length, 0,
+         'Animations on visibility hidden element having no visible ' +
+         'descendants should never cause restyles');
 
       childA.style.visibility = 'visible';
       grandchildAA.style.visibility = 'visible';

layout/base/RestyleManager.cpp

@@ -1729,6 +1729,9 @@ RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList)
       if (hint & nsChangeHint_UpdateTableCellSpans) {
         frameConstructor->UpdateTableCellSpans(content);
       }
+      if (hint & nsChangeHint_VisibilityChange) {
+        frame->UpdateVisibleDescendantsState();
+      }
     }
   }
 

layout/generic/nsFrame.cpp

@@ -734,6 +734,12 @@ nsFrame::Init(nsIContent*       aContent,
 
   if (::IsXULBoxWrapped(this))
     ::InitBoxMetrics(this, false);
+
+  // For a newly created frame, we need to update this frame's visibility state.
+  // Usually we update the state when the frame is restyled and has a
+  // VisibilityChange change hint but we don't generate any change hints for
+  // newly created frames.
+  UpdateVisibleDescendantsState();
 }
 
 void
@@ -11341,6 +11347,39 @@ nsIFrame::GetCompositorHitTestInfo(nsDisplayListBuilder* aBuilder)
   return result;
 }
 
+// Returns true if we can guarantee there is no visible descendants.
+static bool
+HasNoVisibleDescendants(const nsIFrame* aFrame)
+{
+  for (nsIFrame::ChildListIterator lists(aFrame);
+       !lists.IsDone();
+       lists.Next()) {
+    for (nsIFrame* f : lists.CurrentList()) {
+      if (nsPlaceholderFrame::GetRealFrameFor(f)->
+            IsVisibleOrMayHaveVisibleDescendants()) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+void
+nsIFrame::UpdateVisibleDescendantsState()
+{
+  if (StyleVisibility()->IsVisible()) {
+    // Notify invisible ancestors that a visible descendant exists now.
+    nsIFrame* ancestor;
+    for (ancestor = GetInFlowParent();
+         ancestor && !ancestor->StyleVisibility()->IsVisible();
+         ancestor = ancestor->GetInFlowParent()) {
+      ancestor->mAllDescendantsAreInvisible = false;
+    }
+  } else {
+    mAllDescendantsAreInvisible = HasNoVisibleDescendants(this);
+  }
+}
+
 // Box layout debugging
 #ifdef DEBUG_REFLOW
 int32_t gIndent2 = 0;

layout/generic/nsIFrame.h

@@ -628,6 +628,7 @@ public:
     , mIsPrimaryFrame(false)
     , mMayHaveTransformAnimation(false)
     , mMayHaveOpacityAnimation(false)
+    , mAllDescendantsAreInvisible(false)
   {
     mozilla::PodZero(&mOverflow);
   }
@@ -4089,6 +4090,13 @@ public:
     mMayHaveOpacityAnimation = true;
   }
 
+  // Returns true if this frame is visible or may have visible descendants.
+  bool IsVisibleOrMayHaveVisibleDescendants() const {
+    return !mAllDescendantsAreInvisible || StyleVisibility()->IsVisible();
+  }
+  // Update mAllDescendantsAreInvisible flag for this frame and ancestors.
+  void UpdateVisibleDescendantsState();
+
   /**
    * If this returns true, the frame it's called on should get the
    * NS_FRAME_HAS_DIRTY_CHILDREN bit set on it by the caller; either directly
@@ -4348,9 +4356,19 @@ private:
   bool mMayHaveTransformAnimation : 1;
   bool mMayHaveOpacityAnimation : 1;
 
+  /**
+   * True if we are certain that all descendants are not visible.
+   *
+   * This flag is conservative in that it might sometimes be false even if, in
+   * fact, all descendants are invisible.
+   * For example; an element is visibility:visible and has a visibility:hidden
+   * child. This flag is stil false in such case.
+   */
+  bool mAllDescendantsAreInvisible : 1;
+
 protected:
 
-  // There is a 1-bit gap left here.
+  // There is no gap left here.
 
   // Helpers
   /**