Bug 1525509 - Add release asserts. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 15 Feb 2019 06:29:55 +0000
changeset 459585 95b6997c334afa45b588408bc25b99c9f26b391d
parent 459584 3f18f4d9dc1e7e0d226267dea1da72765126f809
child 459586 64eed6f2cc174243c9a17b4583e75e2256d2cd70
push id35563
push userccoroiu@mozilla.com
push dateSat, 16 Feb 2019 09:36:04 +0000
treeherdermozilla-central@1cfd69d05aa1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1525509
milestone67.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 1525509 - Add release asserts. r=dholbert Just for my sanity. I think the other scroll observer is sane after a quick look, but this will ensure we don't ship security issues. Differential Revision: https://phabricator.services.mozilla.com/D19725
layout/base/PresShell.cpp
layout/base/RestyleManager.cpp
layout/base/nsIPresShell.h
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -775,16 +775,17 @@ nsIPresShell::nsIPresShell()
       mCanvasBackgroundColor(NS_RGBA(0, 0, 0, 0)),
       mSelectionFlags(0),
       mChangeNestCount(0),
       mRenderFlags(0),
       mDidInitialize(false),
       mIsDestroying(false),
       mIsReflowing(false),
       mIsObservingDocument(false),
+      mForbiddenToFlush(false),
       mIsDocumentGone(false),
       mHaveShutDown(false),
       mPaintingSuppressed(false),
       mLastRootReflowHadUnconstrainedBSize(false),
       mShouldUnsuppressPainting(false),
       mIgnoreFrameDestruction(false),
       mIsActive(false),
       mFrozen(false),
@@ -863,16 +864,19 @@ PresShell::PresShell()
   mHasReceivedPaintMessage = false;
 }
 
 NS_IMPL_ISUPPORTS(PresShell, nsIPresShell, nsIDocumentObserver,
                   nsISelectionController, nsISelectionDisplay, nsIObserver,
                   nsISupportsWeakReference, nsIMutationObserver)
 
 PresShell::~PresShell() {
+  MOZ_RELEASE_ASSERT(!mForbiddenToFlush,
+                     "Flag should only be set temporarily, while doing things "
+                     "that shouldn't cause destruction");
   MOZ_LOG(gLog, LogLevel::Debug, ("PresShell::~PresShell this=%p", this));
 
   if (!mHaveShutDown) {
     MOZ_ASSERT_UNREACHABLE("Someone did not call nsIPresShell::destroy");
     Destroy();
   }
 
   NS_ASSERTION(mCurrentEventContentStack.Count() == 0,
@@ -3972,16 +3976,18 @@ static inline void AssertFrameTreeIsSane
 #ifdef DEBUG
   if (const nsIFrame* root = aShell.GetRootFrame()) {
     AssertFrameSubtreeIsSane(*root);
   }
 #endif
 }
 
 void PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush aFlush) {
+  MOZ_RELEASE_ASSERT(!mForbiddenToFlush, "This is bad!");
+
   // Per our API contract, hold a strong ref to ourselves until we return.
   nsCOMPtr<nsIPresShell> kungFuDeathGrip = this;
 
   /**
    * VERY IMPORTANT: If you add some sort of new flushing to this
    * method, make sure to add the relevant SetNeedLayoutFlush or
    * SetNeedStyleFlush calls on the shell.
    */
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -2978,17 +2978,17 @@ bool RestyleManager::ProcessPostTraversa
 void RestyleManager::ClearSnapshots() {
   for (auto iter = mSnapshots.Iter(); !iter.Done(); iter.Next()) {
     iter.Key()->UnsetFlags(ELEMENT_HAS_SNAPSHOT | ELEMENT_HANDLED_SNAPSHOT);
     iter.Remove();
   }
 }
 
 ServoElementSnapshot& RestyleManager::SnapshotFor(Element& aElement) {
-  MOZ_ASSERT(!mInStyleRefresh);
+  MOZ_RELEASE_ASSERT(!mInStyleRefresh);
 
   // NOTE(emilio): We can handle snapshots from a one-off restyle of those that
   // we do to restyle stuff for reconstruction, for example.
   //
   // It seems to be the case that we always flush in between that happens and
   // the next attribute change, so we can assert that we haven't handled the
   // snapshot here yet. If this assertion didn't hold, we'd need to unset that
   // flag from here too.
@@ -3003,38 +3003,42 @@ ServoElementSnapshot& RestyleManager::Sn
 
   // Now that we have a snapshot, make sure a restyle is triggered.
   aElement.NoteDirtyForServo();
   return *snapshot;
 }
 
 void RestyleManager::DoProcessPendingRestyles(ServoTraversalFlags aFlags) {
   nsPresContext* presContext = PresContext();
+  nsIPresShell* shell = presContext->PresShell();
 
   MOZ_ASSERT(presContext->Document(), "No document?  Pshaw!");
   // FIXME(emilio): In the "flush animations" case, ideally, we should only
   // recascade animation styles running on the compositor, so we shouldn't care
   // about other styles, or new rules that apply to the page...
   //
   // However, that's not true as of right now, see bug 1388031 and bug 1388692.
   MOZ_ASSERT((aFlags & ServoTraversalFlags::FlushThrottledAnimations) ||
                  !presContext->HasPendingMediaQueryUpdates(),
              "Someone forgot to update media queries?");
   MOZ_ASSERT(!nsContentUtils::IsSafeToRunScript(), "Missing a script blocker!");
-  MOZ_ASSERT(!mInStyleRefresh, "Reentrant call?");
-
-  if (MOZ_UNLIKELY(!presContext->PresShell()->DidInitialize())) {
+  MOZ_RELEASE_ASSERT(!mInStyleRefresh, "Reentrant call?");
+
+  if (MOZ_UNLIKELY(!shell->DidInitialize())) {
     // PresShell::FlushPendingNotifications doesn't early-return in the case
     // where the PresShell hasn't yet been initialized (and therefore we haven't
     // yet done the initial style traversal of the DOM tree). We should arguably
     // fix up the callers and assert against this case, but we just detect and
     // handle it for now.
     return;
   }
 
+  // It'd be bad!
+  nsIPresShell::AutoAssertNoFlush noReentrantFlush(*shell);
+
   // Create a AnimationsWithDestroyedFrame during restyling process to
   // stop animations and transitions on elements that have no frame at the end
   // of the restyling process.
   AnimationsWithDestroyedFrame animationsWithDestroyedFrame(this);
 
   ServoStyleSet* styleSet = StyleSet();
   Document* doc = presContext->Document();
 
@@ -3190,17 +3194,17 @@ void RestyleManager::UpdateOnlyAnimation
     return;
   }
 
   DoProcessPendingRestyles(ServoTraversalFlags::FlushThrottledAnimations);
 }
 
 void RestyleManager::ContentStateChanged(nsIContent* aContent,
                                          EventStates aChangedBits) {
-  MOZ_ASSERT(!mInStyleRefresh);
+  MOZ_RELEASE_ASSERT(!mInStyleRefresh);
 
   if (!aContent->IsElement()) {
     return;
   }
 
   Element& element = *aContent->AsElement();
   if (!element.HasServoData()) {
     return;
@@ -3307,17 +3311,17 @@ void RestyleManager::AttributeWillChange
 void RestyleManager::ClassAttributeWillBeChangedBySMIL(Element* aElement) {
   TakeSnapshotForAttributeChange(*aElement, kNameSpaceID_None,
                                  nsGkAtoms::_class);
 }
 
 void RestyleManager::TakeSnapshotForAttributeChange(Element& aElement,
                                                     int32_t aNameSpaceID,
                                                     nsAtom* aAttribute) {
-  MOZ_ASSERT(!mInStyleRefresh);
+  MOZ_RELEASE_ASSERT(!mInStyleRefresh);
 
   if (!aElement.HasServoData()) {
     return;
   }
 
   bool influencesOtherPseudoClassState;
   if (!NeedToRecordAttrChange(*StyleSet(), aElement, aNameSpaceID, aAttribute,
                               &influencesOtherPseudoClassState)) {
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -1733,16 +1733,28 @@ class nsIPresShell : public nsStubDocume
   virtual Document* GetPrimaryContentDocument() = 0;
 
   // aSheetType is one of the nsIStyleSheetService *_SHEET constants.
   void NotifyStyleSheetServiceSheetAdded(mozilla::StyleSheet* aSheet,
                                          uint32_t aSheetType);
   void NotifyStyleSheetServiceSheetRemoved(mozilla::StyleSheet* aSheet,
                                            uint32_t aSheetType);
 
+  struct MOZ_RAII AutoAssertNoFlush {
+    explicit AutoAssertNoFlush(nsIPresShell& aShell)
+        : mShell(aShell), mOldForbidden(mShell.mForbiddenToFlush) {
+      mShell.mForbiddenToFlush = true;
+    }
+
+    ~AutoAssertNoFlush() { mShell.mForbiddenToFlush = mOldForbidden; }
+
+    nsIPresShell& mShell;
+    const bool mOldForbidden;
+  };
+
  protected:
   friend class nsRefreshDriver;
   friend class ::nsAutoCauseReflowNotifier;
 
   void WillCauseReflow();
   void DidCauseReflow();
 
   void CancelPostedReflowCallbacks();
@@ -1965,16 +1977,21 @@ class nsIPresShell : public nsStubDocume
   // PresShell flushes retained layers when the rendering state
   // changes in a way that prevents us from being able to (usefully)
   // re-use old pixels.
   RenderFlags mRenderFlags;
   bool mDidInitialize : 1;
   bool mIsDestroying : 1;
   bool mIsReflowing : 1;
   bool mIsObservingDocument : 1;
+  // Whether we shouldn't ever get to FlushPendingNotifications. This flag is
+  // meant only to sanity-check / assert that FlushPendingNotifications doesn't
+  // happen during certain periods of time. It shouldn't be made public nor used
+  // for other purposes.
+  bool mForbiddenToFlush : 1;
 
   // We've been disconnected from the document.  We will refuse to paint the
   // document until either our timer fires or all frames are constructed.
   bool mIsDocumentGone : 1;
   bool mHaveShutDown : 1;
 
   // For all documents we initially lock down painting.
   bool mPaintingSuppressed : 1;