Bug 1350115 - Squelch post-traversal generated by additional animation traversals when we're styling a fresh subtree. r=heycam,r=birtles

This patch exists to avoid a crash in layout/style/test/test_animations.html. We end up
generating some ::before content, which causes us to style the new subtree at [1]. In
StyleNewSubtree, we fail the !postTraversalRequired assertion because
PrepareAndTraverseSubtree decided to traverse the tree twice (once to style it, and again
to restyle it for animations), and return that a post-traversal is needed.

The reason this issue happens with my NAC patches and not without is that we were previously
filtering out generated ::before content from the servo traversal, so the servo traversal
wouldn't have reached it and (presumably) the animation restyle wouldn't have happened and
we wouldn't have returned true for needing a post-traversal.

[1] http://searchfox.org/mozilla-central/rev/c48398abd9f0f074c69f2223260939e30e8f99a8/layout/base/nsCSSFrameConstructor.cpp#1918

MozReview-Commit-ID: 8tgzLjV8B3A

dom/base/Element.h

@@ -453,7 +453,7 @@ public:
   inline bool DirtyDescendantsBitIsPropagatedForServo();
 #endif
 
-  bool HasServoData() {
+  bool HasServoData() const {
 #ifdef MOZ_STYLO
     return !!mServoData.Get();
 #else

layout/base/ServoRestyleManager.cpp

@@ -111,10 +111,8 @@ ServoRestyleManager::ClearServoDataFromSubtree(Element* aElement)
 }
 
 
-// Clears HasDirtyDescendants and RestyleData from all elements in the
-// subtree rooted at aElement.
-static void
-ClearRestyleStateFromSubtree(Element* aElement)
+/* static */ void
+ServoRestyleManager::ClearRestyleStateFromSubtree(Element* aElement)
 {
   if (aElement->HasDirtyDescendantsForServo()) {
     StyleChildrenIterator it(aElement);

layout/base/ServoRestyleManager.h

@@ -91,6 +91,12 @@ public:
    */
   static void ClearServoDataFromSubtree(Element* aElement);
 
+  /**
+   * Clears HasDirtyDescendants and RestyleData from all elements in the
+   * subtree rooted at aElement.
+   */
+  static void ClearRestyleStateFromSubtree(Element* aElement);
+
   /**
    * Posts restyle hints for animations.
    * This is only called for the second traversal for CSS animations during

layout/style/ServoStyleSet.cpp

@@ -220,14 +220,32 @@ ServoStyleSet::PrepareAndTraverseSubtree(RawGeckoElementBorrowed aRoot,
 
   MOZ_ASSERT(!sInServoTraversal);
   sInServoTraversal = true;
+
+  bool isInitial = !aRoot->HasServoData();
   bool postTraversalRequired =
     Servo_TraverseSubtree(aRoot, mRawSet.get(), aRootBehavior);
+  MOZ_ASSERT_IF(isInitial, !postTraversalRequired);
 
   // 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;
+  if (mPresContext->EffectCompositor()->PreTraverse()) {
+    if (Servo_TraverseSubtree(aRoot, mRawSet.get(), aRootBehavior)) {
+      if (isInitial) {
+        // We're doing initial styling, and the additional animation
+        // traversal changed the styles that were set by the first traversal.
+        // This would normally require a post-traversal to update the style
+        // contexts, and the DOM now has dirty descendant bits and RestyleData
+        // in expectation of that post-traversal. But since this is actually
+        // the initial styling, there are no style contexts to update and no
+        // frames to apply the change hints to, so we don't need to do that
+        // post-traversal. Instead, just drop this state and tell the caller
+        // that no post-traversal is required.
+        MOZ_ASSERT(!postTraversalRequired);
+        ServoRestyleManager::ClearRestyleStateFromSubtree(const_cast<Element*>(aRoot));
+      } else {
+        postTraversalRequired = true;
+      }
+    }
   }
 
   sInServoTraversal = false;