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 359066 181d381af5dfff2cafcd816be6167a1897d963a0
parent 359065 857a38ebade2a61264d5a7e3c0200e17d7085c8c
child 359067 ddcf602cb70753ecbd965a67ad50fcd41b55639e
push id1324
push usermtabara@mozilla.com
push dateMon, 16 Jan 2017 13:07:44 +0000
treeherdermozilla-release@a01c49833940 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, jcristau
bugs1321357
milestone51.0
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
@@ -152,8 +152,9 @@ pref(layout.css.background-clip-text.ena
 pref(layout.css.background-clip-text.enabled,true) load 1275026.html
 load 1278463-1.html
 pref(dom.animations-api.core.enabled,true) load 1277908-1.html
 load 1277908-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 1321357-1.html