Bug 1321357 part 1 - Copy elements from mMilestoneEntries before iterating over them; r=dholbert
authorBrian Birtles <birtles@gmail.com>
Fri, 02 Dec 2016 11:22:35 +0900
changeset 325088 8c1e9210ea8585b0a266dff886a7f2cd8a180254
parent 325087 05fe18ec6d95b4068f01bd1c636fbf024162a5d8
child 325089 7004ffd040dc2bbe1aa8fb3bd378cb4ce73d2ff6
push id24
push usermaklebus@msu.edu
push dateTue, 20 Dec 2016 03:11:33 +0000
reviewersdholbert
bugs1321357
milestone53.0a1
Bug 1321357 part 1 - Copy elements from mMilestoneEntries before iterating over them; r=dholbert MozReview-Commit-ID: GjkXYlhoeoy
dom/smil/nsSMILTimeContainer.cpp
layout/style/crashtests/1321357-1.html
layout/style/crashtests/crashtests.list
--- a/dom/smil/nsSMILTimeContainer.cpp
+++ b/dom/smil/nsSMILTimeContainer.cpp
@@ -312,23 +312,28 @@ nsSMILTimeContainer::NotifyTimeChange()
   // time. When this happens time dependencies in other time containers need to
   // re-resolve their times because begin and end times are stored in container
   // time.
   //
   // To get the list of timed elements with dependencies we simply re-use the
   // milestone elements. This is because any timed element with dependents and
   // with significant transitions yet to fire should have their next milestone
   // registered. Other timed elements don't matter.
-  AutoRestore<bool> saveHolding(mHoldingEntries);
-  mHoldingEntries = true;
-  const MilestoneEntry* p = mMilestoneEntries.Elements();
-#if DEBUG
-  uint32_t queueLength = mMilestoneEntries.Length();
-#endif
-  while (p < mMilestoneEntries.Elements() + mMilestoneEntries.Length()) {
-    mozilla::dom::SVGAnimationElement* elem = p->mTimebase.get();
+
+  // Copy the timed elements to a separate array before calling
+  // HandleContainerTimeChange on each of them in case doing so mutates
+  // mMilestoneEntries.
+  nsTArray<RefPtr<mozilla::dom::SVGAnimationElement>> elems;
+
+  {
+    AutoRestore<bool> saveHolding(mHoldingEntries);
+    mHoldingEntries = true;
+    for (const MilestoneEntry* p = mMilestoneEntries.Elements();
+        p < mMilestoneEntries.Elements() + mMilestoneEntries.Length();
+        ++p) {
+      elems.AppendElement(p->mTimebase.get());
+    }
+  }
+
+  for (auto& elem : elems) {
     elem->TimedElement().HandleContainerTimeChange();
-    MOZ_ASSERT(queueLength == mMilestoneEntries.Length(),
-               "Call to HandleContainerTimeChange resulted in a change to the "
-               "queue of milestones");
-    ++p;
   }
 }
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1321357-1.html
@@ -0,0 +1,12 @@
+<!doctype html>
+<html>
+<body onload="document.getElementById('containerA').pauseAnimations()">
+  <svg id="containerA">
+    <animate id="ia" end="50s"></animate>
+    <animate begin="60s" end="ic.end"></animate>
+  </svg>
+  <svg>
+    <animate id="ic" end="ia.end"></animate>
+  </svg>
+</body>
+</html>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -159,8 +159,9 @@ pref(dom.animations-api.core.enabled,tru
 pref(dom.animations-api.core.enabled,true) load 1290994-2.html
 pref(dom.animations-api.core.enabled,true) load 1290994-3.html
 load 1290994-4.html
 load 1314531.html
 load 1315889-1.html
 load 1315894-1.html
 load 1319072-1.html
 HTTP load 1320423-1.html
+load 1321357-1.html