Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sat, 12 Nov 2016 16:13:21 -0800
changeset 438071 95f17737b12c457a9aece9159a6cf1e4c2a98174
parent 438070 9ed79e6ee300850f9dcd17e868194e9ed070df63
child 438072 0de312c933cf2e929b8c4c8208dd517570a522a4
push id35614
push usermaglione.k@gmail.com
push dateSun, 13 Nov 2016 03:28:59 +0000
reviewersaswan
bugs1317101
milestone52.0a1
Bug 1317101 - Part 5: Simply remote view initialization code, and fix some inconsistent handling. r?aswan MozReview-Commit-ID: 65BE0oF3rpI
browser/components/extensions/ext-utils.js
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ext-backgroundPage.js
--- a/browser/components/extensions/ext-utils.js
+++ b/browser/components/extensions/ext-utils.js
@@ -220,39 +220,32 @@ class BasePopup {
 
   createBrowser(viewNode, popupURL = null) {
     let document = viewNode.ownerDocument;
     this.browser = document.createElementNS(XUL_NS, "browser");
     this.browser.setAttribute("type", "content");
     this.browser.setAttribute("disableglobalhistory", "true");
     this.browser.setAttribute("transparent", "true");
     this.browser.setAttribute("class", "webextension-popup-browser");
+    this.browser.setAttribute("webextension-view-type", "popup");
     this.browser.setAttribute("tooltip", "aHTMLTooltip");
 
     // We only need flex sizing for the sake of the slide-in sub-views of the
     // main menu panel, so that the browser occupies the full width of the view,
     // and also takes up any extra height that's available to it.
     this.browser.setAttribute("flex", "1");
 
     // Note: When using noautohide panels, the popup manager will add width and
     // height attributes to the panel, breaking our resize code, if the browser
     // starts out smaller than 30px by 10px. This isn't an issue now, but it
     // will be if and when we popup debugging.
 
     viewNode.appendChild(this.browser);
 
     extensions.emit("extension-browser-inserted", this.browser);
-    let windowId = WindowManager.getId(this.browser.ownerGlobal);
-    this.browser.messageManager.sendAsyncMessage("Extension:InitExtensionView", {
-      viewType: "popup",
-      windowId,
-    });
-    // TODO(robwu): Rework this to use the Extension:ExtensionViewLoaded message
-    // to detect loads and so on. And definitely move this content logic inside
-    // a file in the child process.
 
     let initBrowser = browser => {
       let mm = browser.messageManager;
       mm.addMessageListener("DOMTitleChanged", this);
       mm.addMessageListener("Extension:BrowserBackgroundChanged", this);
       mm.addMessageListener("Extension:BrowserContentLoaded", this);
       mm.addMessageListener("Extension:BrowserResized", this);
       mm.addMessageListener("Extension:DOMWindowClose", this, true);
@@ -274,17 +267,17 @@ class BasePopup {
       mm.sendAsyncMessage("Extension:InitBrowser", {
         allowScriptsToClose: true,
         fixedWidth: this.fixedWidth,
         maxWidth: 800,
         maxHeight: 600,
         stylesheets: this.STYLESHEETS,
       });
 
-      this.browser.setAttribute("src", popupURL);
+      this.browser.loadURI(popupURL);
     });
   }
 
   resizeBrowser({width, height, detail}) {
     if (this.fixedWidth) {
       // Figure out how much extra space we have on the side of the panel
       // opposite the arrow.
       let side = this.panel.getAttribute("side") == "top" ? "bottom" : "top";
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -41,16 +41,17 @@ const {
   EventManager,
   SingletonEventManager,
   SpreadArgs,
   defineLazyGetter,
   getInnerWindowID,
   getMessageManager,
   getUniqueId,
   injectAPI,
+  promiseEvent,
 } = ExtensionUtils;
 
 const {
   BaseContext,
   LocalAPIImplementation,
   SchemaAPIInterface,
   SchemaAPIManager,
 } = ExtensionCommon;
@@ -857,26 +858,24 @@ class ContentGlobal {
     // Unless specified otherwise assume that the extension page is in a tab,
     // because the majority of all class instances are going to be a tab. Any
     // special views (background page, extension popup) will immediately send an
     // Extension:InitExtensionView message to change the viewType.
     this.viewType = "tab";
     this.tabId = -1;
     this.windowId = -1;
     this.initialized = false;
+
     this.global.addMessageListener("Extension:InitExtensionView", this);
     this.global.addMessageListener("Extension:SetTabAndWindowId", this);
-
-    this.initialDocuments = new WeakSet();
   }
 
   uninit() {
     this.global.removeMessageListener("Extension:InitExtensionView", this);
     this.global.removeMessageListener("Extension:SetTabAndWindowId", this);
-    this.global.removeEventListener("DOMContentLoaded", this);
   }
 
   ensureInitialized() {
     if (!this.initialized) {
       // Request tab and window ID in case "Extension:InitExtensionView" is not
       // sent (e.g. when `viewType` is "tab").
       let reply = this.global.sendSyncMessage("Extension:GetTabAndWindowId");
       this.handleSetTabAndWindowId(reply[0] || {});
@@ -884,66 +883,48 @@ class ContentGlobal {
     return this;
   }
 
   receiveMessage({name, data}) {
     switch (name) {
       case "Extension:InitExtensionView":
         // The view type is initialized once and then fixed.
         this.global.removeMessageListener("Extension:InitExtensionView", this);
-        let {viewType, url} = data;
-        this.viewType = viewType;
-        this.global.addEventListener("DOMContentLoaded", this);
-        if (url) {
-          // TODO(robwu): Remove this check. It is only here because the popup
-          // implementation does not always load a URL at the initialization,
-          // and the logic is too complex to fix at once.
-          let {document} = this.global.content;
-          this.initialDocuments.add(document);
-          document.location.replace(url);
-        }
-        /* Falls through to allow these properties to be initialized at once */
+        this.viewType = data.viewType;
+
+        promiseEvent(this.global, "DOMContentLoaded", true).then(() => {
+          this.global.sendAsyncMessage("Extension:ExtensionViewLoaded");
+        });
+
+        /* FALLTHROUGH */
       case "Extension:SetTabAndWindowId":
         this.handleSetTabAndWindowId(data);
         break;
     }
   }
 
   handleSetTabAndWindowId(data) {
     let {tabId, windowId} = data;
+
     if (tabId) {
       // Tab IDs are not expected to change.
       if (this.tabId !== -1 && tabId !== this.tabId) {
         throw new Error("Attempted to change a tabId after it was set");
       }
       this.tabId = tabId;
     }
+
     if (windowId !== undefined) {
       // Window IDs may change if a tab is moved to a different location.
       // Note: This is the ID of the browser window for the extension API.
       // Do not confuse it with the innerWindowID of DOMWindows!
       this.windowId = windowId;
     }
     this.initialized = true;
   }
-
-  // "DOMContentLoaded" event.
-  handleEvent(event) {
-    let {document} = this.global.content;
-    if (event.target === document) {
-      // If the document was still being loaded at the time of navigation, then
-      // the DOMContentLoaded event is fired for the old document. Ignore it.
-      if (this.initialDocuments.has(document)) {
-        this.initialDocuments.delete(document);
-        return;
-      }
-      this.global.removeEventListener("DOMContentLoaded", this);
-      this.global.sendAsyncMessage("Extension:ExtensionViewLoaded");
-    }
-  }
 }
 
 ExtensionChild = {
   // Map<nsIContentFrameMessageManager, ContentGlobal>
   contentGlobals: new Map(),
 
   // Map<innerWindowId, ExtensionPageContextChild>
   extensionContexts: new Map(),
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -224,16 +224,29 @@ GlobalManager = {
   _onExtensionBrowser(type, browser) {
     browser.messageManager.loadFrameScript(`data:,
       Components.utils.import("resource://gre/modules/ExtensionContent.jsm");
       ExtensionContent.init(this);
       addEventListener("unload", function() {
         ExtensionContent.uninit(this);
       });
     `, false);
+
+    let viewType = browser.getAttribute("webextension-view-type");
+    if (viewType) {
+      let data = {viewType};
+
+      let {getBrowserInfo} = apiManager.global;
+      if (getBrowserInfo) {
+        Object.assign(data, getBrowserInfo(browser));
+      }
+
+      browser.messageManager.sendAsyncMessage("Extension:InitExtensionView",
+                                              data);
+    }
   },
 
   getExtension(extensionId) {
     return this.extensionMap.get(extensionId);
   },
 
   injectInObject(context, isChromeCompat, dest) {
     apiManager.generateAPIs(context, dest);
--- a/toolkit/components/extensions/ext-backgroundPage.js
+++ b/toolkit/components/extensions/ext-backgroundPage.js
@@ -69,23 +69,22 @@ BackgroundPage.prototype = {
     yield promiseObserved("chrome-document-global-created",
                           win => win.document == chromeShell.document);
 
     let chromeDoc = yield promiseDocumentLoaded(chromeShell.document);
 
     let browser = chromeDoc.createElement("browser");
     browser.setAttribute("type", "content");
     browser.setAttribute("disableglobalhistory", "true");
+    browser.setAttribute("webextension-view-type", "background");
     chromeDoc.documentElement.appendChild(browser);
 
     extensions.emit("extension-browser-inserted", browser);
-    browser.messageManager.sendAsyncMessage("Extension:InitExtensionView", {
-      viewType: "background",
-      url,
-    });
+
+    browser.loadURI(url);
 
     yield new Promise(resolve => {
       browser.messageManager.addMessageListener("Extension:ExtensionViewLoaded", function onLoad() {
         browser.messageManager.removeMessageListener("Extension:ExtensionViewLoaded", onLoad);
         resolve();
       });
     });