Bug 1321066 - Explicitly guard against reentrance in nsSMILTimeContainer. r=dholbert, a=lizzard SEA_COMM490_20160927_RELBRANCH SEAMONKEY_2_46_BUILD7 SEAMONKEY_2_46_BUILD8
authorAndrew McCreight <continuation@gmail.com>
Tue, 29 Nov 2016 15:57:30 -0800
branchSEA_COMM490_20160927_RELBRANCH
changeset 350995 8b8d2820718f
parent 350994 c8c24c1f96ec
child 350996 be230a305072
push id1310
push userewong@pw-wspx.org
push date2016-12-13 07:26 +0000
treeherdermozilla-release@8b8d2820718f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert, lizzard
bugs1321066
milestone49.0.2
Bug 1321066 - Explicitly guard against reentrance in nsSMILTimeContainer. r=dholbert, a=lizzard
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
@@ -262,16 +262,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)