Bug 1420547: Notify the pres shell specially, instead of via mutation observers. r=bz
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 06 Dec 2017 05:26:32 +0100
changeset 448822 46e5fc9628c836db4236cc9c4e6481bfb547e40a
parent 448821 dd70ec8799fff506944df6c1346617da85dc6383
child 448823 e482d3fc80e40910ee6944beb5b4f2e889d0d93c
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1420547
milestone59.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 1420547: Notify the pres shell specially, instead of via mutation observers. r=bz This makes the pres shell be notified as the last observer unconditionally. In practice this doesn't matter, and it may already be the case if an iframe goes display: none and back. In practice, the only dependency that requires this order is that the pres shell needs to be notified _after_ the content sink, so we don't try to enter frame construction before beginning the shell update. That may be worth looking into, but definitely not in the scope of this bug... :) MozReview-Commit-ID: 9WeJ5kaUtBq
dom/base/nsDocument.h
dom/base/nsIDocument.h
dom/base/nsNodeUtils.cpp
layout/base/PresShell.cpp
layout/base/PresShell.h
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsIPresShell.h
--- a/dom/base/nsDocument.h
+++ b/dom/base/nsDocument.h
@@ -1057,19 +1057,25 @@ protected:
   virtual nsIScriptGlobalObject* GetScriptHandlingObjectInternal() const override;
   virtual bool InternalAllowXULXBL() override;
 
   void UpdateScreenOrientation();
 
   virtual mozilla::dom::FlashClassification DocumentFlashClassification() override;
   virtual bool IsThirdParty() override;
 
-#define NS_DOCUMENT_NOTIFY_OBSERVERS(func_, params_)                        \
-  NS_OBSERVER_ARRAY_NOTIFY_XPCOM_OBSERVERS(mObservers, nsIDocumentObserver, \
-                                           func_, params_);
+#define NS_DOCUMENT_NOTIFY_OBSERVERS(func_, params_) do {                     \
+    NS_OBSERVER_ARRAY_NOTIFY_XPCOM_OBSERVERS(mObservers, nsIDocumentObserver, \
+                                             func_, params_);                 \
+    /* FIXME(emilio): Apparently we can keep observing from the BFCache? That \
+       looks bogus. */                                                        \
+    if (nsIPresShell* shell = GetObservingShell()) {                          \
+      shell->func_ params_;                                                   \
+    }                                                                         \
+  } while(0)
 
 #ifdef DEBUG
   void VerifyRootContentState();
 #endif
 
   explicit nsDocument(const char* aContentType);
   virtual ~nsDocument();
 
--- a/dom/base/nsIDocument.h
+++ b/dom/base/nsIDocument.h
@@ -11,16 +11,17 @@
 #include "nsCOMArray.h"                  // for member
 #include "nsCompatibility.h"             // for member
 #include "nsCOMPtr.h"                    // for member
 #include "nsGkAtoms.h"                   // for static class members
 #include "nsIDocumentObserver.h"         // for typedef (nsUpdateType)
 #include "nsILoadGroup.h"                // for member (in nsCOMPtr)
 #include "nsINode.h"                     // for base class
 #include "nsIParser.h"
+#include "nsIPresShell.h"
 #include "nsIScriptGlobalObject.h"       // for member (in nsCOMPtr)
 #include "nsIServiceManager.h"
 #include "nsIUUIDGenerator.h"
 #include "nsPIDOMWindow.h"               // for use in inline functions
 #include "nsPropertyTable.h"             // for member
 #include "nsStringFwd.h"
 #include "nsDataHashtable.h"             // for member
 #include "nsURIHashKey.h"                // for member
@@ -81,17 +82,16 @@ class nsIDOMDocumentType;
 class nsIDOMElement;
 class nsIDOMNodeFilter;
 class nsIDOMNodeList;
 class nsIHTMLCollection;
 class nsILayoutHistoryState;
 class nsILoadContext;
 class nsIObjectLoadingContent;
 class nsIObserver;
-class nsIPresShell;
 class nsIPrincipal;
 class nsIRequest;
 class nsIRunnable;
 class nsIStreamListener;
 class nsIStructuredCloneContainer;
 class nsIURI;
 class nsIVariant;
 class nsViewManager;
@@ -933,16 +933,22 @@ public:
       mozilla::StyleSetHandle aStyleSet) = 0;
   virtual void DeleteShell() = 0;
 
   nsIPresShell* GetShell() const
   {
     return GetBFCacheEntry() ? nullptr : mPresShell;
   }
 
+  nsIPresShell* GetObservingShell() const
+  {
+    return mPresShell && mPresShell->IsObservingDocument()
+      ? mPresShell : nullptr;
+  }
+
   bool HasShellOrBFCacheEntry() const
   {
     return mPresShell || mBFCacheEntry;
   }
 
   // Instead using this method, what you probably want is
   // RemoveFromBFCacheSync() as we do in MessagePort and BroadcastChannel.
   void DisallowBFCaching()
--- a/dom/base/nsNodeUtils.cpp
+++ b/dom/base/nsNodeUtils.cpp
@@ -48,30 +48,36 @@ using mozilla::AutoJSContext;
   PR_BEGIN_MACRO                                                  \
   bool needsEnterLeave = doc->MayHaveDOMMutationObservers();      \
   if (needsEnterLeave) {                                          \
     nsDOMMutationObserver::EnterMutationHandling();               \
   }                                                               \
   nsINode* node = content_;                                       \
   NS_ASSERTION(node->OwnerDoc() == doc, "Bogus document");        \
   doc->BindingManager()->func_ params_;                           \
+  nsINode* last;                                                  \
   do {                                                            \
     nsINode::nsSlots* slots = node->GetExistingSlots();           \
     if (slots && !slots->mMutationObservers.IsEmpty()) {          \
       NS_OBSERVER_AUTO_ARRAY_NOTIFY_OBSERVERS(                    \
         slots->mMutationObservers, nsIMutationObserver, 1,        \
         func_, params_);                                          \
     }                                                             \
-    ShadowRoot* shadow = ShadowRoot::FromNode(node);              \
-    if (shadow) {                                                 \
+    last = node;                                                  \
+    if (ShadowRoot* shadow = ShadowRoot::FromNode(node)) {        \
       node = shadow->GetHost();                                   \
     } else {                                                      \
       node = node->GetParentNode();                               \
     }                                                             \
   } while (node);                                                 \
+  if (last == doc) {                                              \
+    if (nsIPresShell* shell = doc->GetObservingShell()) {         \
+      shell->func_ params_;                                       \
+    }                                                             \
+  }                                                               \
   if (needsEnterLeave) {                                          \
     nsDOMMutationObserver::LeaveMutationHandling();               \
   }                                                               \
   PR_END_MACRO
 
 #define IMPL_ANIMATION_NOTIFICATION(func_, content_, params_)     \
   PR_BEGIN_MACRO                                                  \
   bool needsEnterLeave = doc->MayHaveDOMMutationObservers();      \
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -782,16 +782,18 @@ nsIPresShell::nsIPresShell()
     , mAutoWeakFrames(nullptr)
     , mCanvasBackgroundColor(NS_RGBA(0,0,0,0))
     , mSelectionFlags(0)
     , mChangeNestCount(0)
     , mRenderFlags(0)
     , mDidInitialize(false)
     , mIsDestroying(false)
     , mIsReflowing(false)
+    , mIsObservingDocument(false)
+    , mIsDocumentGone(false)
     , mPaintingSuppressed(false)
     , mIsActive(false)
     , mFrozen(false)
     , mIsFirstPaint(false)
     , mObservesMutationsForPrint(false)
     , mWasLastReflowInterrupted(false)
     , mScrollPositionClampingScrollPortSizeSet(false)
     , mNeedLayoutFlush(true)
@@ -829,17 +831,16 @@ PresShell::PresShell()
   , mLastAnchorScrollPositionY(0)
   , mAPZFocusSequenceNumber(0)
   , mDocumentLoading(false)
   , mIgnoreFrameDestruction(false)
   , mHaveShutDown(false)
   , mLastRootReflowHadUnconstrainedBSize(false)
   , mNoDelayedMouseEvents(false)
   , mNoDelayedKeyEvents(false)
-  , mIsDocumentGone(false)
   , mShouldUnsuppressPainting(false)
   , mResizeEventPending(false)
   , mApproximateFrameVisibilityVisited(false)
   , mNextPaintCompressed(false)
   , mHasCSSBackgroundColor(false)
   , mScaleToResolution(false)
   , mIsLastChromeOnlyEscapeKeyConsumed(false)
   , mHasReceivedPaintMessage(false)
@@ -1673,38 +1674,36 @@ PresShell::RepaintSelection(RawSelection
     return NS_ERROR_NULL_POINTER;
 
   RefPtr<nsFrameSelection> frameSelection = mSelection;
   return frameSelection->RepaintSelection(ToSelectionType(aRawSelectionType));
 }
 
 // Make shell be a document observer
 void
-PresShell::BeginObservingDocument()
+nsIPresShell::BeginObservingDocument()
 {
   if (mDocument && !mIsDestroying) {
-    mDocument->AddObserver(this);
+    mIsObservingDocument = true;
     if (mIsDocumentGone) {
       NS_WARNING("Adding a presshell that was disconnected from the document "
                  "as a document observer?  Sounds wrong...");
       mIsDocumentGone = false;
     }
   }
 }
 
 // Make shell stop being a document observer
 void
-PresShell::EndObservingDocument()
+nsIPresShell::EndObservingDocument()
 {
   // XXXbz do we need to tell the frame constructor that the document
   // is gone, perhaps?  Except for printing it's NOT gone, sometimes.
   mIsDocumentGone = true;
-  if (mDocument) {
-    mDocument->RemoveObserver(this);
-  }
+  mIsObservingDocument = false;
 }
 
 #ifdef DEBUG_kipp
 char* nsPresShell_ReflowStackPointerTop;
 #endif
 
 class XBLConstructorRunner : public Runnable
 {
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -66,17 +66,16 @@ typedef nsClassHashtable<nsUint64HashKey
 // to get the pref for any reason.
 #ifdef MOZ_WIDGET_ANDROID
 #define PAINTLOCK_EVENT_DELAY 250
 #else
 #define PAINTLOCK_EVENT_DELAY 5
 #endif
 
 class PresShell final : public nsIPresShell,
-                        public nsStubDocumentObserver,
                         public nsISelectionController,
                         public nsIObserver,
                         public nsSupportsWeakReference
 {
 protected:
   typedef mozilla::layers::FocusTarget FocusTarget;
 
 public:
@@ -104,18 +103,16 @@ public:
 
   NS_IMETHOD SetDisplaySelection(int16_t aToggle) override;
   NS_IMETHOD GetDisplaySelection(int16_t *aToggle) override;
   NS_IMETHOD ScrollSelectionIntoView(RawSelectionType aRawSelectionType,
                                      SelectionRegion aRegion,
                                      int16_t aFlags) override;
   NS_IMETHOD RepaintSelection(RawSelectionType aRawSelectionType) override;
 
-  virtual void BeginObservingDocument() override;
-  virtual void EndObservingDocument() override;
   virtual nsresult Initialize(nscoord aWidth, nscoord aHeight) override;
   virtual nsresult ResizeReflow(nscoord aWidth, nscoord aHeight,
                                 nscoord aOldWidth = 0, nscoord aOldHeight = 0,
                                 ResizeReflowOptions aOptions =
                                 ResizeReflowOptions::eBSizeExact) override;
   virtual nsresult ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
                                               nscoord aOldWidth, nscoord aOldHeight,
                                               ResizeReflowOptions aOptions =
@@ -862,20 +859,16 @@ protected:
 
   bool                      mDocumentLoading : 1;
   bool                      mIgnoreFrameDestruction : 1;
   bool                      mHaveShutDown : 1;
   bool                      mLastRootReflowHadUnconstrainedBSize : 1;
   bool                      mNoDelayedMouseEvents : 1;
   bool                      mNoDelayedKeyEvents : 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;
-
   // Indicates that it is safe to unlock painting once all pending reflows
   // have been processed.
   bool                      mShouldUnsuppressPainting : 1;
 
   bool                      mResizeEventPending : 1;
 
   bool                      mApproximateFrameVisibilityVisited : 1;
 
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -5121,19 +5121,17 @@ nsCSSFrameConstructor::ConstructNonScrol
 
 void
 nsCSSFrameConstructor::InitAndRestoreFrame(const nsFrameConstructorState& aState,
                                            nsIContent*              aContent,
                                            nsContainerFrame*        aParentFrame,
                                            nsIFrame*                aNewFrame,
                                            bool                     aAllowCounters)
 {
-  NS_PRECONDITION(mUpdateCount != 0,
-                  "Should be in an update while creating frames");
-
+  MOZ_ASSERT(mUpdateCount != 0, "Should be in an update while creating frames");
   MOZ_ASSERT(aNewFrame, "Null frame cannot be initialized");
 
   // Initialize the frame
   aNewFrame->Init(aContent, aParentFrame, nullptr);
   aNewFrame->AddStateBits(aState.mAdditionalStateBits);
 
   if (aState.mFrameState) {
     // Restore frame state for just the newly created frame.
@@ -7465,18 +7463,17 @@ nsCSSFrameConstructor::ContentAppended(n
                                        TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT(!aProvidedTreeMatchContext ||
              aInsertionKind == InsertionKind::Sync);
   MOZ_ASSERT(aInsertionKind == InsertionKind::Sync ||
              !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
-  NS_PRECONDITION(mUpdateCount != 0,
-                  "Should be in an update while creating frames");
+  MOZ_ASSERT(mUpdateCount != 0, "Should be in an update while creating frames");
 
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentAppended container=%p "
            "first-child=%p lazy=%d\n",
            static_cast<void*>(aContainer), aFirstNewContent,
            aInsertionKind == InsertionKind::Async);
     if (gReallyNoisyContentUpdates && aContainer) {
@@ -7869,18 +7866,17 @@ nsCSSFrameConstructor::ContentRangeInser
                                             TreeMatchContext* aProvidedTreeMatchContext)
 {
   MOZ_ASSERT(!aProvidedTreeMatchContext ||
              aInsertionKind == InsertionKind::Sync);
   MOZ_ASSERT(aInsertionKind == InsertionKind::Sync ||
              !RestyleManager()->IsInStyleRefresh());
 
   AUTO_LAYOUT_PHASE_ENTRY_POINT(mPresShell->GetPresContext(), FrameC);
-  NS_PRECONDITION(mUpdateCount != 0,
-                  "Should be in an update while creating frames");
+  MOZ_ASSERT(mUpdateCount != 0, "Should be in an update while creating frames");
 
   NS_PRECONDITION(aStartChild, "must always pass a child");
 
 #ifdef DEBUG
   if (gNoisyContentUpdates) {
     printf("nsCSSFrameConstructor::ContentRangeInserted container=%p "
            "start-child=%p end-child=%p lazy=%d\n",
            static_cast<void*>(aContainer),
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -35,16 +35,17 @@
 #include <stdio.h> // for FILE definition
 #include "nsChangeHint.h"
 #include "nsRefPtrHashtable.h"
 #include "nsClassHashtable.h"
 #include "nsPresArena.h"
 #include "nsIImageLoadingContent.h"
 #include "nsMargin.h"
 #include "nsFrameState.h"
+#include "nsStubDocumentObserver.h"
 #include "Units.h"
 
 class gfxContext;
 class nsDocShell;
 class nsIDocument;
 class nsIFrame;
 class nsPresContext;
 class nsWindowSizes;
@@ -159,17 +160,17 @@ enum nsRectVisibility {
  * presentation context, the style manager, the style set and the root
  * frame. <p>
  *
  * When this object is Release'd, it will release the document, the
  * presentation context, the style manager, the style set and the root
  * frame.
  */
 
-class nsIPresShell : public nsISupports
+class nsIPresShell : public nsStubDocumentObserver
 {
 public:
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_IPRESSHELL_IID)
 
 protected:
   typedef mozilla::layers::LayerManager LayerManager;
   typedef mozilla::gfx::SourceSurface SourceSurface;
 
@@ -331,22 +332,25 @@ public:
   already_AddRefed<nsFrameSelection> FrameSelection();
 
   /**
    * ConstFrameSelection returns an object which methods are safe to use for
    * example in nsIFrame code.
    */
   const nsFrameSelection* ConstFrameSelection() const { return mSelection; }
 
-  // Make shell be a document observer.  If called after Destroy() has
-  // been called on the shell, this will be ignored.
-  virtual void BeginObservingDocument() = 0;
+  // Start receiving notifications from our document. If called after Destroy,
+  // this will be ignored.
+  void BeginObservingDocument();
 
-  // Make shell stop being a document observer
-  virtual void EndObservingDocument() = 0;
+  // Stop receiving notifications from our document. If called after Destroy,
+  // this will be ignored.
+  void EndObservingDocument();
+
+  bool IsObservingDocument() const { return mIsObservingDocument; }
 
   /**
    * Return whether Initialize() was previously called.
    */
   bool DidInitialize() const { return mDidInitialize; }
 
   /**
    * Perform initialization. Constructs the frame for the root content
@@ -1733,16 +1737,21 @@ protected:
   // between paints and so are tied with retained layer pixels.
   // 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;
+
+  // 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;
 
   // For all documents we initially lock down painting.
   bool                      mPaintingSuppressed : 1;
 
   bool                      mIsActive : 1;
   bool                      mFrozen : 1;
   bool                      mIsFirstPaint : 1;
   bool                      mObservesMutationsForPrint : 1;