Bug 611242: InstallTrigger should not use a prototype shared across all windows. r=robstrong, a=blocks-betaN
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 11 Jan 2011 09:42:50 -0800
changeset 60303 286f71fe6a2ee5f240ab3a820ad55657bd4bceee
parent 60302 3df7a7d1699273cb79712cee9446c870c8e83d00
child 60304 e30ee0ae39b6a0f28289501005a98ddd3d8dcf03
push id17922
push userdtownsend@mozilla.com
push dateTue, 11 Jan 2011 17:54:17 +0000
treeherdermozilla-central@e30ee0ae39b6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobstrong, blocks-betaN
bugs611242
milestone2.0b10pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 611242: InstallTrigger should not use a prototype shared across all windows. r=robstrong, a=blocks-betaN
toolkit/mozapps/extensions/content/extensions-content.js
toolkit/mozapps/extensions/test/xpinstall/Makefile.in
toolkit/mozapps/extensions/test/xpinstall/browser_bug611242.js
--- a/toolkit/mozapps/extensions/content/extensions-content.js
+++ b/toolkit/mozapps/extensions/content/extensions-content.js
@@ -43,155 +43,155 @@ const Cu = Components.utils;
 
 const MSG_INSTALL_ENABLED  = "WebInstallerIsInstallEnabled";
 const MSG_INSTALL_ADDONS   = "WebInstallerInstallAddonsFromWebpage";
 const MSG_INSTALL_CALLBACK = "WebInstallerInstallCallback";
 
 var gIoService = Components.classes["@mozilla.org/network/io-service;1"]
                            .getService(Components.interfaces.nsIIOService);
 
-function InstallTrigger(window) {
-  this.window = window;
-}
+function createInstallTrigger(window) {
+  return {
+    window: window,
 
-InstallTrigger.prototype = {
-  __exposedProps__: {
-    SKIN: "r",
-    LOCALE: "r",
-    CONTENT: "r",
-    PACKAGE: "r",
-    enabled: "r",
-    updateEnabled: "r",
-    install: "r",
-    installChrome: "r",
-    startSoftwareUpdate: "r",
-    toSource: "r", // XXX workaround for bug 582100
-  },
+    __exposedProps__: {
+      SKIN: "r",
+      LOCALE: "r",
+      CONTENT: "r",
+      PACKAGE: "r",
+      enabled: "r",
+      updateEnabled: "r",
+      install: "r",
+      installChrome: "r",
+      startSoftwareUpdate: "r",
+      toSource: "r", // XXX workaround for bug 582100
+    },
 
-  // == Public interface ==
+    // == Public interface ==
+
+    SKIN: Ci.amIInstallTrigger.SKIN,
+    LOCALE: Ci.amIInstallTrigger.LOCALE,
+    CONTENT: Ci.amIInstallTrigger.CONTENT,
+    PACKAGE: Ci.amIInstallTrigger.PACKAGE,
 
-  SKIN: Ci.amIInstallTrigger.SKIN,
-  LOCALE: Ci.amIInstallTrigger.LOCALE,
-  CONTENT: Ci.amIInstallTrigger.CONTENT,
-  PACKAGE: Ci.amIInstallTrigger.PACKAGE,
+    /**
+     * @see amIInstallTriggerInstaller.idl
+     */
+    enabled: function() {
+      return sendSyncMessage(MSG_INSTALL_ENABLED, {
+        mimetype: "application/x-xpinstall", referer: this.window.location.href
+      })[0];
+    },
 
-  /**
-   * @see amIInstallTriggerInstaller.idl
-   */
-  enabled: function() {
-    return sendSyncMessage(MSG_INSTALL_ENABLED, {
-      mimetype: "application/x-xpinstall", referer: this.window.location.href
-    })[0];
-  },
+    /**
+     * @see amIInstallTriggerInstaller.idl
+     */
+    updateEnabled: function() {
+      return this.enabled();
+    },
 
-  /**
-   * @see amIInstallTriggerInstaller.idl
-   */
-  updateEnabled: function() {
-    return this.enabled();
-  },
+    /**
+     * @see amIInstallTriggerInstaller.idl
+     */
+    install: function(aArgs, aCallback) {
+      if (!aArgs || typeof aArgs != "object")
+        throw new Error("Incorrect arguments passed to InstallTrigger.install()");
 
-  /**
-   * @see amIInstallTriggerInstaller.idl
-   */
-  install: function(aArgs, aCallback) {
-    if (!aArgs || typeof aArgs != "object")
-      throw new Error("Incorrect arguments passed to InstallTrigger.install()");
+      var params = {
+        installerId: this.installerId,
+        mimetype: "application/x-xpinstall",
+        referer: this.window.location.href,
+        uris: [],
+        hashes: [],
+        names: [],
+        icons: [],
+      };
 
-    var params = {
-      installerId: this.installerId,
-      mimetype: "application/x-xpinstall",
-      referer: this.window.location.href,
-      uris: [],
-      hashes: [],
-      names: [],
-      icons: [],
-    };
+      for (var name in aArgs) {
+        var item = aArgs[name];
+        if (typeof item === 'string') {
+          item = { URL: item };
+        } else if (!("URL" in item) || item.URL === undefined) {
+          throw new Error("Missing URL property for '" + name + "'");
+        }
 
-    for (var name in aArgs) {
-      var item = aArgs[name];
-      if (typeof item === 'string') {
-        item = { URL: item };
-      } else if (!("URL" in item) || item.URL === undefined) {
-        throw new Error("Missing URL property for '" + name + "'");
-      }
+        // Resolve and validate urls
+        var url = this.resolveURL(item.URL);
+        if (!this.checkLoadURIFromScript(url))
+          throw new Error("insufficient permissions to install: " + url);
 
-      // Resolve and validate urls
-      var url = this.resolveURL(item.URL);
-      if (!this.checkLoadURIFromScript(url))
-        throw new Error("insufficient permissions to install: " + url);
-
-      var iconUrl = null;
-      if ("IconURL" in item && item.IconURL !== undefined) {
-        iconUrl = this.resolveURL(item.IconURL);
-        if (!this.checkLoadURIFromScript(iconUrl)) {
-          iconUrl = null; // If page can't load the icon, just ignore it
+        var iconUrl = null;
+        if ("IconURL" in item && item.IconURL !== undefined) {
+          iconUrl = this.resolveURL(item.IconURL);
+          if (!this.checkLoadURIFromScript(iconUrl)) {
+            iconUrl = null; // If page can't load the icon, just ignore it
+          }
         }
+        params.uris.push(url.spec);
+        params.hashes.push("Hash" in item ? item.Hash : null);
+        params.names.push(name);
+        params.icons.push(iconUrl ? iconUrl.spec : null);
       }
-      params.uris.push(url.spec);
-      params.hashes.push("Hash" in item ? item.Hash : null);
-      params.names.push(name);
-      params.icons.push(iconUrl ? iconUrl.spec : null);
-    }
-    // Add callback Id, done here, so only if we actually got here
-    params.callbackId = manager.addCallback(aCallback, params.uris);
-    // Send message
-    return sendSyncMessage(MSG_INSTALL_ADDONS, params)[0];
-  },
+      // Add callback Id, done here, so only if we actually got here
+      params.callbackId = manager.addCallback(aCallback, params.uris);
+      // Send message
+      return sendSyncMessage(MSG_INSTALL_ADDONS, params)[0];
+    },
 
-  /**
-   * @see amIInstallTriggerInstaller.idl
-   */
-  startSoftwareUpdate: function(aUrl, aFlags) {
-    var url = gIoService.newURI(aUrl, null, null)
-                        .QueryInterface(Ci.nsIURL).filename;
-    var object = {};
-    object[url] = { "URL": aUrl };
-    return this.install(object);
-  },
+    /**
+     * @see amIInstallTriggerInstaller.idl
+     */
+    startSoftwareUpdate: function(aUrl, aFlags) {
+      var url = gIoService.newURI(aUrl, null, null)
+                          .QueryInterface(Ci.nsIURL).filename;
+      var object = {};
+      object[url] = { "URL": aUrl };
+      return this.install(object);
+    },
 
-  /**
-   * @see amIInstallTriggerInstaller.idl
-   */
-  installChrome: function(aType, aUrl, aSkin) {
-    return this.startSoftwareUpdate(aUrl);
-  },
+    /**
+     * @see amIInstallTriggerInstaller.idl
+     */
+    installChrome: function(aType, aUrl, aSkin) {
+      return this.startSoftwareUpdate(aUrl);
+    },
 
-  /**
-   * Resolves a URL in the context of our current window. We need to do
-   * this before sending URLs to the parent process.
-   *
-   * @param  aUrl
-   *         The url to resolve.
-   *
-   * @return A resolved, absolute nsURI object.
-   */
-  resolveURL: function(aUrl) {
-    return gIoService.newURI(aUrl, null,
-                             this.window.document.documentURIObject);
-  },
+    /**
+     * Resolves a URL in the context of our current window. We need to do
+     * this before sending URLs to the parent process.
+     *
+     * @param  aUrl
+     *         The url to resolve.
+     *
+     * @return A resolved, absolute nsURI object.
+     */
+    resolveURL: function(aUrl) {
+      return gIoService.newURI(aUrl, null,
+                               this.window.document.documentURIObject);
+    },
 
-  /**
-   * @see amInstallTrigger.cpp
-   * TODO: When e10s lands on m-c, consider removing amInstallTrigger.cpp
-   *       See bug 571166
-   */
-  checkLoadURIFromScript: function(aUri) {
-    var secman = Cc["@mozilla.org/scriptsecuritymanager;1"].
-                 getService(Ci.nsIScriptSecurityManager);
-    var principal = this.window.content.document.nodePrincipal;
-    try {
-      secman.checkLoadURIWithPrincipal(principal, aUri,
-        Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL);
-      return true;
+    /**
+     * @see amInstallTrigger.cpp
+     * TODO: When e10s lands on m-c, consider removing amInstallTrigger.cpp
+     *       See bug 571166
+     */
+    checkLoadURIFromScript: function(aUri) {
+      var secman = Cc["@mozilla.org/scriptsecuritymanager;1"].
+                   getService(Ci.nsIScriptSecurityManager);
+      var principal = this.window.content.document.nodePrincipal;
+      try {
+        secman.checkLoadURIWithPrincipal(principal, aUri,
+          Ci.nsIScriptSecurityManager.DISALLOW_INHERIT_PRINCIPAL);
+        return true;
+      }
+      catch(e) {
+        return false;
+      }
     }
-    catch(e) {
-      return false;
-    }
-  },
+  };
 };
 
 /**
  * Child part of InstallTrigger e10s handling.
  *
  * Sets up InstallTrigger for newly-created windows,
  * that will relay messages for InstallTrigger
  * activity. We also process the parameters for
@@ -220,38 +220,27 @@ InstallTriggerManager.prototype = {
     // content windows. DOMWindowCreated is called on *all* HTMLDocuments,
     // some of which belong to chrome windows or other special content.
     //
     var uri = window.document.documentURIObject;
     if (uri.scheme === "chrome" || uri.spec.split(":")[0] == "about") {
       return;
     }
 
-    window.wrappedJSObject.__defineGetter__("InstallTrigger", this.createInstallTrigger);
-  },
+    window.wrappedJSObject.__defineGetter__("InstallTrigger", function() {
+      // We do this in a getter, so that we create these objects
+      // only on demand (this is a potential concern, since
+      // otherwise we might add one per iframe, and keep them
+      // alive for as long as the tab is alive).
 
-  createInstallTrigger: function createInstallTrigger() {
-    // We do this in a getter, so that we create these objects
-    // only on demand (this is a potential concern, since
-    // otherwise we might add one per iframe, and keep them
-    // alive for as long as the tab is alive).
-    // In order for this lazy instantiation to work, we need
-    // 'this' to be a window. However, we can get here with the
-    // window being on the prototype chain of our actual 'this'
-    // object (see bug 609794). Note that we need the
-    // XPCNativeWrapper.unwrap because getting the prototype
-    // doesn't respect the .wrappedJSObject unwrapping above.
-    var obj = XPCNativeWrapper.unwrap(this);
-    while (!obj.hasOwnProperty('InstallTrigger')) {
-      obj = XPCNativeWrapper.unwrap(Object.getPrototypeOf(obj));
-    }
-
-    delete obj.InstallTrigger; // remove getter
-    obj.InstallTrigger = new InstallTrigger(this);
-    return obj.InstallTrigger;
+      delete window.wrappedJSObject.InstallTrigger;
+      var installTrigger = createInstallTrigger(window.wrappedJSObject);
+      window.wrappedJSObject.InstallTrigger = installTrigger;
+      return installTrigger;
+    });
   },
 
   /**
    * Adds a callback to the list of callbacks we may receive messages
    * about from the parent process. We save them here; only callback IDs
    * are sent over IPC.
    *
    * @param  callback
--- a/toolkit/mozapps/extensions/test/xpinstall/Makefile.in
+++ b/toolkit/mozapps/extensions/test/xpinstall/Makefile.in
@@ -90,16 +90,17 @@ include $(topsrcdir)/config/rules.mk
                  browser_httphash.js \
                  browser_httphash2.js \
                  browser_httphash3.js \
                  browser_httphash4.js \
                  browser_httphash5.js \
                  browser_httphash6.js \
                  browser_badargs.js \
                  browser_badargs2.js \
+                 browser_bug611242.js \
                  unsigned.xpi \
                  signed.xpi \
                  signed2.xpi \
                  signed-no-o.xpi \
                  signed-no-cn.xpi \
                  signed-untrusted.xpi \
                  signed-tampered.xpi \
                  theme.xpi \
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpinstall/browser_bug611242.js
@@ -0,0 +1,32 @@
+// ----------------------------------------------------------------------------
+// Test whether setting a new property in InstallTrigger then persists to other
+// page loads
+function loadURI(aUri, aCallback) {
+  gBrowser.selectedBrowser.addEventListener("load", function() {
+    if (gBrowser.selectedBrowser.currentURI.spec != aUri)
+      return;
+
+    aCallback();
+  }, true);
+
+  gBrowser.loadURI(aUri);
+}
+
+function test() {
+  waitForExplicitFinish();
+
+  gBrowser.selectedTab = gBrowser.addTab();
+
+  loadURI(TESTROOT + "enabled.html", function() {
+    window.content.wrappedJSObject.InstallTrigger.enabled.k = function() { };
+
+    loadURI(TESTROOT2 + "enabled.html", function() {
+      is(window.content.wrappedJSObject.InstallTrigger.enabled.k, undefined, "Property should not be defined");
+
+      gBrowser.removeTab(gBrowser.selectedTab);
+
+      finish();
+    });
+  });
+}
+// ----------------------------------------------------------------------------