Bug 1358415: Don't trigger reflow just to compute tab geometry. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 21 Apr 2017 13:22:34 -0700
changeset 566579 bdf053e17d2ecb1ad5b52e074bd8459582a8b6ba
parent 566568 5ed2f24d732fa54640f5c796def8b57bdfd3d316
child 625357 39a3e3d0215c56c4ad7858e6a1c1e166f100cd0f
push id55263
push usermaglione.k@gmail.com
push dateFri, 21 Apr 2017 20:24:17 +0000
reviewersaswan
bugs1358415
milestone55.0a1
Bug 1358415: Don't trigger reflow just to compute tab geometry. r?aswan MozReview-Commit-ID: DnFSbDfOskT
browser/components/extensions/ext-tabs.js
browser/components/extensions/ext-utils.js
toolkit/components/extensions/ExtensionTabs.jsm
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -107,17 +107,17 @@ this.tabs = class extends ExtensionAPI {
           tabTracker.on("tab-activated", listener);
           return () => {
             tabTracker.off("tab-activated", listener);
           };
         }).api(),
 
         onCreated: new SingletonEventManager(context, "tabs.onCreated", fire => {
           let listener = (eventName, event) => {
-            fire.async(tabManager.convert(event.nativeTab));
+            fire.async(tabManager.convert(event.nativeTab, event.currentTab));
           };
 
           tabTracker.on("tab-created", listener);
           return () => {
             tabTracker.off("tab-created", listener);
           };
         }).api(),
 
@@ -355,16 +355,17 @@ this.tabs = class extends ExtensionAPI {
               }
             }
 
             // Make sure things like about:blank and data: URIs never inherit,
             // and instead always get a NullPrincipal.
             options.disallowInheritPrincipal = true;
 
             tabListener.initTabReady();
+            let currentTab = window.gBrowser.selectedTab;
             let nativeTab = window.gBrowser.addTab(url || window.BROWSER_NEW_TAB_URL, options);
 
             let active = true;
             if (createProperties.active !== null) {
               active = createProperties.active;
             }
             if (active) {
               window.gBrowser.selectedTab = nativeTab;
@@ -389,17 +390,17 @@ this.tabs = class extends ExtensionAPI {
 
               // Mark the tab as initializing, so that operations like
               // `executeScript` wait until the requested URL is loaded in
               // the tab before dispatching messages to the inner window
               // that contains the URL we're attempting to load.
               tabListener.initializingTabs.add(nativeTab);
             }
 
-            return tabManager.convert(nativeTab);
+            return tabManager.convert(nativeTab, currentTab);
           });
         },
 
         async remove(tabs) {
           if (!Array.isArray(tabs)) {
             tabs = [tabs];
           }
 
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -233,23 +233,28 @@ class TabTracker extends TabTrackerBase 
           // Copy the ID from the old tab to the new.
           this.setId(nativeTab, this.getId(adoptedTab));
 
           adoptedTab.linkedBrowser.messageManager.sendAsyncMessage("Extension:SetTabAndWindowId", {
             windowId: windowTracker.getId(nativeTab.ownerGlobal),
           });
         }
 
+        // Save the current tab, since the newly-created tab will likely be
+        // active by the time the promise below resolves and the event is
+        // dispatched.
+        let currentTab = nativeTab.ownerGlobal.gBrowser.selectedTab;
+
         // We need to delay sending this event until the next tick, since the
         // tab does not have its final index when the TabOpen event is dispatched.
         Promise.resolve().then(() => {
           if (event.detail.adoptedTab) {
-            this.emitAttached(event.originalTarget);
+            this.emitAttached(event.originalTarget, currentTab);
           } else {
-            this.emitCreated(event.originalTarget);
+            this.emitCreated(event.originalTarget, currentTab);
           }
         });
         break;
 
       case "TabClose":
         let {adoptedBy} = event.detail;
         if (adoptedBy) {
           // This tab is being closed because it was adopted by a new window.
@@ -384,20 +389,22 @@ class TabTracker extends TabTrackerBase 
     this.emit("tab-detached", {nativeTab, adoptedBy, tabId, oldWindowId, oldPosition: nativeTab._tPos});
   }
 
   /**
    * Emits a "tab-created" event for the given tab element.
    *
    * @param {NativeTab} nativeTab
    *        The tab element which is being created.
+   * @param {NativeTab} [currentTab]
+   *        The tab element for the currently active tab.
    * @private
    */
-  emitCreated(nativeTab) {
-    this.emit("tab-created", {nativeTab});
+  emitCreated(nativeTab, currentTab) {
+    this.emit("tab-created", {nativeTab, currentTab});
   }
 
   /**
    * Emits a "tab-removed" event for the given tab element.
    *
    * @param {NativeTab} nativeTab
    *        The tab element which is being removed.
    * @param {boolean} isWindowClosing
@@ -474,22 +481,28 @@ class Tab extends TabBase {
   get audible() {
     return this.nativeTab.soundPlaying;
   }
 
   get browser() {
     return this.nativeTab.linkedBrowser;
   }
 
+  get frameLoader() {
+    // If we don't have a frameLoader yet, just return a dummy with no width and
+    // height.
+    return super.frameLoader || {lazyWidth: 0, lazyHeight: 0};
+  }
+
   get cookieStoreId() {
     return getCookieStoreIdForTab(this, this.nativeTab);
   }
 
   get height() {
-    return this.browser.clientHeight;
+    return this.frameLoader.lazyHeight;
   }
 
   get index() {
     return this.nativeTab._tPos;
   }
 
   get mutedInfo() {
     let {nativeTab} = this;
@@ -520,17 +533,17 @@ class Tab extends TabBase {
   get status() {
     if (this.nativeTab.getAttribute("busy") === "true") {
       return "loading";
     }
     return "complete";
   }
 
   get width() {
-    return this.browser.clientWidth;
+    return this.frameLoader.lazyWidth;
   }
 
   get window() {
     return this.nativeTab.ownerGlobal;
   }
 
   get windowId() {
     return windowTracker.getId(this.window);
--- a/toolkit/components/extensions/ExtensionTabs.jsm
+++ b/toolkit/components/extensions/ExtensionTabs.jsm
@@ -280,16 +280,26 @@ class TabBase {
    *        @readonly
    *        @abstract
    */
   get browser() {
     throw new Error("Not implemented");
   }
 
   /**
+   * @property {nsIFrameLoader} browser
+   *        Returns the frameloader for the given tab.
+   *        @readonly
+   *        @abstract
+   */
+  get frameLoader() {
+    return this.browser.frameLoader;
+  }
+
+  /**
    * @property {string} cookieStoreId
    *        Returns the cookie store identifier for the given tab.
    *        @readonly
    *        @abstract
    */
   get cookieStoreId() {
     throw new Error("Not implemented");
   }
@@ -449,34 +459,44 @@ class TabBase {
     return true;
   }
 
   /**
    * Converts this tab object to a JSON-compatible object containing the values
    * of its properties which the extension is permitted to access, in the format
    * requried to be returned by WebExtension APIs.
    *
+   * @param {Tab} [fallbackTab]
+   *        A tab to retrieve geometry data from if the lazy geometry data for
+   *        this tab hasn't been initialized yet.
    * @returns {object}
    */
-  convert() {
+  convert(fallbackTab = null) {
     let result = {
       id: this.id,
       index: this.index,
       windowId: this.windowId,
       highlighted: this.selected,
       active: this.selected,
       pinned: this.pinned,
       status: this.status,
       incognito: this.incognito,
       width: this.width,
       height: this.height,
       audible: this.audible,
       mutedInfo: this.mutedInfo,
     };
 
+    // If the tab has not been fully layed-out yet, fallback to the geometry
+    // from a different tab (usually the currently active tab).
+    if (fallbackTab && (!result.width || !result.height)) {
+      result.width = fallbackTab.width;
+      result.height = fallbackTab.height;
+    }
+
     if (this.extension.hasPermission("cookies")) {
       result.cookieStoreId = this.cookieStoreId;
     }
 
     if (this.hasTabPermission) {
       for (let prop of ["url", "title", "favIconUrl"]) {
         // We use the underscored variants here to avoid the redundant
         // permissions checks imposed on the public properties.
@@ -1627,21 +1647,25 @@ class TabManagerBase {
 
   /**
    * Converts the given native tab to a JSON-compatible object, in the format
    * requried to be returned by WebExtension APIs, which may be safely passed to
    * extension code.
    *
    * @param {NativeTab} nativeTab
    *        The native tab to convert.
+   * @param {NativeTab} [fallbackTab]
+   *        A tab to retrieve geometry data from if the lazy geometry data for
+   *        this tab hasn't been initialized yet.
    *
    * @returns {Object}
    */
-  convert(nativeTab) {
-    return this.getWrapper(nativeTab).convert();
+  convert(nativeTab, fallbackTab = null) {
+    return this.getWrapper(nativeTab)
+               .convert(fallbackTab && this.getWrapper(fallbackTab));
   }
 
   // The JSDoc validator does not support @returns tags in abstract functions or
   // star functions without return statements.
   /* eslint-disable valid-jsdoc */
   /**
    * Returns an iterator of TabBase objects which match the given query info.
    *