Bug 709256 part 3. Skip calling PresShell::FlushPendingNotifications altogether if there might not be anything to flush. r=roc

2011-12-14 23:42:15 -05:00 · 2011-12-14 23:42:15 -05:00 · ea35d340a7
--- a/content/base/public/nsIDocument.h
+++ b/content/base/public/nsIDocument.h
@ -1601,6 +1601,20 @@ public:

  virtual void PostVisibilityUpdateEvent() = 0;

+  void SetNeedLayoutFlush() {
+    mNeedLayoutFlush = true;
+    if (mDisplayDocument) {
+      mDisplayDocument->SetNeedLayoutFlush();
+    }
+  }
+
+  void SetNeedStyleFlush() {
+    mNeedStyleFlush = true;
+    if (mDisplayDocument) {
+      mDisplayDocument->SetNeedStyleFlush();
+    }
+  }
+
 private:
  PRUint64 mWarnedAbout;

@ -1765,6 +1779,12 @@ protected:
  // True if this document has links whose state needs updating
  bool mHasLinksToUpdate;

+  // True if a layout flush might not be a no-op
+  bool mNeedLayoutFlush;
+
+  // True if a style flush might not be a no-op
+  bool mNeedStyleFlush;
+
  // The document's script global object, the object from which the
  // document can get its script context and scope. This is the
  // *inner* window object.
--- a/content/base/src/nsDocument.cpp
+++ b/content/base/src/nsDocument.cpp
@ -6267,9 +6267,28 @@ nsDocument::FlushPendingNotifications(mozFlushType aType)
    mParentDocument->FlushPendingNotifications(parentType);
  }

-  nsCOMPtr<nsIPresShell> shell = GetShell();
-  if (shell) {
-    shell->FlushPendingNotifications(aType);
+  // 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 >= Flush_InterruptibleLayout) ||
+      aType >= Flush_Display ||
+      mInFlush) {
+    nsCOMPtr<nsIPresShell> shell = GetShell();
+    if (shell) {
+      mNeedStyleFlush = false;
+      mNeedLayoutFlush = mNeedLayoutFlush && (aType < Flush_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;
+    }
  }
 }

@ -6759,7 +6778,7 @@ nsDocument::CreateElem(const nsAString& aName, nsIAtom *aPrefix, PRInt32 aNamesp
 bool
 nsDocument::IsSafeToFlush() const
 {
-  nsCOMPtr<nsIPresShell> shell = GetShell();
+  nsIPresShell* shell = GetShell();
  if (!shell)
    return true;

--- a/content/base/src/nsDocument.h
+++ b/content/base/src/nsDocument.h
@ -1187,6 +1187,10 @@ protected:
  // Whether we are currently in full-screen mode, as per the DOM API.
  bool mIsFullScreen:1;

+  // Whether we're currently under a FlushPendingNotifications call to
+  // our presshell.  This is used to handle flush reentry correctly.
+  bool mInFlush:1;
+
  PRUint8 mXMLDeclarationBits;

  nsInterfaceHashtable<nsVoidPtrHashKey, nsPIBoxObject> *mBoxObjectTable;
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@ -537,6 +537,7 @@ _TEST_FILES2 = \
 		file_bug707142_baseline.json \
 		file_bug707142_bom.json \
 		file_bug707142_utf-16.json \
+		test_reentrant_flush.html \
 		$(NULL)

 _CHROME_FILES =	\
--- a/content/base/test/test_reentrant_flush.html
+++ b/content/base/test/test_reentrant_flush.html
@ -0,0 +1,58 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=709256
+-->
+<head>
+  <title>Test for Bug 709256</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=709256">Mozilla Bug 709256</a>
+<p id="display">
+<iframe id="test"
+        style="width: 100px" src="data:text/html,<body style='width: 100%'>">
+</iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 709256 **/
+addLoadEvent(function() {
+  var ifr = $("test");
+  var bod = ifr.contentDocument.body;
+
+  is(bod.getBoundingClientRect().width, 100,
+    "Width of body should be 100px to start with");
+
+  var resizeHandlerRan = false;
+
+  function handleResize() {
+    resizeHandlerRan = true;
+    is(bod.getBoundingClientRect().width, 50,
+       "Width of body should now be 50px");
+  }
+
+  var win = ifr.contentWindow;
+
+  win.addEventListener("resize", handleResize, false);
+  SpecialPowers.setFullZoom(win, 2);
+
+  is(resizeHandlerRan, false,
+     "Resize handler should not have run yet for this test to be valid");
+
+  // Now flush out layout on the subdocument, to trigger the resize handler
+  is(bod.getBoundingClientRect().width, 50, "Width of body should still be 50px");
+
+  is(resizeHandlerRan, true, "Resize handler should have run");
+
+  win.removeEventListener("resize", handleResize, false);
+});
+</script>
+</pre>
+</body>
+</html>
--- a/content/smil/nsSMILAnimationController.cpp
+++ b/content/smil/nsSMILAnimationController.cpp
@ -852,3 +852,9 @@ nsSMILAnimationController::GetRefreshDriver()
  nsPresContext* context = shell->GetPresContext();
  return context ? context->RefreshDriver() : nsnull;
 }
+
+void
+nsSMILAnimationController::FlagDocumentNeedsFlush()
+{
+  mDocument->SetNeedStyleFlush();
+}
--- a/content/smil/nsSMILAnimationController.h
+++ b/content/smil/nsSMILAnimationController.h
@ -96,9 +96,13 @@ public:
  // (A resample performs the same operations as a sample but doesn't advance
  // the current time and doesn't check if the container is paused)
  void Resample() { DoSample(false); }
+
  void SetResampleNeeded()
  {
    if (!mRunningSample) {
+      if (!mResampleNeeded) {
+        FlagDocumentNeedsFlush();
+      }
      mResampleNeeded = true;
    }
  }
@ -198,6 +202,8 @@ protected:
  virtual nsresult AddChild(nsSMILTimeContainer& aChild);
  virtual void     RemoveChild(nsSMILTimeContainer& aChild);

+  void FlagDocumentNeedsFlush();
+
  // Members
  nsAutoRefCnt mRefCnt;
  NS_DECL_OWNINGTHREAD
--- a/content/xbl/src/nsBindingManager.cpp
+++ b/content/xbl/src/nsBindingManager.cpp
@ -989,6 +989,9 @@ nsBindingManager::AddToAttachedQueue(nsXBLBinding* aBinding)
    PostProcessAttachedQueueEvent();
  }

+  // Make sure that flushes will flush out the new items as needed.
+  mDocument->SetNeedStyleFlush();
+
  return NS_OK;

 }
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@ -11703,6 +11703,11 @@ nsCSSFrameConstructor::PostRestyleEventInternal(bool aForLazyConstruction)
    mObservingRefreshDriver = mPresShell->GetPresContext()->RefreshDriver()->
      AddStyleFlushObserver(mPresShell);
  }
+
+  // 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).
+  mPresShell->GetDocument()->SetNeedStyleFlush();
 }

 void
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@ -1774,11 +1774,13 @@ private:
  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();
  }

 public:
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@ -1677,6 +1677,7 @@ nsPresContext::PostMediaFeatureValuesChangedEvent()
      NS_NewRunnableMethod(this, &nsPresContext::HandleMediaFeatureValuesChangedEvent);
    if (NS_SUCCEEDED(NS_DispatchToCurrentThread(ev))) {
      mPendingMediaFeatureValuesChanged = true;
+      mDocument->SetNeedStyleFlush();
    }
  }
 }
@ -1886,6 +1887,7 @@ nsPresContext::RebuildUserFontSet()
  }

  mUserFontSetDirty = true;
+  mDocument->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
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@ -2183,6 +2183,7 @@ PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight)
        NS_NewRunnableMethod(this, &PresShell::FireResizeEvent);
      if (NS_SUCCEEDED(NS_DispatchToCurrentThread(resizeEvent))) {
        mResizeEvent = resizeEvent;
+        mDocument->SetNeedStyleFlush();
      }
    }
  }
@ -2864,6 +2865,7 @@ PresShell::FrameNeedsReflow(nsIFrame *aFrame, IntrinsicDirty aIntrinsicDirty,
        // we've hit a reflow root or the root frame
        if (!wasDirty) {
          mDirtyRoots.AppendElement(f);
+          mDocument->SetNeedLayoutFlush();
        }
 #ifdef DEBUG
        else {
@ -3444,6 +3446,7 @@ PresShell::ScrollContentIntoView(nsIContent* aContent,
  mContentToScrollToFlags = aFlags;

  // Flush layout and attempt to scroll in the process.
+  currentDoc->SetNeedLayoutFlush();
  currentDoc->FlushPendingNotifications(Flush_InterruptibleLayout);

  // If mContentToScrollTo is non-null, that means we interrupted the reflow
@ -3966,6 +3969,12 @@ PresShell::IsSafeToFlush() const
 void
 PresShell::FlushPendingNotifications(mozFlushType aType)
 {
+  /**
+   * 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.
+   */
+
 #ifdef NS_FUNCTION_TIMER
  NS_TIME_FUNCTION_DECLARE_DOCURL;
  static const char *flushTypeNames[] = {
@ -4100,6 +4109,11 @@ PresShell::FlushPendingNotifications(mozFlushType aType)
                                mContentToScrollToFlags);
        mContentToScrollTo = nsnull;
      }
+    } else if (!mIsDestroying && mSuppressInterruptibleReflows &&
+               aType == Flush_InterruptibleLayout) {
+      // We suppressed this flush, but the document thinks it doesn't
+      // need to flush anymore.  Let it know what's really going on.
+      mDocument->SetNeedLayoutFlush();
    }

    if (aType >= Flush_Layout) {
@ -7343,6 +7357,7 @@ PresShell::DoReflow(nsIFrame* target, bool aInterruptible)
    mFramesToDirty.EnumerateEntries(&MarkFramesDirtyToRoot, target);
    NS_ASSERTION(NS_SUBTREE_DIRTY(target), "Why is the target not dirty?");
    mDirtyRoots.AppendElement(target);
+    mDocument->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.
@ -7465,8 +7480,11 @@ PresShell::ProcessReflowCommands(bool aInterruptible)
      // after DidDoReflow(), since that method can change whether there are
      // dirty roots around by flushing, and there's no point in posting a
      // reflow event just to have the flush revoke it.
-      if (!mDirtyRoots.IsEmpty())
+      if (!mDirtyRoots.IsEmpty()) {
        MaybeScheduleReflow();
+        // And tell our document that we might need flushing
+        mDocument->SetNeedLayoutFlush();
+      }
    }
  }

--- a/layout/style/nsAnimationManager.cpp
+++ b/layout/style/nsAnimationManager.cpp
@ -546,6 +546,9 @@ nsAnimationManager::CheckAnimationRule(nsStyleContext* aStyleContext,
    // dispatch them the next time we get a refresh driver notification
    // or the next time somebody calls
    // nsPresShell::FlushPendingNotifications.
+    if (!mPendingEvents.IsEmpty()) {
+      mPresContext->Document()->SetNeedStyleFlush();
+    }
  }

  return GetAnimationRule(aElement, aStyleContext->GetPseudoType());
--- a/view/src/nsViewManager.cpp
+++ b/view/src/nsViewManager.cpp
@ -281,6 +281,9 @@ NS_IMETHODIMP nsViewManager::SetWindowDimensions(nscoord aWidth, nscoord aHeight
      DoSetWindowDimensions(aWidth, aHeight);
    } else {
      mDelayedResize.SizeTo(aWidth, aHeight);
+      if (mPresShell && mPresShell->GetDocument()) {
+        mPresShell->GetDocument()->SetNeedStyleFlush();
+      }
    }
  }