Bug 1442020 - Ensure neither style nor layout flushes are required when running promiseDocumentFlushed callbacks. r=bz
authorMike Conley <mconley@mozilla.com>
Wed, 28 Feb 2018 16:15:51 -0500
changeset 462048 93a515ea172174484a42fc0855e0461e433bee46
parent 462047 eb1dfa51050f47c2b6a66e521e75c9ed6e95800f
child 462049 6409e2ef806792557a01b47a883f11d218ec8b3a
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1442020
milestone60.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 1442020 - Ensure neither style nor layout flushes are required when running promiseDocumentFlushed callbacks. r=bz MozReview-Commit-ID: INGpltVvNmZ
dom/base/nsGlobalWindowInner.cpp
dom/base/test/browser_promiseDocumentFlushed.js
layout/base/nsIPresShell.h
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -7423,17 +7423,17 @@ nsGlobalWindowInner::PromiseDocumentFlus
   RefPtr<Promise> resultPromise = Promise::Create(global, aError);
   if (aError.Failed()) {
     return nullptr;
   }
 
   UniquePtr<PromiseDocumentFlushedResolver> flushResolver(
     new PromiseDocumentFlushedResolver(resultPromise, aCallback));
 
-  if (!shell->NeedFlush(FlushType::Style)) {
+  if (!shell->NeedStyleFlush() && !shell->NeedLayoutFlush()) {
     flushResolver->Call();
     return resultPromise.forget();
   }
 
   if (!mObservingDidRefresh) {
     bool success = shell->AddPostRefreshObserver(this);
     if (!success) {
       aError.Throw(NS_ERROR_FAILURE);
@@ -7518,20 +7518,21 @@ nsGlobalWindowInner::DidRefresh()
     mObservingDidRefresh = false;
   });
 
   MOZ_ASSERT(mDoc);
 
   nsIPresShell* shell = mDoc->GetShell();
   MOZ_ASSERT(shell);
 
-  if (shell->NeedStyleFlush() || shell->HasPendingReflow()) {
+  if (shell->NeedStyleFlush() || shell->NeedLayoutFlush()) {
     // By the time our observer fired, something has already invalidated
-    // style and maybe layout. We'll wait until the next refresh driver
-    // tick instead.
+    // style or layout - or perhaps we're still in the middle of a flush that
+    // was interrupted. In either case, we'll wait until the next refresh driver
+    // tick instead and try again.
     rejectionGuard.release();
     return;
   }
 
   bool success = shell->RemovePostRefreshObserver(this);
   if (!success) {
     return;
   }
--- a/dom/base/test/browser_promiseDocumentFlushed.js
+++ b/dom/base/test/browser_promiseDocumentFlushed.js
@@ -26,26 +26,30 @@ const gWindowUtils = window.QueryInterfa
                            .getInterface(Ci.nsIDOMWindowUtils);
 
 /**
  * Asserts that no style or layout flushes are required by the
  * current window.
  */
 function assertNoFlushesRequired() {
   Assert.ok(!gWindowUtils.needsFlush(Ci.nsIDOMWindowUtils.FLUSH_STYLE),
-            "No flushes are required.");
+            "No style flushes are required.");
+  Assert.ok(!gWindowUtils.needsFlush(Ci.nsIDOMWindowUtils.FLUSH_LAYOUT),
+            "No layout flushes are required.");
 }
 
 /**
  * Asserts that the DOM has been dirtied, and so style and layout flushes
  * are required.
  */
 function assertFlushesRequired() {
+  Assert.ok(gWindowUtils.needsFlush(Ci.nsIDOMWindowUtils.FLUSH_STYLE),
+            "Style flush required.");
   Assert.ok(gWindowUtils.needsFlush(Ci.nsIDOMWindowUtils.FLUSH_LAYOUT),
-            "Style and layout flushes are required.");
+            "Layout flush required.");
 }
 
 /**
  * Removes style changes from dirtyTheDOM() from the browser window,
  * and resolves once the refresh driver ticks.
  */
 async function cleanTheDOM() {
   gNavToolbox.style.padding = "";
--- a/layout/base/nsIPresShell.h
+++ b/layout/base/nsIPresShell.h
@@ -615,16 +615,22 @@ public:
 
   void ObserveStyleFlushes()
   {
     if (!ObservingStyleFlushes())
       DoObserveStyleFlushes();
   }
 
   bool NeedStyleFlush() const { return mNeedStyleFlush; }
+  /**
+   * Returns true if we might need to flush layout, even if we haven't scheduled
+   * one yet (as opposed to HasPendingReflow, which returns true if a flush is
+   * scheduled or will soon be scheduled).
+   */
+  bool NeedLayoutFlush() const { return mNeedLayoutFlush; }
 
   /**
    * Callbacks will be called even if reflow itself fails for
    * some reason.
    */
   virtual nsresult PostReflowCallback(nsIReflowCallback* aCallback) = 0;
   virtual void CancelReflowCallback(nsIReflowCallback* aCallback) = 0;
 
@@ -1633,16 +1639,20 @@ public:
   bool IsNeverPainting() {
     return mIsNeverPainting;
   }
 
   void SetNeverPainting(bool aNeverPainting) {
     mIsNeverPainting = aNeverPainting;
   }
 
+  /**
+   * True if a reflow event has been scheduled, or is going to be scheduled
+   * to run in the future.
+   */
   bool HasPendingReflow() const
     { return mObservingLayoutFlushes || mReflowContinueTimer; }
 
   void SyncWindowProperties(nsView* aView);
 
   virtual nsIDocument* GetPrimaryContentDocument() = 0;
 
   // aSheetType is one of the nsIStyleSheetService *_SHEET constants.