Bug 1474248 - Improvements to legacy extension handling and the overlay loader; r=philipp a=jorgk
authorGeoff Lankow <geoff@darktrojan.net>
Tue, 17 Jul 2018 23:30:22 +1200
changeset 32340 8d96840851f322e476d94a6db4d6999a30cfef5d
parent 32339 ba4452fb46ee4988525d703da7db58b4c56426fa
child 32341 93917f786dd20f71742e2898ed27506aded0433a
push id385
push userclokep@gmail.com
push dateTue, 04 Sep 2018 23:26:14 +0000
reviewersphilipp, jorgk
bugs1474248
Bug 1474248 - Improvements to legacy extension handling and the overlay loader; r=philipp a=jorgk
common/content/overlayBindings.css
common/content/overlayBindings.xml
common/jar.mn
common/moz.build
common/src/Overlays.jsm
mail/components/extensions/parent/ext-legacy.js
mail/components/extensions/parent/ext-mail.js
mail/test/mozmill/folder-display/test-opening-messages.js
new file mode 100644
--- /dev/null
+++ b/common/content/overlayBindings.css
@@ -0,0 +1,7 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+overlayTrigger {
+  -moz-binding: url("chrome://messenger/content/overlayBindings.xml#overlayTrigger");
+}
new file mode 100644
--- /dev/null
+++ b/common/content/overlayBindings.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+
+<bindings id="overlay-bindings" xmlns="http://www.mozilla.org/xbl">
+  <binding id="overlayTrigger">
+    <implementation>
+      <constructor><![CDATA[
+          this.dispatchEvent(new CustomEvent("bindingattached", { bubbles: false }));
+      ]]></constructor>
+    </implementation>
+  </binding>
+</bindings>
new file mode 100644
--- /dev/null
+++ b/common/jar.mn
@@ -0,0 +1,3 @@
+messenger.jar:
+    content/messenger/overlayBindings.css (content/overlayBindings.css)
+    content/messenger/overlayBindings.xml (content/overlayBindings.xml)
--- a/common/moz.build
+++ b/common/moz.build
@@ -2,8 +2,10 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DIRS += [
     'public',
     'src'
 ]
+
+JAR_MANIFESTS += ['jar.mn']
--- a/common/src/Overlays.jsm
+++ b/common/src/Overlays.jsm
@@ -49,44 +49,55 @@ class Overlays {
    *
    * @param {ChromeManifest} overlayProvider        The overlay provider that contains information
    *                                                  about styles and overlays.
    * @param {DOMWindow} window                      The window to load into
    */
   constructor(overlayProvider, window) {
     this.overlayProvider = overlayProvider;
     this.window = window;
-
-    this.location = window.location.origin + window.location.pathname;
+    if (window.location.protocol == "about:") {
+      this.location = window.location.protocol + window.location.pathname;
+    } else {
+      this.location = window.location.origin + window.location.pathname;
+    }
   }
 
   /**
    * A shorthand to this.window.document
    */
   get document() {
     return this.window.document;
   }
 
   /**
    * Loads the given urls into the window, recursively loading further overlays as provided by the
    * overlayProvider.
    *
    * @param {String[]} urls                         The urls to load
    */
   load(urls) {
-    let unloadedOverlays = urls;
+    let unloadedOverlays = this._collectOverlays(this.document).concat(urls);
     let forwardReferences = [];
     let unloadedScripts = [];
     let unloadedSheets = [];
+    this._toolbarsToResolve = [];
+
+    // Load css styles from the registry
+    for (let sheet of this.overlayProvider.style.get(this.location, false)) {
+      unloadedSheets.push(sheet);
+    }
 
     while (unloadedOverlays.length) {
       let url = unloadedOverlays.shift();
       let xhr = this.fetchOverlay(url);
       let doc = xhr.responseXML;
 
+      oconsole.debug(`Applying ${url} to ${this.location}`);
+
       // clean the document a bit
       let emptyNodes = doc.evaluate("//text()[normalize-space(.) = '']", doc, null, 7, null);
       for (let i = 0, len = emptyNodes.snapshotLength; i < len; ++i) {
         let node = emptyNodes.snapshotItem(i);
         node.remove();
       }
 
       let commentNodes = doc.evaluate("//comment()", doc, null, 7, null);
@@ -106,24 +117,17 @@ class Overlays {
         let node = stylesheets.snapshotItem(i);
         let match = node.nodeValue.match(/href=["']([^"']*)["']/);
         if (match) {
           unloadedSheets.push(match[1]);
         }
       }
 
       // Prepare loading further nested xul overlays from the overlay
-      let xuloverlays = doc.evaluate("/processing-instruction('xul-overlay')", doc, null, 7, null);
-      for (let i = 0, len = xuloverlays.snapshotLength; i < len; ++i) {
-        let node = xuloverlays.snapshotItem(i);
-        let match = node.nodeValue.match(/href=["']([^"']*)["']/);
-        if (match) {
-          unloadedOverlays.push(match[1]);
-        }
-      }
+      unloadedOverlays.push(...this._collectOverlays(doc));
 
       // Prepare loading further nested xul overlays from the registry
       for (let overlayUrl of this.overlayProvider.overlay.get(url, false)) {
         unloadedOverlays.push(overlayUrl);
       }
 
       // Run through all overlay nodes on the first level (hookup nodes). Scripts will be deferred
       // until later for simplicity (c++ code seems to process them earlier?).
@@ -156,37 +160,52 @@ class Overlays {
       oconsole.warn(`Could not resolve ${forwardReferences.length} references`, forwardReferences);
     }
 
     // Loading the sheets now to avoid race conditions with xbl bindings
     for (let sheet of unloadedSheets) {
       this.loadCSS(sheet);
     }
 
+    for (let bar of this._toolbarsToResolve) {
+      let currentset = xulStoreService.getValue(this.location, bar.id, "currentset");
+      if (currentset) {
+        bar.currentSet = currentset;
+      } else {
+        bar.currentSet = bar.getAttribute("defaultset");
+      }
+    }
+
     // We've resolved all the forward references we can, we can now go ahead and load the scripts
     let deferredLoad = [];
     for (let script of unloadedScripts) {
       deferredLoad.push(...this.loadScript(script));
     }
 
-    if (this.window.document.readyState == "complete") {
-      // Now here is where it gets a little tricky. The subscript loader is synchronous, but the
-      // script itself may have some asynchronous side-effects (xbl bindings attached). Throwing in a
-      // 1500ms timeout before we fire the load handlers seems to help, even though it is an ugly hack.
+    let sheet;
+    let overlayTrigger = this.document.createElement("overlayTrigger");
+    overlayTrigger.addEventListener("bindingattached", () => {
+      oconsole.debug("XBL binding attached, continuing with load");
+      sheet.remove();
+      overlayTrigger.remove();
+
       setTimeout(() => {
         let ids = xulStoreService.getIDsEnumerator(this.location);
         while (ids.hasMore()) {
           let id = ids.getNext();
           let element = this.document.getElementById(id);
           if (element) {
             let attrNames = xulStoreService.getAttributeEnumerator(this.location, id);
             while (attrNames.hasMore()) {
               let attrName = attrNames.getNext();
               let attrValue = xulStoreService.getValue(this.location, id, attrName);
               element.setAttribute(attrName, attrValue);
+              if (attrName == "currentset") {
+                element.currentSet = attrValue;
+              }
             }
           }
         }
 
         // Now execute load handlers since we are done loading scripts
         let bubbles = [];
         for (let { listener, useCapture } of deferredLoad) {
           if (useCapture) {
@@ -194,23 +213,39 @@ class Overlays {
           } else {
             bubbles.push(listener);
           }
         }
 
         for (let listener of bubbles) {
           this._fireEventListener(listener);
         }
-      }, 1500);
-    } else {
-      // Window load is not yet complete, just add the listener normally
-      for (let { listener, useCapture } of deferredLoad) {
-        this.window.addEventListener("load", listener, useCapture);
+      }, 0);
+    }, { once: true });
+    this.document.documentElement.appendChild(overlayTrigger);
+    sheet = this.loadCSS("chrome://messenger/content/overlayBindings.css");
+  }
+
+  /**
+   * Gets the overlays referenced by processing instruction on a document.
+   *
+   * @param {DOMDocument} document  The document to read instuctions from
+   * @return {String[]}             URLs of the overlays from the document
+   */
+  _collectOverlays(doc) {
+    let urls = [];
+    let instructions = doc.evaluate("/processing-instruction('xul-overlay')", doc, null, 7, null);
+    for (let i = 0, len = instructions.snapshotLength; i < len; ++i) {
+      let node = instructions.snapshotItem(i);
+      let match = node.nodeValue.match(/href=["']([^"']*)["']/);
+      if (match) {
+        urls.push(match[1]);
       }
     }
+    return urls;
   }
 
   /**
    * Fires a "load" event for the given listener, using the current window
    *
    * @param {EventListener|Function} listener       The event listener to call
    */
   _fireEventListener(listener) {
@@ -229,30 +264,28 @@ class Overlays {
    * is merged in with the target node. If the node has no id it is inserted at documentElement
    * level.
    *
    * @param {Element} node          The DOM Element to resolve in the target document.
    * @return {Boolean}              True, if the node was merged/inserted, false otherwise
    */
   _resolveForwardReference(node) {
     if (node.id && node.localName == "toolbarpalette") {
-      let palette = this.document.getElementById(node.id);
-      if (!palette) {
-        // These vanish from the document but still exist via the palette property
-        let boxes = [...this.document.getElementsByTagName("toolbox")];
-        let box = boxes.find(box => box.palette && box.palette.id == node.id);
-        palette = box ? box.palette : null;
-      }
+      // These vanish from the document but still exist via the palette property
+      let boxes = [...this.document.getElementsByTagName("toolbox")];
+      let box = boxes.find(box => box.palette && box.palette.id == node.id);
+      let palette = box ? box.palette : null;
 
       if (!palette) {
         oconsole.debug(`The palette for ${node.id} could not be found, deferring to later`);
         return false;
       }
 
       this._mergeElement(palette, node);
+      this._toolbarsToResolve.push(...box.getElementsByTagName("toolbar"));
     } else if (node.id) {
       let target = this.document.getElementById(node.id);
       if (!target) {
         oconsole.debug(`The node ${node.id} could not be found, deferring to later`);
         return false;
       }
 
       this._mergeElement(target, node);
@@ -424,22 +457,24 @@ class Overlays {
     // loadSubScript is synchronous. Everyone else should be checking readyState anyway.
     return deferredLoad;
   }
 
   /**
    * Load the CSS stylesheet from the given url
    *
    * @param {String} url        The url to load from
+   * @return {Element}          An HTML link element for this stylesheet
    */
   loadCSS(url) {
     oconsole.debug(`Loading ${url} into ${this.window.location}`);
 
     // domWindowUtils.loadSheetUsingURIString doesn't record the sheet in document.styleSheets,
     // adding a html link element seems to do so.
     let link = this.document.createElementNS("http://www.w3.org/1999/xhtml", "link");
     link.setAttribute("rel", "stylesheet");
     link.setAttribute("type", "text/css");
     link.setAttribute("href", url);
 
     this.document.documentElement.appendChild(link);
+    return link;
   }
 }
--- a/mail/components/extensions/parent/ext-legacy.js
+++ b/mail/components/extensions/parent/ext-legacy.js
@@ -15,18 +15,20 @@ var loadedOnce = new Set();
 this.legacy = class extends ExtensionAPI {
   async onManifestEntry(entryName) {
     if (this.extension.manifest.legacy) {
       await this.register();
     }
   }
 
   async register() {
-    if (this.extension.startupReason != "APP_STARTUP") {
-      console.log(`Legacy WebExtension ${this.extension.id} loading for other reason than startup (${this.extension.startupReason}), refusing to load immediately`);
+    let enumerator = Services.wm.getEnumerator("mail:3pane");
+    if (enumerator.hasMoreElements() && enumerator.getNext().document.readyState == "complete") {
+      // It's too late!
+      console.log(`Legacy WebExtension ${this.extension.id} loading after app startup, refusing to load immediately.`);
       return;
     }
 
     this.extension.legacyLoaded = true;
 
     if (loadedOnce.has(this.extension.id)) {
       console.log(`Legacy WebExtension ${this.extension.id} has already been loaded in this run, refusing to do so again. Please restart`);
       return;
@@ -87,28 +89,24 @@ this.legacy = class extends ExtensionAPI
         }
 
         instance.observe(null, "profile-after-change", null);
       } catch (e) {
         console.error("Error firing profile-after-change listener for", contractid);
       }
     }
 
-    // Load overlays for each window
-    console.log("Loading legacy overlays for", this.extension.id);
-    let chromeManifestLoad = Overlays.load.bind(null, chromeManifest);
-    let targets = [...chromeManifest.overlay.keys()];
-    for (let target of targets) {
-      ExtensionSupport.registerWindowListener(this.extension.id + "-" + target, {
-        chromeURLs: [target],
-        onLoadWindow: chromeManifestLoad
-      });
-    }
+    let documentObserver = {
+      observe(document) {
+        if (ExtensionCommon.instanceOf(document, "XULDocument")) {
+          Overlays.load(chromeManifest, document.defaultView);
+        }
+      }
+    };
+    Services.obs.addObserver(documentObserver, "chrome-document-loaded");
 
     this.extension.callOnClose({
       close: () => {
-        for (let target of targets) {
-          ExtensionSupport.unregisterWindowListener(this.extension.id + "-" + target);
-        }
+        Services.obs.removeObserver(documentObserver, "chrome-document-loaded");
       }
     });
   }
 };
--- a/mail/components/extensions/parent/ext-mail.js
+++ b/mail/components/extensions/parent/ext-mail.js
@@ -1,32 +1,35 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var {
   ExtensionError,
+} = ExtensionUtils;
+
+var {
   defineLazyGetter,
-} = ExtensionUtils;
+} = ExtensionCommon;
 
 let tabTracker;
 let windowTracker;
 
 // This function is pretty tightly tied to Extension.jsm.
 // Its job is to fill in the |tab| property of the sender.
 const getSender = (extension, target, sender) => {
   let tabId = -1;
   if ("tabId" in sender) {
     // The message came from a privileged extension page running in a tab. In
     // that case, it should include a tabId property (which is filled in by the
     // page-open listener below).
     tabId = sender.tabId;
     delete sender.tabId;
-  } else if (ExtensionUtils.instanceOf(target, "XULElement") ||
-             ExtensionUtils.instanceOf(target, "HTMLIFrameElement")) {
+  } else if (ExtensionCommon.instanceOf(target, "XULElement") ||
+             ExtensionCommon.instanceOf(target, "HTMLIFrameElement")) {
     tabId = tabTracker.getBrowserData(target).tabId;
   }
 
   if (tabId != null && tabId >= 0) {
     let tab = extension.tabManager.get(tabId, null);
     if (tab) {
       sender.tab = tab.convert();
     }
--- a/mail/test/mozmill/folder-display/test-opening-messages.js
+++ b/mail/test/mozmill/folder-display/test-opening-messages.js
@@ -183,18 +183,19 @@ function check_message_pane_in_tab_full_
  * empty box is visible below it.
  */
 
 function check_message_pane_in_window_full_height(aWC) {
   let messengerWindowHeight = aWC.e("messengerWindow").boxObject.height;
   let messengerChildren = aWC.e("messengerWindow").children;
   let childrenHeightsSum = 0;
   let childrenHeightsStr = "";
-  for (var i=0; i < messengerChildren.length; i++) {
-    childrenHeightsSum += messengerChildren[i].boxObject.height;
-    childrenHeightsStr += '"' + messengerChildren[i].id + '": ' +
-                          messengerChildren[i].boxObject.height + ', ';
+  for (let child of messengerChildren) {
+    if ("boxObject" in child) {
+      childrenHeightsSum += child.boxObject.height;
+      childrenHeightsStr += '"' + child.id + '": ' + child.boxObject.height + ', ';
+    }
   }
 
   assert_equals(messengerWindowHeight, childrenHeightsSum,
     "messenger window height not equal to the sum of children heights: " +
     childrenHeightsStr);
 }