Bug 1397360 - Fix BrowserUtils.promiseLayoutFlushed to always return a Promise and unregister its callbacks to avoid memory leaks. r?kmag draft
authorMike de Boer <mdeboer@mozilla.com>
Wed, 06 Sep 2017 19:02:14 +0200
changeset 660183 d4dbef2483a77abcdf5dc35f29806fc9d6b4d263
parent 660013 997ad5a443f606596d2e404a02cf0b6af8c6c8b8
child 660189 f044a2bee520a081ac062c0a6b35874df816d4c2
push id78305
push usermdeboer@mozilla.com
push dateWed, 06 Sep 2017 17:07:39 +0000
reviewerskmag
bugs1397360
milestone57.0a1
Bug 1397360 - Fix BrowserUtils.promiseLayoutFlushed to always return a Promise and unregister its callbacks to avoid memory leaks. r?kmag I also added a non-Promise version to aid in the development of timing-sensitive features, like animations, because using callbacks instead avoids mixing two different clock-based primitives. MozReview-Commit-ID: GhPHkhVpE2o
toolkit/modules/BrowserUtils.jsm
--- a/toolkit/modules/BrowserUtils.jsm
+++ b/toolkit/modules/BrowserUtils.jsm
@@ -38,16 +38,30 @@ ReflowObserver.prototype = {
       try {
         callback();
       } catch (e) {
         Cu.reportError(e);
       }
     }
   },
 
+  addCallback(callback) {
+    if (this.callbacks.indexOf(callback) == -1) {
+      this.callbacks.push(callback);
+    }
+  },
+
+  removeCallback(callback) {
+    let idx = this.callbacks.indexOf(callback);
+    if (idx == -1) {
+      return;
+    }
+    this.callbacks.splice(idx, 1);
+  },
+
   reflow() {
     this._onReflow();
   },
 
   reflowInterruptible() {
     this._onReflow();
   },
 };
@@ -672,38 +686,80 @@ this.BrowserUtils = {
    * 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) {
+  whenReflowed(doc, callback) {
     let observer = reflowObservers.get(doc);
     if (!observer) {
       observer = new ReflowObserver(doc);
       reflowObservers.set(doc, observer);
     }
 
+    observer.addCallback(function onReflow() {
+      observer.removeCallback(onReflow);
+      callback();
+    });
+  },
+
+  /**
+   * 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) {
     return new Promise((resolve, reject) => {
-      observer.callbacks.push(() => {
+      this.whenReflowed(doc, () => {
         try {
           resolve(callback());
         } catch (e) {
           reject(e);
         }
       });
     });
   },
 
   /**
    * Calls the given function as soon as a layout flush of the given
+   * type is not necessary, or waits for a reflow and calls the function then.
+   *
+   * @param {Document} doc
+   * @param {string} flushType
+   *        The flush type required. Must be one of:
+   *
+   *          - "style"
+   *          - "layout"
+   *          - "display"
+   * @param {function} callback
+   */
+  whenLayoutFlushed(doc, flushType, callback) {
+    let utils = doc.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
+                   .getInterface(Ci.nsIDOMWindowUtils);
+
+    if (!utils.needsFlush(FLUSH_TYPES[flushType])) {
+      callback();
+    } else {
+      this.whenReflowed(doc, callback);
+    }
+  },
+
+  /**
+   * 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
@@ -715,14 +771,14 @@ this.BrowserUtils = {
    * @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 Promise.resolve(callback());
     }
 
     return this.promiseReflowed(doc, callback);
   },
 };