Bug 1092156 - [e10s] Don't use compartment-per-addon if window already associated with add-on (r=bholley)
authorBill McCloskey <wmccloskey@mozilla.com>
Tue, 18 Nov 2014 16:20:59 -0800
changeset 240684 dcaec0014bf3fee123ec5f085b847218455bbb13
parent 240683 5bfcda09ea1327263cde327675ce5cd5804675a4
child 240685 23c81cdab64f3f182c8e62d569cffc7a4d08fea5
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1092156
milestone36.0a1
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 1092156 - [e10s] Don't use compartment-per-addon if window already associated with add-on (r=bholley)
dom/base/nsGlobalWindow.cpp
js/xpconnect/src/XPCWrappedNativeScope.cpp
toolkit/components/addoncompat/tests/addon/bootstrap.js
toolkit/components/addoncompat/tests/addon/chrome.manifest
toolkit/components/addoncompat/tests/addon/content/page.html
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -189,16 +189,17 @@
 #ifdef MOZ_GAMEPAD
 #include "mozilla/dom/GamepadService.h"
 #endif
 
 #include "nsRefreshDriver.h"
 
 #include "mozilla/dom/SelectionChangeEvent.h"
 
+#include "mozilla/AddonPathService.h"
 #include "mozilla/Services.h"
 #include "mozilla/Telemetry.h"
 #include "nsLocation.h"
 #include "nsHTMLDocument.h"
 #include "nsWrapperCacheInlines.h"
 #include "mozilla/DOMEventTargetHelper.h"
 #include "prrng.h"
 #include "nsSandboxFlags.h"
@@ -2265,16 +2266,24 @@ CreateNativeGlobalForInner(JSContext* aC
   nsCOMPtr<nsIExpandedPrincipal> nsEP = do_QueryInterface(aPrincipal);
   MOZ_RELEASE_ASSERT(!nsEP, "DOMWindow with nsEP is not supported");
 
   nsGlobalWindow *top = nullptr;
   if (aNewInner->GetOuterWindow()) {
     top = aNewInner->GetTop();
   }
   JS::CompartmentOptions options;
+
+  // Sometimes add-ons load their own XUL windows, either as separate top-level
+  // windows or inside a browser element. In such cases we want to tag the
+  // window's compartment with the add-on ID. See bug 1092156.
+  if (nsContentUtils::IsSystemPrincipal(aPrincipal)) {
+    options.setAddonId(MapURIToAddonID(aURI));
+  }
+
   if (top) {
     if (top->GetGlobalJSObject()) {
       options.setSameZoneAs(top->GetGlobalJSObject());
     }
   }
 
   // Determine if we need the Components object.
   bool needComponents = nsContentUtils::IsSystemPrincipal(aPrincipal) ||
--- a/js/xpconnect/src/XPCWrappedNativeScope.cpp
+++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp
@@ -341,16 +341,26 @@ XPCWrappedNativeScope::EnsureAddonScope(
 {
     JS::RootedObject global(cx, GetGlobalJSObject());
     MOZ_ASSERT(js::IsObjectInContextCompartment(global, cx));
     MOZ_ASSERT(!mIsContentXBLScope);
     MOZ_ASSERT(!mIsAddonScope);
     MOZ_ASSERT(addonId);
     MOZ_ASSERT(nsContentUtils::IsSystemPrincipal(GetPrincipal()));
 
+    // In bug 1092156, we found that add-on scopes don't work correctly when the
+    // window navigates. The add-on global's prototype is an outer window, so,
+    // after the navigation, looking up window properties in the add-on scope
+    // will fail. However, in most cases where the window can be navigated, the
+    // entire window is part of the add-on. To solve the problem, we avoid
+    // returning an add-on scope for a window that is already tagged with the
+    // add-on ID.
+    if (AddonIdOfObject(global) == addonId)
+        return global;
+
     // If we already have an addon scope object, we know what to use.
     for (size_t i = 0; i < mAddonScopes.Length(); i++) {
         if (JS::AddonIdOfObject(js::UncheckedUnwrap(mAddonScopes[i])) == addonId)
             return mAddonScopes[i];
     }
 
     SandboxOptions options;
     options.wantComponents = true;
--- a/toolkit/components/addoncompat/tests/addon/bootstrap.js
+++ b/toolkit/components/addoncompat/tests/addon/bootstrap.js
@@ -1,13 +1,14 @@
 var Cc = Components.classes;
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 
 Cu.import("resource://gre/modules/Services.jsm");
+Cu.import("resource://gre/modules/BrowserUtils.jsm");
 
 const baseURL = "http://mochi.test:8888/browser/" +
   "toolkit/components/addoncompat/tests/browser/";
 
 function forEachWindow(f)
 {
   let wins = Services.ww.getWindowEnumerator("navigator:browser");
   while (wins.hasMoreElements()) {
@@ -227,30 +228,56 @@ function testSandbox()
          "EP sandbox code ran successfully");
 
       gBrowser.removeTab(tab);
       resolve();
     }, true);
   });
 }
 
+// Test for bug 1095305. We just want to make sure that loading some
+// unprivileged content from an add-on package doesn't crash.
+function testAddonContent()
+{
+  let chromeRegistry = Components.classes["@mozilla.org/chrome/chrome-registry;1"]
+    .getService(Components.interfaces.nsIChromeRegistry);
+  let base = chromeRegistry.convertChromeURL(BrowserUtils.makeURI("chrome://addonshim1/content/"));
+
+  let res = Services.io.getProtocolHandler("resource")
+    .QueryInterface(Ci.nsIResProtocolHandler);
+  res.setSubstitution("addonshim1", base);
+
+  return new Promise(function(resolve, reject) {
+    const url = "resource://addonshim1/page.html";
+    let tab = gBrowser.addTab(url);
+    let browser = tab.linkedBrowser;
+    addLoadListener(browser, function handler() {
+      gBrowser.removeTab(tab);
+      res.setSubstitution("addonshim1", null);
+
+      resolve();
+    });
+  });
+}
+
 function runTests(win, funcs)
 {
   ok = funcs.ok;
   is = funcs.is;
   info = funcs.info;
 
   gWin = win;
   gBrowser = win.gBrowser;
 
   return testContentWindow().
     then(testListeners).
     then(testCapturing).
     then(testObserver).
-    then(testSandbox);
+    then(testSandbox).
+    then(testAddonContent);
 }
 
 /*
  bootstrap.js API
 */
 
 function startup(aData, aReason)
 {
--- a/toolkit/components/addoncompat/tests/addon/chrome.manifest
+++ b/toolkit/components/addoncompat/tests/addon/chrome.manifest
@@ -1,1 +1,1 @@
-content addonshim1 content/
\ No newline at end of file
+content addonshim1 content/
new file mode 100644
--- /dev/null
+++ b/toolkit/components/addoncompat/tests/addon/content/page.html
@@ -0,0 +1,2 @@
+<html>
+</html>