From 887595286ba92ba9e50cb7d3fa882340bb3ff2af Mon Sep 17 00:00:00 2001
From: Bobby Holley <bobbyholley@gmail.com>
Date: Thu, 23 Mar 2017 15:21:18 -0700
Subject: [PATCH] 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                  |  2 +-
 layout/base/ServoRestyleManager.cpp |  6 ++----
 layout/base/ServoRestyleManager.h   |  6 ++++++
 layout/style/ServoStyleSet.cpp      | 24 +++++++++++++++++++++---
 4 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/dom/base/Element.h b/dom/base/Element.h
index 95d76f1a1f08..83d2fb7308e8 100644
--- a/dom/base/Element.h
+++ b/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
diff --git a/layout/base/ServoRestyleManager.cpp b/layout/base/ServoRestyleManager.cpp
index 4bdee54c0be4..f02a1eabc77e 100644
--- a/layout/base/ServoRestyleManager.cpp
+++ b/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);
diff --git a/layout/base/ServoRestyleManager.h b/layout/base/ServoRestyleManager.h
index e34db1040194..e0de00cc810f 100644
--- a/layout/base/ServoRestyleManager.h
+++ b/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
diff --git a/layout/style/ServoStyleSet.cpp b/layout/style/ServoStyleSet.cpp
index 454b673b37a8..229a65271732 100644
--- a/layout/style/ServoStyleSet.cpp
+++ b/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;