Bug 1334735 - Part 1: Move need style/flush flags from document to pres shell. r=bz

MozReview-Commit-ID: 2Amf9yGRiJA

dom/animation/EffectCompositor.cpp

@@ -22,6 +22,7 @@
 #include "nsCSSPropertyIDSet.h"
 #include "nsCSSProps.h"
 #include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 #include "nsLayoutUtils.h"
 #include "nsRuleNode.h" // For nsRuleNode::ComputePropertiesOverridingAnimation
 #include "nsRuleProcessorData.h" // For ElementRuleProcessorData etc.
@@ -263,7 +264,7 @@ EffectCompositor::RequestRestyle(dom::Element* aElement,
     if (!elementsToRestyle.Contains(key)) {
       elementsToRestyle.Put(key, false);
     }
-    mPresContext->Document()->SetNeedStyleFlush();
+    mPresContext->PresShell()->SetNeedStyleFlush();
   } else {
     // Get() returns 0 if the element is not found. It will also return
     // false if the element is found but does not have a pending restyle.

dom/base/nsDocument.cpp

@@ -254,6 +254,7 @@
 #include "mozilla/dom/SVGSVGElement.h"
 #include "mozilla/dom/DocGroup.h"
 #include "mozilla/dom/TabGroup.h"
+#include "nsIPresShellInlines.h"
 
 #include "mozilla/DocLoadingTimelineMarker.h"
 
@@ -1313,8 +1314,6 @@ nsIDocument::nsIDocument()
     mIsBeingUsedAsImage(false),
     mIsSyntheticDocument(false),
     mHasLinksToUpdate(false),
-    mNeedLayoutFlush(false),
-    mNeedStyleFlush(false),
     mMayHaveDOMMutationObservers(false),
     mMayHaveAnimationObservers(false),
     mHasMixedActiveContentLoaded(false),
@@ -1392,7 +1391,6 @@ nsDocument::nsDocument(const char* aContentType)
   , mDelayFrameLoaderInitialization(false)
   , mSynchronousDOMContentLoaded(false)
   , mInXBLUpdate(false)
-  , mInFlush(false)
   , mParserAborted(false)
   , mCurrentOrientationAngle(0)
   , mCurrentOrientationType(OrientationType::Portrait_primary)
@@ -7949,27 +7947,12 @@ nsDocument::FlushPendingNotifications(FlushType aType)
     mParentDocument->FlushPendingNotifications(parentType);
   }
 
-  // We can optimize away getting our presshell and calling
-  // FlushPendingNotifications on it if we don't need a flush of the sort we're
-  // looking at.  The one exception is if mInFlush is true, because in that
-  // case we might have set mNeedStyleFlush and mNeedLayoutFlush to false
-  // already but the presshell hasn't actually done the corresponding work yet.
-  // So if mInFlush and reentering this code, we need to flush the presshell.
-  if (mNeedStyleFlush ||
-      (mNeedLayoutFlush && aType >= FlushType::InterruptibleLayout) ||
-      aType >= FlushType::Display ||
-      mInFlush) {
-    nsCOMPtr<nsIPresShell> shell = GetShell();
-    if (shell) {
-      mNeedStyleFlush = false;
-      mNeedLayoutFlush = mNeedLayoutFlush && (aType < FlushType::InterruptibleLayout);
-      // mInFlush is a bitfield, so can't us AutoRestore here.  But we
-      // need to keep track of multi-level reentry correctly, so need
-      // to restore the old mInFlush value.
-      bool oldInFlush = mInFlush;
-      mInFlush = true;
-      shell->FlushPendingNotifications(aType);
-      mInFlush = oldInFlush;
+  // Call nsIPresShell::NeedFlush (inline, non-virtual) to check whether we
+  // really need to flush the shell (virtual, and needs a strong reference).
+  if (nsIPresShell* shell = GetShell()) {
+    if (shell->NeedFlush(aType)) {
+      nsCOMPtr<nsIPresShell> presShell = shell;
+      presShell->FlushPendingNotifications(aType);
     }
   }
 }
@@ -12902,7 +12885,9 @@ nsIDocument::RebuildUserFontSet()
   }
 
   mFontFaceSetDirty = true;
-  SetNeedStyleFlush();
+  if (nsIPresShell* shell = GetShell()) {
+    shell->SetNeedStyleFlush();
+  }
 
   // Somebody has already asked for the user font set, so we need to
   // post an event to rebuild it.  Setting the user font set to be dirty

dom/base/nsDocument.h

@@ -1445,10 +1445,6 @@ public:
 
   bool mInXBLUpdate:1;
 
-  // Whether we're currently under a FlushPendingNotifications call to
-  // our presshell.  This is used to handle flush reentry correctly.
-  bool mInFlush:1;
-
   // Parser aborted. True if the parser of this document was forcibly
   // terminated instead of letting it finish at its own pace.
   bool mParserAborted:1;

dom/base/nsIDocument.h

@@ -2477,20 +2477,6 @@ public:
 
   bool IsSyntheticDocument() const { return mIsSyntheticDocument; }
 
-  void SetNeedLayoutFlush() {
-    mNeedLayoutFlush = true;
-    if (mDisplayDocument) {
-      mDisplayDocument->SetNeedLayoutFlush();
-    }
-  }
-
-  void SetNeedStyleFlush() {
-    mNeedStyleFlush = true;
-    if (mDisplayDocument) {
-      mDisplayDocument->SetNeedStyleFlush();
-    }
-  }
-
   // Note: nsIDocument is a sub-class of nsINode, which has a
   // SizeOfExcludingThis function.  However, because nsIDocument objects can
   // only appear at the top of the DOM tree, we have a specialized measurement
@@ -3124,12 +3110,6 @@ protected:
   // True if this document has links whose state needs updating
   bool mHasLinksToUpdate : 1;
 
-  // True if a layout flush might not be a no-op
-  bool mNeedLayoutFlush : 1;
-
-  // True if a style flush might not be a no-op
-  bool mNeedStyleFlush : 1;
-
   // True if a DOMMutationObserver is perhaps attached to a node in the document.
   bool mMayHaveDOMMutationObservers : 1;
 

dom/smil/nsSMILAnimationController.cpp

@@ -16,6 +16,8 @@
 #include <algorithm>
 #include "mozilla/AutoRestore.h"
 #include "RestyleTracker.h"
+#include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -793,5 +795,7 @@ nsSMILAnimationController::GetRefreshDriver()
 void
 nsSMILAnimationController::FlagDocumentNeedsFlush()
 {
-  mDocument->SetNeedStyleFlush();
+  if (nsIPresShell* shell = mDocument->GetShell()) {
+    shell->SetNeedStyleFlush();
+  }
 }

dom/xbl/nsBindingManager.cpp

@@ -20,6 +20,7 @@
 #include "nsIDocument.h"
 #include "nsContentUtils.h"
 #include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 #include "nsIXMLContentSink.h"
 #include "nsContentCID.h"
 #include "mozilla/dom/XMLDocument.h"
@@ -338,7 +339,9 @@ nsBindingManager::AddToAttachedQueue(nsXBLBinding* aBinding)
   }
 
   // Make sure that flushes will flush out the new items as needed.
-  mDocument->SetNeedStyleFlush();
+  if (nsIPresShell* shell = mDocument->GetShell()) {
+    shell->SetNeedStyleFlush();
+  }
 
   return NS_OK;
 

layout/base/PresShell.cpp

@@ -796,6 +796,8 @@ nsIPresShell::nsIPresShell()
     , mReflowScheduled(false)
     , mSuppressInterruptibleReflows(false)
     , mScrollPositionClampingScrollPortSizeSet(false)
+    , mNeedLayoutFlush(true)
+    , mNeedStyleFlush(true)
     , mPresShellId(0)
     , mFontSizeInflationEmPerLine(0)
     , mFontSizeInflationMinTwips(0)
@@ -806,6 +808,7 @@ nsIPresShell::nsIPresShell()
     , mPaintingIsFrozen(false)
     , mFontSizeInflationEnabledIsDirty(false)
     , mIsNeverPainting(false)
+    , mInFlush(false)
   {}
 
 PresShell::PresShell()
@@ -942,6 +945,13 @@ PresShell::Init(nsIDocument* aDocument,
   mDocument = aDocument;
   mViewManager = aViewManager;
 
+  // mDocument is now set.  It might have a display document whose "need layout/
+  // style" flush flags are not set, but ours will be set.  To keep these
+  // consistent, call the flag setting functions to propagate those flags up
+  // to the display document.
+  SetNeedLayoutFlush();
+  SetNeedStyleFlush();
+
   // Create our frame constructor.
   mFrameConstructor = new nsCSSFrameConstructor(mDocument, this);
 
@@ -2027,7 +2037,7 @@ PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight, nscoord a
         NewRunnableMethod(this, &PresShell::FireResizeEvent);
       if (NS_SUCCEEDED(NS_DispatchToCurrentThread(resizeEvent))) {
         mResizeEvent = resizeEvent;
-        mDocument->SetNeedStyleFlush();
+        SetNeedStyleFlush();
       }
     }
   }
@@ -2773,7 +2783,7 @@ PresShell::FrameNeedsReflow(nsIFrame *aFrame, IntrinsicDirty aIntrinsicDirty,
         // we've hit a reflow root or the root frame
         if (!wasDirty) {
           mDirtyRoots.AppendElement(f);
-          mDocument->SetNeedLayoutFlush();
+          SetNeedLayoutFlush();
         }
 #ifdef DEBUG
         else {
@@ -3500,7 +3510,9 @@ PresShell::ScrollContentIntoView(nsIContent*              aContent,
   }
 
   // Flush layout and attempt to scroll in the process.
-  composedDoc->SetNeedLayoutFlush();
+  if (nsIPresShell* shell = composedDoc->GetShell()) {
+    shell->SetNeedLayoutFlush();
+  }
   composedDoc->FlushPendingNotifications(FlushType::InterruptibleLayout);
 
   // If mContentToScrollTo is non-null, that means we interrupted the reflow
@@ -3727,9 +3739,7 @@ PresShell::ScheduleViewManagerFlush(PaintType aType)
   if (presContext) {
     presContext->RefreshDriver()->ScheduleViewManagerFlush();
   }
-  if (mDocument) {
-    mDocument->SetNeedLayoutFlush();
-  }
+  SetNeedLayoutFlush();
 }
 
 bool
@@ -4067,7 +4077,7 @@ PresShell::FlushPendingNotifications(mozilla::ChangesToFlush aFlush)
   /**
    * 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 document.
+   * SetNeedStyleFlush calls on the shell.
    */
   FlushType flushType = aFlush.mFlushType;
 
@@ -4101,6 +4111,18 @@ PresShell::FlushPendingNotifications(mozilla::ChangesToFlush aFlush)
 
   NS_ASSERTION(flushType >= FlushType::Frames, "Why did we get called?");
 
+  // Record that we are in a flush, so that our optimization in
+  // nsDocument::FlushPendingNotifications doesn't skip any re-entrant
+  // calls to us.  Otherwise, we might miss some needed flushes, since
+  // we clear mNeedStyleFlush / mNeedLayoutFlush here at the top of
+  // the function but we might not have done the work yet.
+  AutoRestore<bool> guard(mInFlush);
+  mInFlush = true;
+
+  mNeedStyleFlush = false;
+  mNeedLayoutFlush =
+    mNeedLayoutFlush && (flushType < FlushType::InterruptibleLayout);
+
   bool isSafeToFlush = IsSafeToFlush();
 
   // If layout could possibly trigger scripts, then it's only safe to flush if
@@ -4224,7 +4246,7 @@ PresShell::FlushPendingNotifications(mozilla::ChangesToFlush aFlush)
   }
 
   if (!didStyleFlush && flushType >= FlushType::Style && !mIsDestroying) {
-    mDocument->SetNeedStyleFlush();
+    SetNeedStyleFlush();
   }
 
   if (!didLayoutFlush && !mIsDestroying &&
@@ -4232,9 +4254,9 @@ PresShell::FlushPendingNotifications(mozilla::ChangesToFlush aFlush)
        (mSuppressInterruptibleReflows ? FlushType::Layout
                                       : FlushType::InterruptibleLayout))) {
     // We suppressed this flush due to mSuppressInterruptibleReflows or
-    // !isSafeToFlush, but the document thinks it doesn't
-    // need to flush anymore.  Let it know what's really going on.
-    mDocument->SetNeedLayoutFlush();
+    // !isSafeToFlush, but now we think we don't need to flush any more.
+    // Record what's really going on.
+    SetNeedLayoutFlush();
   }
 }
 
@@ -9294,7 +9316,7 @@ PresShell::DoReflow(nsIFrame* target, bool aInterruptible)
 
     NS_ASSERTION(NS_SUBTREE_DIRTY(target), "Why is the target not dirty?");
     mDirtyRoots.AppendElement(target);
-    mDocument->SetNeedLayoutFlush();
+    SetNeedLayoutFlush();
 
     // Clear mFramesToDirty after we've done the NS_SUBTREE_DIRTY(target)
     // assertion so that if it fails it's easier to see what's going on.
@@ -9431,8 +9453,8 @@ PresShell::ProcessReflowCommands(bool aInterruptible)
       // reflow event just to have the flush revoke it.
       if (!mDirtyRoots.IsEmpty()) {
         MaybeScheduleReflow();
-        // And tell our document that we might need flushing
-        mDocument->SetNeedLayoutFlush();
+        // And record that we might need flushing
+        SetNeedLayoutFlush();
       }
     }
   }

layout/base/RestyleManagerBase.cpp

@@ -7,6 +7,7 @@
 #include "mozilla/RestyleManagerBase.h"
 #include "mozilla/StyleSetHandleInlines.h"
 #include "nsIFrame.h"
+#include "nsIPresShellInlines.h"
 
 namespace mozilla {
 
@@ -218,7 +219,7 @@ RestyleManagerBase::PostRestyleEventInternal(bool aForLazyConstruction)
   // Unconditionally flag our document as needing a flush.  The other
   // option here would be a dedicated boolean to track whether we need
   // to do so (set here and unset in ProcessPendingRestyles).
-  presShell->GetDocument()->SetNeedStyleFlush();
+  presShell->SetNeedStyleFlush();
 }
 
 /**

layout/base/moz.build

@@ -50,6 +50,7 @@ EXPORTS += [
     'nsILayoutHistoryState.h',
     'nsIPercentBSizeObserver.h',
     'nsIPresShell.h',
+    'nsIPresShellInlines.h',
     'nsIReflowCallback.h',
     'nsLayoutUtils.h',
     'nsPresArena.h',

layout/base/nsCSSFrameConstructor.cpp

@@ -122,6 +122,7 @@
 #include "nsRuleProcessorData.h"
 #include "nsTextNode.h"
 #include "ActiveLayerTracker.h"
+#include "nsIPresShellInlines.h"
 
 using namespace mozilla;
 using namespace mozilla::dom;
@@ -12936,3 +12937,19 @@ Iterator::DeleteItemsTo(const Iterator& aEnd)
     delete item;
   } while (*this != aEnd);
 }
+
+void
+nsCSSFrameConstructor::QuotesDirty()
+{
+  NS_PRECONDITION(mUpdateCount != 0, "Instant quote updates are bad news");
+  mQuotesDirty = true;
+  mPresShell->SetNeedLayoutFlush();
+}
+
+void
+nsCSSFrameConstructor::CountersDirty()
+{
+  NS_PRECONDITION(mUpdateCount != 0, "Instant counter updates are bad news");
+  mCountersDirty = true;
+  mPresShell->SetNeedLayoutFlush();
+}

layout/base/nsCSSFrameConstructor.h

@@ -2061,17 +2061,8 @@ private:
                       nsIContent*            aContent,
                       mozilla::StyleDisplay& aDisplay);
 
-  void QuotesDirty() {
-    NS_PRECONDITION(mUpdateCount != 0, "Instant quote updates are bad news");
-    mQuotesDirty = true;
-    mDocument->SetNeedLayoutFlush();
-  }
-
-  void CountersDirty() {
-    NS_PRECONDITION(mUpdateCount != 0, "Instant counter updates are bad news");
-    mCountersDirty = true;
-    mDocument->SetNeedLayoutFlush();
-  }
+  void QuotesDirty();
+  void CountersDirty();
 
   /**
    * Add the pair (aContent, aStyleContext) to the undisplayed items

layout/base/nsIPresShell.h

@@ -585,6 +585,32 @@ public:
   virtual void FlushPendingNotifications(mozilla::FlushType aType) = 0;
   virtual void FlushPendingNotifications(mozilla::ChangesToFlush aType) = 0;
 
+  /**
+   * Whether we might need a flush for the given flush type.  If this
+   * function returns false, we definitely don't need to flush.
+   *
+   * @param aFlushType The flush type to check.  This must be
+   *   >= FlushType::Style.
+   */
+  bool NeedFlush(mozilla::FlushType aType) const
+  {
+    // We check mInFlush to handle re-entrant calls to FlushPendingNotifications
+    // by reporting that we always need a flush in that case.  Otherwise,
+    // we could end up missing needed flushes, since we clear mNeedStyleFlush
+    // and mNeedLayoutFlush at the top of FlushPendingNotifications.
+    MOZ_ASSERT(aType >= mozilla::FlushType::Style);
+    return mNeedStyleFlush ||
+           (mNeedLayoutFlush &&
+            aType >= mozilla::FlushType::InterruptibleLayout) ||
+           aType >= mozilla::FlushType::Display ||
+           mInFlush;
+  }
+
+  inline void SetNeedStyleFlush();
+  inline void SetNeedLayoutFlush();
+
+  bool NeedStyleFlush() { return mNeedStyleFlush; }
+
   /**
    * Callbacks will be called even if reflow itself fails for
    * some reason.
@@ -1805,6 +1831,12 @@ protected:
   bool                      mSuppressInterruptibleReflows : 1;
   bool                      mScrollPositionClampingScrollPortSizeSet : 1;
 
+  // True if a layout flush might not be a no-op
+  bool mNeedLayoutFlush : 1;
+
+  // True if a style flush might not be a no-op
+  bool mNeedStyleFlush : 1;
+
   uint32_t                  mPresShellId;
 
   // List of subtrees rooted at style scope roots that need to be restyled.
@@ -1835,6 +1867,10 @@ protected:
   // to true, so we can avoid any paint calls for widget related to this
   // presshell.
   bool mIsNeverPainting;
+
+  // Whether we're currently under a FlushPendingNotifications.
+  // This is used to handle flush reentry correctly.
+  bool mInFlush;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(nsIPresShell, NS_IPRESSHELL_IID)

layout/base/nsIPresShellInlines.h

@@ -0,0 +1,28 @@
+#ifndef nsIPresShellInlines_h
+#define nsIPresShellInlines_h
+
+#include "nsIDocument.h"
+
+void
+nsIPresShell::SetNeedLayoutFlush()
+{
+  mNeedLayoutFlush = true;
+  if (nsIDocument* doc = mDocument->GetDisplayDocument()) {
+    if (nsIPresShell* shell = doc->GetShell()) {
+      shell->mNeedLayoutFlush = true;
+    }
+  }
+}
+
+void
+nsIPresShell::SetNeedStyleFlush()
+{
+  mNeedStyleFlush = true;
+  if (nsIDocument* doc = mDocument->GetDisplayDocument()) {
+    if (nsIPresShell* shell = doc->GetShell()) {
+      shell->mNeedStyleFlush = true;
+    }
+  }
+}
+
+#endif // nsIPresShellInlines_h

layout/base/nsPresContext.cpp

@@ -15,6 +15,7 @@
 #include "nsCOMPtr.h"
 #include "nsPresContext.h"
 #include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 #include "nsDocShell.h"
 #include "nsIContentViewer.h"
 #include "nsPIDOMWindow.h"
@@ -2069,12 +2070,12 @@ nsPresContext::PostMediaFeatureValuesChangedEvent()
   // FIXME: We should probably replace this event with use of
   // nsRefreshDriver::AddStyleFlushObserver (except the pres shell would
   // need to track whether it's been added).
-  if (!mPendingMediaFeatureValuesChanged) {
+  if (!mPendingMediaFeatureValuesChanged && mShell) {
     nsCOMPtr<nsIRunnable> ev =
       NewRunnableMethod(this, &nsPresContext::HandleMediaFeatureValuesChangedEvent);
     if (NS_SUCCEEDED(NS_DispatchToCurrentThread(ev))) {
       mPendingMediaFeatureValuesChanged = true;
-      mDocument->SetNeedStyleFlush();
+      mShell->SetNeedStyleFlush();
     }
   }
 }
@@ -2258,7 +2259,9 @@ nsPresContext::RebuildCounterStyles()
   }
 
   mCounterStylesDirty = true;
-  mDocument->SetNeedStyleFlush();
+  if (mShell) {
+    mShell->SetNeedStyleFlush();
+  }
   if (!mPostedFlushCounterStyles) {
     nsCOMPtr<nsIRunnable> ev =
       NewRunnableMethod(this, &nsPresContext::HandleRebuildCounterStyles);

layout/style/nsAnimationManager.cpp

@@ -24,6 +24,8 @@
 #include "nsIFrame.h"
 #include "nsIDocument.h"
 #include "nsDOMMutationObserver.h"
+#include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 #include <algorithm> // std::stable_sort
 #include <math.h>
 
@@ -455,7 +457,7 @@ nsAnimationManager::UpdateAnimations(nsStyleContext* aStyleContext,
   // or the next time somebody calls
   // nsPresShell::FlushPendingNotifications.
   if (mEventDispatcher.HasQueuedEvents()) {
-    mPresContext->Document()->SetNeedStyleFlush();
+    mPresContext->PresShell()->SetNeedStyleFlush();
   }
 }
 

view/nsViewManager.cpp

@@ -17,6 +17,7 @@
 #include "nsIPluginWidget.h"
 #include "nsXULPopupManager.h"
 #include "nsIPresShell.h"
+#include "nsIPresShellInlines.h"
 #include "nsPresContext.h"
 #include "mozilla/StartupTimeline.h"
 #include "GeckoProfiler.h"
@@ -229,10 +230,9 @@ nsViewManager::SetWindowDimensions(nscoord aWidth, nscoord aHeight, bool aDelayR
       DoSetWindowDimensions(aWidth, aHeight);
     } else {
       mDelayedResize.SizeTo(aWidth, aHeight);
-      if (mPresShell && mPresShell->GetDocument()) {
-        nsIDocument* doc = mPresShell->GetDocument();
-        doc->SetNeedStyleFlush();
-        doc->SetNeedLayoutFlush();
+      if (mPresShell) {
+        mPresShell->SetNeedStyleFlush();
+        mPresShell->SetNeedLayoutFlush();
       }
     }
   }