Bug 1334036 - Part 1: Avoid mutating mElementsToRestyle during pre-traversal. draft
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 28 Apr 2017 14:00:13 +0800
changeset 581934 783b89bcc22cd660a670b0d0accfa90729919c8e
parent 581933 c269375f4fc7732c9593232e1ec5175e2c567d27
child 581935 c43c3ff1796e471c01a2b772c145b9ee56226e79
push id59916
push userbmo:boris.chiou@gmail.com
push dateSat, 20 May 2017 06:48:29 +0000
bugs1334036
milestone55.0a1
Bug 1334036 - Part 1: Avoid mutating mElementsToRestyle during pre-traversal. During pre-traversal of EffectCompositor, we call MaybeUpdateCascadeResult(), which may add new element into mElementsToRestyle, as a result, we may iterate a mutated mElementsToRestyle. In this patch, we copy the element which needs update cascade results into another set and traverse this new set to call MaybeUpdateCascadeResult(). After that, do normal pre-traversal on mElementsToRestyle. MozReview-Commit-ID: 3uo6Ec5JNjp
dom/animation/EffectCompositor.cpp
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -955,59 +955,98 @@ EffectCompositor::PreTraverse()
 }
 
 bool
 EffectCompositor::PreTraverseInSubtree(Element* aRoot)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mPresContext->RestyleManager()->IsServo());
 
+  using ElementsToRestyleIterType =
+    nsDataHashtable<PseudoElementHashEntry, bool>::Iterator;
+  auto getNeededRestyleTarget = [&](const ElementsToRestyleIterType& aIter)
+                                -> NonOwningAnimationTarget {
+    NonOwningAnimationTarget returnTarget;
+
+    // Ignore throttled restyle.
+    if (!aIter.Data()) {
+      return returnTarget;
+    }
+
+    const NonOwningAnimationTarget& target = aIter.Key();
+
+    // Ignore restyles that aren't in the flattened tree subtree rooted at
+    // aRoot.
+    if (aRoot &&
+        !nsContentUtils::ContentIsFlattenedTreeDescendantOf(target.mElement,
+                                                            aRoot)) {
+      return returnTarget;
+    }
+
+    returnTarget = target;
+    return returnTarget;
+  };
+
   bool foundElementsNeedingRestyle = false;
+
+  nsTArray<NonOwningAnimationTarget> elementsWithCascadeUpdates;
   for (size_t i = 0; i < kCascadeLevelCount; ++i) {
     CascadeLevel cascadeLevel = CascadeLevel(i);
-    for (auto iter = mElementsToRestyle[cascadeLevel].Iter();
-         !iter.Done(); iter.Next()) {
-      bool postedRestyle = iter.Data();
-      // Ignore throttled restyle.
-      if (!postedRestyle) {
+    auto& elementSet = mElementsToRestyle[cascadeLevel];
+    for (auto iter = elementSet.Iter(); !iter.Done(); iter.Next()) {
+      const NonOwningAnimationTarget& target = getNeededRestyleTarget(iter);
+      if (!target.mElement) {
         continue;
       }
 
-      NonOwningAnimationTarget target = iter.Key();
+      EffectSet* effects = EffectSet::GetEffectSet(target.mElement,
+                                                   target.mPseudoType);
+      if (!effects || !effects->CascadeNeedsUpdate()) {
+        continue;
+      }
+
+      elementsWithCascadeUpdates.AppendElement(target);
+    }
+  }
 
-      // Ignore restyles that aren't in the flattened tree subtree rooted at
-      // aRoot.
-      if (aRoot &&
-          !nsContentUtils::ContentIsFlattenedTreeDescendantOf(target.mElement,
-                                                              aRoot)) {
+  for (const NonOwningAnimationTarget& target: elementsWithCascadeUpdates) {
+      MaybeUpdateCascadeResults(target.mElement,
+                                target.mPseudoType);
+  }
+  elementsWithCascadeUpdates.Clear();
+
+  for (size_t i = 0; i < kCascadeLevelCount; ++i) {
+    CascadeLevel cascadeLevel = CascadeLevel(i);
+    auto& elementSet = mElementsToRestyle[cascadeLevel];
+    for (auto iter = elementSet.Iter(); !iter.Done(); iter.Next()) {
+      const NonOwningAnimationTarget& target = getNeededRestyleTarget(iter);
+      if (!target.mElement) {
         continue;
       }
 
       // 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,
                                       cascadeLevel == CascadeLevel::Transitions
                                         ? eRestyle_CSSTransitions
                                         : eRestyle_CSSAnimations);
 
       foundElementsNeedingRestyle = true;
 
-      EffectSet* effects =
-        EffectSet::GetEffectSet(target.mElement, target.mPseudoType);
+      EffectSet* effects = EffectSet::GetEffectSet(target.mElement,
+                                                   target.mPseudoType);
       if (!effects) {
         // Drop EffectSets that have been destroyed.
         iter.Remove();
         continue;
       }
 
-      MaybeUpdateCascadeResults(target.mElement, target.mPseudoType);
-
       for (KeyframeEffectReadOnly* effect : *effects) {
         effect->GetAnimation()->WillComposeStyle();
       }
 
       // Remove the element from the list of elements to restyle since we are
       // about to restyle it.
       iter.Remove();
     }