Bug 1321357 part 1 - Copy elements from mMilestoneEntries before iterating over them; r=dholbert a=jcristau
authorBrian Birtles <birtles@gmail.com>
Fri, 02 Dec 2016 11:22:35 +0900
changeset 352858 6c2cb17c6db341f49599522c4fb1350b7c37b9e0
parent 352857 65b9d6f30e5201f6ee0208956a29890c8c0f0283
child 352859 7a3131a3f75177799898ef2b97b278609a1bb8c2
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, jcristau
bugs1321357
milestone52.0a2
Bug 1321357 part 1 - Copy elements from mMilestoneEntries before iterating over them; r=dholbert a=jcristau 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
@@ -157,8 +157,9 @@ load 1282076-1.html
 pref(dom.animations-api.core.enabled,true) load 1282076-2.html
 pref(dom.animations-api.core.enabled,true) load 1290994-1.html
 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 1321357-1.html