Bug 1442020 - Ensure neither style nor layout flushes are required when running promiseDocumentFlushed callbacks. r?bz draft
authorMike Conley <mconley@mozilla.com>
Wed, 28 Feb 2018 16:15:51 -0500
changeset 765356 aa6a3a48a2d12daac49cba2d3a46c4bb4dd48336
parent 765246 f1965cf7425fe422c9e9c78018f11b97e0a0f229
child 765357 7546f0738f5df0cd6429de85aae9b6c9b60a4e22
push id102042
push usermconley@mozilla.com
push dateFri, 09 Mar 2018 16:53:19 +0000
reviewersbz
bugs1442020
milestone60.0a1
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
@@ -7420,17 +7420,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);
@@ -7515,20 +7515,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.