From 62af1a5643a459a44062a32774173a804eb5f623 Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Tue, 18 Nov 2014 16:20:59 -0800 Subject: [PATCH] Bug 1092156 - [e10s] Don't use compartment-per-addon if window already associated with add-on (r=bholley) --- dom/base/nsGlobalWindow.cpp | 9 ++++++ js/xpconnect/src/XPCWrappedNativeScope.cpp | 10 +++++++ .../addoncompat/tests/addon/bootstrap.js | 29 ++++++++++++++++++- .../addoncompat/tests/addon/chrome.manifest | 2 +- .../addoncompat/tests/addon/content/page.html | 2 ++ 5 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 toolkit/components/addoncompat/tests/addon/content/page.html diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 99bfac307d5c..8922327344cb 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -194,6 +194,7 @@ #include "mozilla/dom/SelectionChangeEvent.h" +#include "mozilla/AddonPathService.h" #include "mozilla/Services.h" #include "mozilla/Telemetry.h" #include "nsLocation.h" @@ -2270,6 +2271,14 @@ CreateNativeGlobalForInner(JSContext* aCx, 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()); diff --git a/js/xpconnect/src/XPCWrappedNativeScope.cpp b/js/xpconnect/src/XPCWrappedNativeScope.cpp index 35eedabb6415..c6831981c8b8 100644 --- a/js/xpconnect/src/XPCWrappedNativeScope.cpp +++ b/js/xpconnect/src/XPCWrappedNativeScope.cpp @@ -346,6 +346,16 @@ XPCWrappedNativeScope::EnsureAddonScope(JSContext *cx, JSAddonId *addonId) 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) diff --git a/toolkit/components/addoncompat/tests/addon/bootstrap.js b/toolkit/components/addoncompat/tests/addon/bootstrap.js index 043f23045562..e54569f924ab 100644 --- a/toolkit/components/addoncompat/tests/addon/bootstrap.js +++ b/toolkit/components/addoncompat/tests/addon/bootstrap.js @@ -3,6 +3,7 @@ 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/"; @@ -232,6 +233,31 @@ function testSandbox() }); } +// 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; @@ -245,7 +271,8 @@ function runTests(win, funcs) then(testListeners). then(testCapturing). then(testObserver). - then(testSandbox); + then(testSandbox). + then(testAddonContent); } /* diff --git a/toolkit/components/addoncompat/tests/addon/chrome.manifest b/toolkit/components/addoncompat/tests/addon/chrome.manifest index 443666893e47..602ba3a5dc28 100644 --- a/toolkit/components/addoncompat/tests/addon/chrome.manifest +++ b/toolkit/components/addoncompat/tests/addon/chrome.manifest @@ -1 +1 @@ -content addonshim1 content/ \ No newline at end of file +content addonshim1 content/ diff --git a/toolkit/components/addoncompat/tests/addon/content/page.html b/toolkit/components/addoncompat/tests/addon/content/page.html new file mode 100644 index 000000000000..90531a4b3ed1 --- /dev/null +++ b/toolkit/components/addoncompat/tests/addon/content/page.html @@ -0,0 +1,2 @@ + +