Bug 1321066 - Explicitly guard against reentrance in nsSMILTimeContainer. r=dholbert
authorAndrew McCreight <continuation@gmail.com>
Tue, 29 Nov 2016 15:57:30 -0800
changeset 324701 9271347b07d201df26cdffde75483c0b0001528c
parent 324700 539af8b2ccc96cfb689534178b1e59cd9cf8ba95
child 324702 f4a2a329a7080cab9b7ebd1eb4b3295a1232b775
push id84484
push useramccreight@mozilla.com
push dateTue, 29 Nov 2016 23:57:42 +0000
treeherdermozilla-inbound@9271347b07d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1321066
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1321066 - Explicitly guard against reentrance in nsSMILTimeContainer. r=dholbert
dom/smil/nsSMILTimeContainer.cpp
dom/smil/nsSMILTimeContainer.h
--- a/dom/smil/nsSMILTimeContainer.cpp
+++ b/dom/smil/nsSMILTimeContainer.cpp
@@ -4,25 +4,28 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsSMILTimeContainer.h"
 #include "nsSMILTimeValue.h"
 #include "nsSMILTimedElement.h"
 #include <algorithm>
 
+#include "mozilla/AutoRestore.h"
+
 nsSMILTimeContainer::nsSMILTimeContainer()
 :
   mParent(nullptr),
   mCurrentTime(0L),
   mParentOffset(0L),
   mPauseStart(0L),
   mNeedsPauseSample(false),
   mNeedsRewind(false),
   mIsSeeking(false),
+  mHoldingEntries(false),
   mPauseState(PAUSE_BEGIN)
 {
 }
 
 nsSMILTimeContainer::~nsSMILTimeContainer()
 {
   if (mParent) {
     mParent->RemoveChild(*this);
@@ -208,22 +211,24 @@ nsSMILTimeContainer::SetParent(nsSMILTim
 bool
 nsSMILTimeContainer::AddMilestone(const nsSMILMilestone& aMilestone,
                                   mozilla::dom::SVGAnimationElement& aElement)
 {
   // We record the milestone time and store it along with the element but this
   // time may change (e.g. if attributes are changed on the timed element in
   // between samples). If this happens, then we may do an unecessary sample
   // but that's pretty cheap.
+  MOZ_RELEASE_ASSERT(!mHoldingEntries);
   return mMilestoneEntries.Push(MilestoneEntry(aMilestone, aElement));
 }
 
 void
 nsSMILTimeContainer::ClearMilestones()
 {
+  MOZ_RELEASE_ASSERT(!mHoldingEntries);
   mMilestoneEntries.Clear();
 }
 
 bool
 nsSMILTimeContainer::GetNextMilestoneInParentTime(
     nsSMILMilestone& aNextMilestone) const
 {
   if (mMilestoneEntries.IsEmpty())
@@ -254,41 +259,46 @@ nsSMILTimeContainer::PopMilestoneElement
 
   nsSMILMilestone containerMilestone(containerTime.GetMillis(),
                                      aMilestone.mIsEnd);
 
   MOZ_ASSERT(mMilestoneEntries.Top().mMilestone >= containerMilestone,
              "Trying to pop off earliest times but we have earlier ones that "
              "were overlooked");
 
+  MOZ_RELEASE_ASSERT(!mHoldingEntries);
+
   bool gotOne = false;
   while (!mMilestoneEntries.IsEmpty() &&
       mMilestoneEntries.Top().mMilestone == containerMilestone)
   {
     aMatchedElements.AppendElement(mMilestoneEntries.Pop().mTimebase);
     gotOne = true;
   }
 
   return gotOne;
 }
 
 void
 nsSMILTimeContainer::Traverse(nsCycleCollectionTraversalCallback* aCallback)
 {
+  AutoRestore<bool> saveHolding(mHoldingEntries);
+  mHoldingEntries = true;
   const MilestoneEntry* p = mMilestoneEntries.Elements();
   while (p < mMilestoneEntries.Elements() + mMilestoneEntries.Length()) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*aCallback, "mTimebase");
     aCallback->NoteXPCOMChild(static_cast<nsIContent*>(p->mTimebase.get()));
     ++p;
   }
 }
 
 void
 nsSMILTimeContainer::Unlink()
 {
+  MOZ_RELEASE_ASSERT(!mHoldingEntries);
   mMilestoneEntries.Clear();
 }
 
 void
 nsSMILTimeContainer::UpdateCurrentTime()
 {
   nsSMILTime now = IsPaused() ? mPauseStart : GetParentTime();
   mCurrentTime = now - mParentOffset;
@@ -302,16 +312,18 @@ 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();
     elem->TimedElement().HandleContainerTimeChange();
     MOZ_ASSERT(queueLength == mMilestoneEntries.Length(),
--- a/dom/smil/nsSMILTimeContainer.h
+++ b/dom/smil/nsSMILTimeContainer.h
@@ -261,16 +261,18 @@ protected:
   nsSMILTime mPauseStart;
 
   // Whether or not a pause sample is required
   bool mNeedsPauseSample;
 
   bool mNeedsRewind; // Backwards seek performed
   bool mIsSeeking; // Currently in the middle of a seek operation
 
+  bool mHoldingEntries; // True if there's a raw pointer to mMilestoneEntries on the stack.
+
   // A bitfield of the pause state for all pause requests
   uint32_t mPauseState;
 
   struct MilestoneEntry
   {
     MilestoneEntry(nsSMILMilestone aMilestone,
                    mozilla::dom::SVGAnimationElement& aElement)
       : mMilestone(aMilestone), mTimebase(&aElement)