Bug 1434376 - Switch over all uses of BrowserUtils.promiseLayoutFlushed to window.promiseDocumentFlushed. r?paolo draft
authorMike Conley <mconley@mozilla.com>
Sun, 11 Feb 2018 20:15:11 -0500
changeset 759488 0308b3734ccf5ec44f1afb62880a0a6661040e90
parent 759487 483a0cd846ded2ccf85aa3a433319309484185c2
child 759489 1fd1431e06193098a819e1135590e5c1d459529a
child 759490 b03967b2ccc2704ee259b84a98c939db47d0920d
child 759516 1a6cb879ec532c1e93d384df17f15f2c21e96bde
push id100366
push usermconley@mozilla.com
push dateSun, 25 Feb 2018 01:20:16 +0000
reviewerspaolo
bugs1434376
milestone60.0a1
Bug 1434376 - Switch over all uses of BrowserUtils.promiseLayoutFlushed to window.promiseDocumentFlushed. r?paolo window.promiseDocumentFlushed will call a callback as soon as a style or layout flush is not required for the document (which might be immediately). This is a new ChromeOnly API introduced in an earlier patch in this series. This patch also removes the now-unneeded BrowserUtils.promiseLayoutFlushed and BrowserUtils.promiseReflowed methods and infrastructure. MozReview-Commit-ID: Jv7KoxBXhHG
browser/base/content/browser.js
browser/base/content/tabbrowser.xml
browser/base/content/test/performance/head.js
browser/components/customizableui/CustomizeMode.jsm
browser/components/customizableui/PanelMultiView.jsm
browser/components/places/content/browserPlacesViews.js
toolkit/modules/BrowserUtils.jsm
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -8005,17 +8005,17 @@ var gIdentityHandler = {
       classes += " blocked-permission-icon";
 
     if (aPermission.sharingState == Ci.nsIMediaManagerService.STATE_CAPTURE_ENABLED ||
        (aPermission.id == "screen" && aPermission.sharingState &&
         !aPermission.sharingState.includes("Paused"))) {
       classes += " in-use";
 
       // Synchronize control center and identity block blinking animations.
-      BrowserUtils.promiseLayoutFlushed(document, "style", () => {
+      window.promiseDocumentFlushed(() => {}).then(() => {
         let sharingIconBlink = document.getElementById("sharing-icon").getAnimations()[0];
         if (sharingIconBlink) {
           let startTime = sharingIconBlink.startTime;
           window.requestAnimationFrame(() => {
             // TODO(Bug 1440607): This could cause a style flush, but putting
             // the getAnimations() call outside of rAF causes a leak.
             let imgBlink = img.getAnimations()[0];
             if (imgBlink) {
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -278,17 +278,17 @@
           ]]>
         </body>
       </method>
 
       <method name="syncThrobberAnimations">
         <parameter name="aTab"/>
         <body>
           <![CDATA[
-            BrowserUtils.promiseLayoutFlushed(aTab.ownerDocument, "style", () => {
+            aTab.ownerGlobal.promiseDocumentFlushed(() => {
               if (!aTab.parentNode) {
                 return;
               }
 
               const animations =
                 Array.from(aTab.parentNode.getElementsByTagName("tab"))
                 .map(tab => {
                   const throbber =
--- a/browser/base/content/test/performance/head.js
+++ b/browser/base/content/test/performance/head.js
@@ -77,19 +77,18 @@ async function withReflowObserver(testFn
       // Gather information about the current code path.
       reflows.push(new Error().stack);
 
       // Just in case, dirty the frame now that we've reflowed.
       dirtyFrameFn();
     },
 
     reflowInterruptible(start, end) {
-      // We're not interested in interruptible reflows, but might as well take the
-      // opportuntiy to dirty the root frame.
-      dirtyFrameFn();
+      // Interruptible reflows are the reflows caused by the refresh
+      // driver ticking. These are fine.
     },
 
     QueryInterface: XPCOMUtils.generateQI([Ci.nsIReflowObserver,
                                            Ci.nsISupportsWeakReference])
   };
 
   let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
                     .getInterface(Ci.nsIWebNavigation)
--- a/browser/components/customizableui/CustomizeMode.jsm
+++ b/browser/components/customizableui/CustomizeMode.jsm
@@ -564,21 +564,17 @@ CustomizeMode.prototype = {
 
       function resolveAnimationPromise() {
         animationNode.removeEventListener("animationend", cleanupWidgetAnimationEnd);
         animationNode.removeEventListener("customizationending", cleanupCustomizationExit);
         resolve();
       }
 
       // Wait until the next frame before setting the class to ensure
-      // we do start the animation. We cannot use promiseLayoutFlushed
-      // here because callback is only invoked when any actual reflow
-      // happens, while that may not happen soonish enough. If we have
-      // an observer for style flush, we may be able to replace the
-      // nested rAFs with that.
+      // we do start the animation.
       this.window.requestAnimationFrame(() => {
         this.window.requestAnimationFrame(() => {
           animationNode.classList.add("animate-out");
           animationNode.ownerGlobal.gNavToolbox.addEventListener("customizationending", cleanupCustomizationExit);
           animationNode.addEventListener("animationend", cleanupWidgetAnimationEnd);
         });
       });
     });
@@ -2416,17 +2412,18 @@ CustomizeMode.prototype = {
     let panelOnTheLeft = false;
     let toolbarContainer = button.closest("toolbar");
     if (toolbarContainer && toolbarContainer.id == "nav-bar") {
       let navbarWidgets = CustomizableUI.getWidgetIdsInArea("nav-bar");
       if (navbarWidgets.indexOf("urlbar-container") <= navbarWidgets.indexOf("downloads-button")) {
         panelOnTheLeft = true;
       }
     } else {
-      await BrowserUtils.promiseLayoutFlushed(doc, "display", () => {});
+      await this.window.promiseDocumentFlushed(() => {});
+
       if (!this._customizing || !this._wantToBeInCustomizeMode) {
         return;
       }
       let buttonBounds = this._dwu.getBoundsWithoutFlushing(button);
       let windowBounds = this._dwu.getBoundsWithoutFlushing(doc.documentElement);
       panelOnTheLeft = (buttonBounds.left + buttonBounds.width / 2) > windowBounds.width / 2;
     }
     let position;
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -490,18 +490,18 @@ var PanelMultiView = class extends this.
       }
       try {
         // Most of the panel elements in the browser window have their display
         // turned off for performance reasons, typically by setting the "hidden"
         // attribute. If the caller has just turned on the display, the XBL
         // binding for the <panelmultiview> element may still be disconnected.
         // In this case, give the layout code a chance to run.
         if (!this.connected) {
-          await BrowserUtils.promiseLayoutFlushed(this.document, "layout",
-                                                  () => {});
+          await this.window.promiseDocumentFlushed(() => {});
+
           // The XBL binding must be connected at this point. If this is not the
           // case, the calling code should be updated to unhide the panel.
           if (!this.connected) {
             throw new Error("The binding for the panelmultiview element isn't" +
                             " connected. The containing panel may still have" +
                             " its display turned off by the hidden attribute.");
           }
         }
@@ -783,17 +783,17 @@ var PanelMultiView = class extends this.
     await this._cleanupTransitionPhase();
 
     // There's absolutely no need to show off our epic animation skillz when
     // the panel's not even open.
     if (this._panel.state != "open") {
       return;
     }
 
-    const {window, document} = this;
+    const { window } = this;
 
     let nextPanelView = PanelView.forNode(viewNode);
     let prevPanelView = PanelView.forNode(previousViewNode);
 
     if (this._autoResizeWorkaroundTimer)
       window.clearTimeout(this._autoResizeWorkaroundTimer);
 
     let details = this._transitionDetails = {
@@ -842,17 +842,17 @@ var PanelMultiView = class extends this.
       this._offscreenViewStack.style.minHeight = olderView.knownHeight + "px";
       this._offscreenViewStack.appendChild(viewNode);
       viewNode.setAttribute("in-transition", true);
 
       // Now that the subview is visible, we can check the height of the
       // description elements it contains.
       nextPanelView.descriptionHeightWorkaround();
 
-      viewRect = await BrowserUtils.promiseLayoutFlushed(this.document, "layout", () => {
+      viewRect = await window.promiseDocumentFlushed(() => {
         return this._dwu.getBoundsWithoutFlushing(viewNode);
       });
 
       try {
         this._viewStack.insertBefore(viewNode, oldSibling);
       } catch (ex) {
         this._viewStack.appendChild(viewNode);
       }
@@ -889,17 +889,17 @@ var PanelMultiView = class extends this.
     this._viewContainer.style.height = viewRect.height + "px";
     this._viewContainer.style.width = viewRect.width + "px";
     this._panel.removeAttribute("width");
     this._panel.removeAttribute("height");
     // We're setting the width property to prevent flickering during the
     // sliding animation with smaller views.
     viewNode.style.width = viewRect.width + "px";
 
-    await BrowserUtils.promiseLayoutFlushed(document, "layout", () => {});
+    await window.promiseDocumentFlushed(() => {});
 
     // Kick off the transition!
     details.phase = TRANSITION_PHASES.TRANSITION;
     this._viewStack.style.transform = "translateX(" + (moveToLeft ? "" : "-") + deltaX + "px)";
 
     await new Promise(resolve => {
       details.resolve = resolve;
       this._viewContainer.addEventListener("transitionend", details.listener = ev => {
@@ -995,17 +995,17 @@ var PanelMultiView = class extends this.
         this._viewContainer.removeEventListener("transitioncancel", cancelListener);
       if (resolve)
         resolve();
     }
     if (phase >= TRANSITION_PHASES.END) {
       // We force 'display: none' on the previous view node to make sure that it
       // doesn't cause an annoying flicker whilst resetting the styles above.
       previousViewNode.style.display = "none";
-      await BrowserUtils.promiseLayoutFlushed(this.document, "layout", () => {});
+      await this.window.promiseDocumentFlushed(() => {});
       previousViewNode.style.removeProperty("display");
     }
   }
 
   _calculateMaxHeight() {
     // While opening the panel, we have to limit the maximum height of any
     // view based on the space that will be available. We cannot just use
     // window.screen.availTop and availHeight because these may return an
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -1603,20 +1603,18 @@ PlacesToolbar.prototype = {
     let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
     timer.initWithCallback(this, aTime, timer.TYPE_ONE_SHOT);
     return timer;
   },
 
   notify: function PT_notify(aTimer) {
     if (aTimer == this._updateNodesVisibilityTimer) {
       this._updateNodesVisibilityTimer = null;
-      // TODO: This should use promiseLayoutFlushed("layout"), so that
-      // _updateNodesVisibilityTimerCallback can use getBoundsWithoutFlush. But
-      // for yet unknown reasons doing that causes intermittent failures,
-      // apparently due the flush not happening in a meaningful time.
+      // Bug 1440070: This should use promiseDocumentFlushed, so that
+      // _updateNodesVisibilityTimerCallback can use getBoundsWithoutFlush.
       window.requestAnimationFrame(this._updateNodesVisibilityTimerCallback.bind(this));
     } else if (aTimer == this._ibTimer) {
       // * Timer to turn off indicator bar.
       this._dropIndicator.collapsed = true;
       this._ibTimer = null;
     } else if (aTimer == this._overFolder.openTimer) {
       // * Timer to open a menubutton that's being dragged over.
       // Set the autoopen attribute on the folder's menupopup so that
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -9,58 +9,16 @@ var EXPORTED_SYMBOLS = [ "BrowserUtils" 
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.defineModuleGetter(this, "PlacesUtils",
   "resource://gre/modules/PlacesUtils.jsm");
 
 Cu.importGlobalProperties(["URL"]);
 
-let reflowObservers = new WeakMap();
-
-function ReflowObserver(doc) {
-  this._doc = doc;
-
-  doc.docShell.addWeakReflowObserver(this);
-  reflowObservers.set(this._doc, this);
-
-  this.callbacks = [];
-}
-
-ReflowObserver.prototype = {
-  QueryInterface: XPCOMUtils.generateQI(["nsIReflowObserver", "nsISupportsWeakReference"]),
-
-  _onReflow() {
-    reflowObservers.delete(this._doc);
-    this._doc.docShell.removeWeakReflowObserver(this);
-
-    for (let callback of this.callbacks) {
-      try {
-        callback();
-      } catch (e) {
-        Cu.reportError(e);
-      }
-    }
-  },
-
-  reflow() {
-    this._onReflow();
-  },
-
-  reflowInterruptible() {
-    this._onReflow();
-  },
-};
-
-const FLUSH_TYPES = {
-  "style": Ci.nsIDOMWindowUtils.FLUSH_STYLE,
-  "layout": Ci.nsIDOMWindowUtils.FLUSH_LAYOUT,
-  "display": Ci.nsIDOMWindowUtils.FLUSH_DISPLAY,
-};
-
 var BrowserUtils = {
 
   /**
    * Prints arguments separated by a space and appends a new line.
    */
   dumpLn(...args) {
     for (let a of args)
       dump(a + " ");
@@ -423,18 +381,17 @@ var BrowserUtils = {
       // needs to use #urlbar-container to calculate the bounds.
       toolbarItem = urlBarContainer;
     }
     if (!toolbarItem) {
       return;
     }
     let bounds = dwu.getBoundsWithoutFlushing(toolbarItem);
     if (!bounds.height) {
-      let document = element.ownerDocument;
-      await BrowserUtils.promiseLayoutFlushed(document, "layout", () => {
+      await window.promiseDocumentFlushed(() => {
         bounds = dwu.getBoundsWithoutFlushing(toolbarItem);
       });
     }
     if (bounds.height) {
       toolbarItem.style.setProperty("--toolbarbutton-height", bounds.height + "px");
     }
   },
 
@@ -683,75 +640,16 @@ var BrowserUtils = {
     if (hasPOSTParam) {
       postData = decodedPostData.replace(/%s/g, encodedParam)
                                 .replace(/%S/g, param);
     }
     return [url, postData];
   },
 
   /**
-   * Calls the given function when the given document has just reflowed,
-   * and returns a promise which resolves to its return value after it
-   * has been called.
-   *
-   * The function *must not trigger any reflows*, or make any changes
-   * which would require a layout flush.
-   *
-   * @param {Document} doc
-   * @param {function} callback
-   * @returns {Promise}
-   */
-  promiseReflowed(doc, callback) {
-    let observer = reflowObservers.get(doc);
-    if (!observer) {
-      observer = new ReflowObserver(doc);
-      reflowObservers.set(doc, observer);
-    }
-
-    return new Promise((resolve, reject) => {
-      observer.callbacks.push(() => {
-        try {
-          resolve(callback());
-        } catch (e) {
-          reject(e);
-        }
-      });
-    });
-  },
-
-  /**
-   * Calls the given function as soon as a layout flush of the given
-   * type is not necessary, and returns a promise which resolves to the
-   * callback's return value after it executes.
-   *
-   * The function *must not trigger any reflows*, or make any changes
-   * which would require a layout flush.
-   *
-   * @param {Document} doc
-   * @param {string} flushType
-   *        The flush type required. Must be one of:
-   *
-   *          - "style"
-   *          - "layout"
-   *          - "display"
-   * @param {function} callback
-   * @returns {Promise}
-   */
-  async promiseLayoutFlushed(doc, flushType, callback) {
-    let utils = doc.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
-                   .getInterface(Ci.nsIDOMWindowUtils);
-
-    if (!utils.needsFlush(FLUSH_TYPES[flushType])) {
-      return callback();
-    }
-
-    return this.promiseReflowed(doc, callback);
-  },
-
-  /**
    * Generate a document fragment for a localized string that has DOM
    * node replacements. This avoids using getFormattedString followed
    * by assigning to innerHTML. Fluent can probably replace this when
    * it is in use everywhere.
    *
    * @param {Document} doc
    * @param {String}   msg
    *                   The string to put replacements in. Fetch from