Bug 1341985 - Trigger the second traversal for updating CSS animations. r=birtles,heycam

The restyle request during restyling is a result of creating/updating/removing
CSS animations that will come from a SequentialTask which will be implemented
in a subsequent patch.

MozReview-Commit-ID: JoAqvcN3y51

dom/animation/EffectCompositor.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/LayerAnimationInfo.h"
 #include "mozilla/RestyleManager.h"
 #include "mozilla/RestyleManagerInlines.h"
+#include "mozilla/ServoStyleSet.h"
 #include "mozilla/StyleAnimationValue.h"
 #include "nsComputedDOMStyle.h" // nsComputedDOMStyle::GetPresShellForContent
 #include "nsCSSPseudoElements.h"
@@ -313,11 +314,28 @@ EffectCompositor::PostRestyleForAnimation(dom::Element* aElement,
                                         eRestyle_CSSTransitions :
                                         eRestyle_CSSAnimations;
 
-  // FIXME: stylo only supports Self and Subtree hints now, so we override it
-  // for stylo if we are not in process of restyling.
-  if (mPresContext->StyleSet()->IsServo() &&
-      !mPresContext->RestyleManager()->IsInStyleRefresh()) {
-    hint = eRestyle_Self | eRestyle_Subtree;
+  if (mPresContext->StyleSet()->IsServo()) {
+    MOZ_ASSERT(NS_IsMainThread(),
+               "Restyle request during restyling should be requested only on "
+               "the main-thread. e.g. after the parallel traversal");
+    if (ServoStyleSet::IsInServoTraversal()) {
+      // FIXME: Bug 1302946: We will handle eRestyle_CSSTransitions.
+      MOZ_ASSERT(hint == eRestyle_CSSAnimations);
+
+      // We can't call Servo_NoteExplicitHints here since AtomicRefCell does not
+      // allow us mutate ElementData of the |aElement| in SequentialTask.
+      // Instead we call Servo_NoteExplicitHints for the element in PreTraverse() right
+      // which will be called right before the second traversal that we do for
+      // updating CSS animations.
+      // In that case PreTraverse() will return true so that we know to do the
+      // second traversal so we don't need to post any restyle requests to the
+      // PresShell.
+      return;
+    } else if (!mPresContext->RestyleManager()->IsInStyleRefresh()) {
+      // FIXME: stylo only supports Self and Subtree hints now, so we override
+      // it for stylo if we are not in process of restyling.
+      hint = eRestyle_Self | eRestyle_Subtree;
+    }
   }
   mPresContext->PresShell()->RestyleForAnimation(element, hint);
 }
@@ -932,12 +950,13 @@ EffectCompositor::SetPerformanceWarning(
   }
 }
 
-void
+bool
 EffectCompositor::PreTraverse()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mPresContext->RestyleManager()->IsServo());
 
+  bool foundElementsNeedingRestyle = false;
   for (auto& elementsToRestyle : mElementsToRestyle) {
     for (auto iter = elementsToRestyle.Iter(); !iter.Done(); iter.Next()) {
       bool postedRestyle = iter.Data();
@@ -947,10 +966,20 @@ EffectCompositor::PreTraverse()
       }
 
       NonOwningAnimationTarget target = iter.Key();
+
+      // We need to post restyle hints even if the target is not in EffectSet to
+      // ensure the final restyling for removed animations.
+      // We can't call PostRestyleEvent directly here since we are still in the
+      // middle of the servo traversal.
+      mPresContext->RestyleManager()->AsServo()->
+        PostRestyleEventForAnimations(target.mElement,
+                                      eRestyle_Self | eRestyle_Subtree);
+      foundElementsNeedingRestyle = true;
+
       EffectSet* effects =
         EffectSet::GetEffectSet(target.mElement, target.mPseudoType);
       if (!effects) {
-        // Drop the EffectSets that have been destroyed.
+        // Drop EffectSets that have been destroyed.
         iter.Remove();
         continue;
       }
@@ -964,6 +993,7 @@ EffectCompositor::PreTraverse()
       iter.Remove();
     }
   }
+  return foundElementsNeedingRestyle;
 }
 
 void

dom/animation/EffectCompositor.h

@@ -229,7 +229,8 @@ public:
   // Do a bunch of stuff that we should avoid doing during the parallel
   // traversal (e.g. changing member variables) for all elements that we expect
   // to restyle on the next traversal.
-  void PreTraverse();
+  // Returns true if there are elements needing a restyle for animation.
+  bool PreTraverse();
 
   // Similar to the above but only for the (pseudo-)element.
   void PreTraverse(dom::Element* aElement, nsIAtom* aPseudoTagOrNull);

layout/base/ServoRestyleManager.cpp

@@ -79,6 +79,13 @@ ServoRestyleManager::PostRestyleEvent(Element* aElement,
   PostRestyleEventInternal(false);
 }
 
+/* static */ void
+ServoRestyleManager::PostRestyleEventForAnimations(Element* aElement,
+                                                   nsRestyleHint aRestyleHint)
+{
+  Servo_NoteExplicitHints(aElement, aRestyleHint, nsChangeHint(0));
+}
+
 void
 ServoRestyleManager::RebuildAllStyleData(nsChangeHint aExtraHint,
                                          nsRestyleHint aRestyleHint)

layout/base/ServoRestyleManager.h

@@ -92,6 +92,17 @@ public:
    */
   static void ClearServoDataFromSubtree(Element* aElement);
 
+  /**
+   * Posts restyle hints for animations.
+   * This is only called for the second traversal for CSS animations during
+   * updating CSS animations in a SequentialTask.
+   * This function does neither register a refresh observer nor flag that a
+   * style flush is needed since this function is supposed to be called during
+   * restyling process and this restyle event will be processed in the second
+   * traversal of the same restyling process.
+   */
+  static void PostRestyleEventForAnimations(dom::Element* aElement,
+                                            nsRestyleHint aRestyleHint);
 protected:
   ~ServoRestyleManager() override
   {

layout/style/ServoStyleSet.cpp

@@ -226,6 +226,14 @@ ServoStyleSet::PrepareAndTraverseSubtree(RawGeckoElementBorrowed aRoot,
   sInServoTraversal = true;
   bool postTraversalRequired =
     Servo_TraverseSubtree(aRoot, mRawSet.get(), aRootBehavior);
+
+  // If there are still animation restyles needed, trigger a second traversal to
+  // update CSS animations' styles.
+  if (mPresContext->EffectCompositor()->PreTraverse() &&
+      Servo_TraverseSubtree(aRoot, mRawSet.get(), aRootBehavior)) {
+    postTraversalRequired = true;
+  }
+
   sInServoTraversal = false;
   return postTraversalRequired;
 }