Bug 1301862 - Call tabs.create sooner r?kmag draft
authorRob Wu <rob@robwu.nl>
Sun, 11 Sep 2016 01:32:24 -0700
changeset 417116 1182c1b694d7b5785a1c84aff93b60c5d464a7f0
parent 416981 60cc643978c7020926fe4145761e26945fcd5c37
child 532038 0209ca24134f6db9e7e61b9755fb5517ed98a29d
push id30346
push userbmo:rob@robwu.nl
push dateFri, 23 Sep 2016 17:49:36 +0000
reviewerskmag
bugs1301862
milestone52.0a1
Bug 1301862 - Call tabs.create sooner r?kmag Call tabs.create immediately after the tab is created without delay to reduce the chance of losing tabs.onUpdated events. If needed, the tab waiting is done by `executeScript`, etc. MozReview-Commit-ID: 7A1zH99zafK
browser/components/extensions/ext-tabs.js
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -3,17 +3,18 @@
 "use strict";
 
 XPCOMUtils.defineLazyServiceGetter(this, "aboutNewTabService",
                                    "@mozilla.org/browser/aboutnewtab-service;1",
                                    "nsIAboutNewTabService");
 
 XPCOMUtils.defineLazyModuleGetter(this, "MatchPattern",
                                   "resource://gre/modules/MatchPattern.jsm");
-
+XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
+                                  "resource://gre/modules/PromiseUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Services",
                                   "resource://gre/modules/Services.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
   EventManager,
   ignoreEvent,
@@ -237,42 +238,62 @@ let tabListener = {
     let windowId = WindowManager.getId(tab.ownerGlobal);
     let tabId = TabManager.getId(tab);
 
     this.emit("tab-removed", {tab, tabId, windowId, isWindowClosing});
   },
 
   tabReadyInitialized: false,
   tabReadyPromises: new WeakMap(),
+  initializingTabs: new WeakSet(),
 
   initTabReady() {
     if (!this.tabReadyInitialized) {
       AllWindowEvents.addListener("progress", this);
 
       this.tabReadyInitialized = true;
     }
   },
 
   onLocationChange(browser, webProgress, request, locationURI, flags) {
     if (webProgress.isTopLevel) {
       let gBrowser = browser.ownerGlobal.gBrowser;
       let tab = gBrowser.getTabForBrowser(browser);
 
+      // Now we are certain that the first page in the tab was loaded.
+      this.initializingTabs.delete(tab);
+
+      // browser.innerWindowID is now set, resolve the promises if any.
       let deferred = this.tabReadyPromises.get(tab);
       if (deferred) {
         deferred.resolve(tab);
         this.tabReadyPromises.delete(tab);
       }
     }
   },
 
+  /**
+   * Returns a promise that resolves when the tab is ready.
+   * Tabs created via the `tabs.create` method are "ready" once the location
+   * changed to the requested URL. Other tabs are always assumed to be ready.
+   *
+   * @param {XULElement} tab The <tab> element.
+   * @returns {Promise} Resolves with the given tab once ready.
+   */
   awaitTabReady(tab) {
-    return new Promise((resolve, reject) => {
-      this.tabReadyPromises.set(tab, {resolve, reject});
-    });
+    let deferred = this.tabReadyPromises.get(tab);
+    if (!deferred) {
+      deferred = PromiseUtils.defer();
+      if (!this.initializingTabs.has(tab) && tab.linkedBrowser.innerWindowID) {
+        deferred.resolve(tab);
+      } else {
+        this.tabReadyPromises.set(tab, deferred);
+      }
+    }
+    return deferred.promise;
   },
 };
 
 extensions.registerSchemaAPI("tabs", "addon_parent", context => {
   let {extension} = context;
   let self = {
     tabs: {
       onActivated: new WindowEventManager(context, "tabs.onActivated", "TabSelect", (fire, event) => {
@@ -533,28 +554,28 @@ extensions.registerSchemaAPI("tabs", "ad
           if (createProperties.index !== null) {
             window.gBrowser.moveTabTo(tab, createProperties.index);
           }
 
           if (createProperties.pinned) {
             window.gBrowser.pinTab(tab);
           }
 
-          if (!createProperties.url || createProperties.url.startsWith("about:")) {
+          if (createProperties.url && !createProperties.url.startsWith("about:")) {
             // We can't wait for a location change event for about:newtab,
             // since it may be pre-rendered, in which case its initial
             // location change event has already fired.
-            return tab;
+
+            // 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(tab);
           }
 
-          // Wait for the first location change event, so that operations
-          // like `executeScript` are dispatched to the inner window that
-          // contains the URL we're attempting to load.
-          return tabListener.awaitTabReady(tab);
-        }).then(tab => {
           return TabManager.convert(extension, tab);
         });
       },
 
       remove: function(tabs) {
         if (!Array.isArray(tabs)) {
           tabs = [tabs];
         }
@@ -712,75 +733,75 @@ extensions.registerSchemaAPI("tabs", "ad
         if (!extension.hasPermission("<all_urls>")) {
           return Promise.reject({message: "The <all_urls> permission is required to use the captureVisibleTab API"});
         }
 
         let window = windowId == null ?
           WindowManager.topWindow :
           WindowManager.getWindow(windowId, context);
 
-        let browser = window.gBrowser.selectedBrowser;
-        let recipient = {
-          innerWindowID: browser.innerWindowID,
-        };
+        let tab = window.gBrowser.selectedTab;
+        return tabListener.awaitTabReady(tab).then(() => {
+          let browser = tab.linkedBrowser;
+          let recipient = {
+            innerWindowID: browser.innerWindowID,
+          };
 
-        if (!options) {
-          options = {};
-        }
-        if (options.format == null) {
-          options.format = "png";
-        }
-        if (options.quality == null) {
-          options.quality = 92;
-        }
+          if (!options) {
+            options = {};
+          }
+          if (options.format == null) {
+            options.format = "png";
+          }
+          if (options.quality == null) {
+            options.quality = 92;
+          }
 
-        let message = {
-          options,
-          width: browser.clientWidth,
-          height: browser.clientHeight,
-        };
+          let message = {
+            options,
+            width: browser.clientWidth,
+            height: browser.clientHeight,
+          };
 
-        return context.sendMessage(browser.messageManager, "Extension:Capture",
-                                   message, {recipient});
+          return context.sendMessage(browser.messageManager, "Extension:Capture",
+                                     message, {recipient});
+        });
       },
 
       detectLanguage: function(tabId) {
         let tab = tabId !== null ? TabManager.getTab(tabId, context) : TabManager.activeTab;
 
-        let browser = tab.linkedBrowser;
-        let recipient = {innerWindowID: browser.innerWindowID};
+        return tabListener.awaitTabReady(tab).then(() => {
+          let browser = tab.linkedBrowser;
+          let recipient = {innerWindowID: browser.innerWindowID};
 
-        return context.sendMessage(browser.messageManager, "Extension:DetectLanguage",
-                                   {}, {recipient});
+          return context.sendMessage(browser.messageManager, "Extension:DetectLanguage",
+                                     {}, {recipient});
+        });
       },
 
       // Used to executeScript, insertCSS and removeCSS.
       _execute: function(tabId, details, kind, method) {
         let tab = tabId !== null ? TabManager.getTab(tabId, context) : TabManager.activeTab;
-        let mm = tab.linkedBrowser.messageManager;
 
         let options = {
           js: [],
           css: [],
           remove_css: method == "removeCSS",
         };
 
         // We require a `code` or a `file` property, but we can't accept both.
         if ((details.code === null) == (details.file === null)) {
           return Promise.reject({message: `${method} requires either a 'code' or a 'file' property, but not both`});
         }
 
         if (details.frameId !== null && details.allFrames) {
           return Promise.reject({message: `'frameId' and 'allFrames' are mutually exclusive`});
         }
 
-        let recipient = {
-          innerWindowID: tab.linkedBrowser.innerWindowID,
-        };
-
         if (TabManager.for(extension).hasActiveTabPermission(tab)) {
           // If we have the "activeTab" permission for this tab, ignore
           // the host whitelist.
           options.matchesHost = ["<all_urls>"];
         } else {
           options.matchesHost = extension.whiteListedHosts.serialize();
         }
 
@@ -804,17 +825,24 @@ extensions.registerSchemaAPI("tabs", "ad
           options.match_about_blank = details.matchAboutBlank;
         }
         if (details.runAt !== null) {
           options.run_at = details.runAt;
         } else {
           options.run_at = "document_idle";
         }
 
-        return context.sendMessage(mm, "Extension:Execute", {options}, {recipient});
+        return tabListener.awaitTabReady(tab).then(() => {
+          let browser = tab.linkedBrowser;
+          let recipient = {
+            innerWindowID: browser.innerWindowID,
+          };
+
+          return context.sendMessage(browser.messageManager, "Extension:Execute", {options}, {recipient});
+        });
       },
 
       executeScript: function(tabId, details) {
         return self.tabs._execute(tabId, details, "js", "executeScript");
       },
 
       insertCSS: function(tabId, details) {
         return self.tabs._execute(tabId, details, "css", "insertCSS").then(() => {});