From e9c3a589bba2f94fb498d2f841fe2f746d15c304 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 2 Nov 2016 19:21:04 -0700 Subject: [PATCH] Bug 1312690: Lazily initialize extension APIs. r=aswan MozReview-Commit-ID: 2ofzT6wPvus --HG-- extra : rebase_source : 462e3eca2a9750fb07ad753322e86bc1494a8b4e --- browser/components/extensions/ext-utils.js | 17 +++-- .../browser/browser_ext_popup_shutdown.js | 2 - toolkit/components/extensions/Extension.jsm | 23 +++--- .../components/extensions/ExtensionChild.jsm | 71 ++++++++++++------- .../extensions/ExtensionContent.jsm | 68 +++++++++++------- .../components/extensions/ExtensionUtils.jsm | 46 +++++++++++- .../test/xpcshell/test_ext_api_permissions.js | 15 +++- 7 files changed, 167 insertions(+), 75 deletions(-) diff --git a/browser/components/extensions/ext-utils.js b/browser/components/extensions/ext-utils.js index 00fb499cb199..5ed57f08eac0 100644 --- a/browser/components/extensions/ext-utils.js +++ b/browser/components/extensions/ext-utils.js @@ -78,15 +78,6 @@ XPCOMUtils.defineLazyGetter(this, "standaloneStylesheets", () => { return stylesheets; }); -/* eslint-disable mozilla/balanced-listeners */ -extensions.on("page-shutdown", (type, context) => { - if (context.viewType == "popup" && context.active) { - // TODO(robwu): This is not webext-oop compatible. - context.xulBrowser.contentWindow.close(); - } -}); -/* eslint-enable mozilla/balanced-listeners */ - class BasePopup { constructor(extension, viewNode, popupURL, browserStyle, fixedWidth = false) { this.extension = extension; @@ -97,6 +88,8 @@ class BasePopup { this.destroyed = false; this.fixedWidth = fixedWidth; + extension.callOnClose(this); + this.contentReady = new Promise(resolve => { this._resolveContentReady = resolve; }); @@ -120,7 +113,13 @@ class BasePopup { return BasePopup.instances.get(window).get(extension); } + close() { + this.closePopup(); + } + destroy() { + this.extension.forgetOnClose(this); + this.destroyed = true; this.browserLoadedDeferred.reject(new Error("Popup destroyed")); return this.browserReady.then(() => { diff --git a/browser/components/extensions/test/browser/browser_ext_popup_shutdown.js b/browser/components/extensions/test/browser/browser_ext_popup_shutdown.js index 4f317ca88fed..22ee2b10043a 100644 --- a/browser/components/extensions/test/browser/browser_ext_popup_shutdown.js +++ b/browser/components/extensions/test/browser/browser_ext_popup_shutdown.js @@ -74,7 +74,5 @@ add_task(function* testPageAction() { yield extension.unload(); - yield new Promise(resolve => setTimeout(resolve, 0)); - is(panel.parentNode, null, "Panel should be removed from the document"); }); diff --git a/toolkit/components/extensions/Extension.jsm b/toolkit/components/extensions/Extension.jsm index 2ab51b13e972..7ad9a376d171 100644 --- a/toolkit/components/extensions/Extension.jsm +++ b/toolkit/components/extensions/Extension.jsm @@ -83,6 +83,7 @@ if (!AppConstants.RELEASE_OR_BETA) { Cu.import("resource://gre/modules/ExtensionUtils.jsm"); var { BaseContext, + defineLazyGetter, EventEmitter, SchemaAPIManager, LocaleData, @@ -289,22 +290,16 @@ class ProxyContext extends BaseContext { this.currentMessageManager = xulBrowser.messageManager; this._docShellTracker = new BrowserDocshellFollower(xulBrowser, this.onBrowserChange.bind(this)); - this.principal_ = principal; - this.apiObj = {}; - GlobalManager.injectInObject(this, false, this.apiObj); + Object.defineProperty(this, "principal", { + value: principal, enumerable: true, configurable: true, + }); this.listenerProxies = new Map(); - this.sandbox = Cu.Sandbox(principal, {}); - Management.emit("proxy-context-load", this); } - get principal() { - return this.principal_; - } - get cloneScope() { return this.sandbox; } @@ -334,6 +329,16 @@ class ProxyContext extends BaseContext { } } +defineLazyGetter(ProxyContext.prototype, "apiObj", function() { + let obj = {}; + GlobalManager.injectInObject(this, false, obj); + return obj; +}); + +defineLazyGetter(ProxyContext.prototype, "sandbox", function() { + return Cu.Sandbox(this.principal); +}); + // The parent ProxyContext of an ExtensionContext in ExtensionChild.jsm. class ExtensionChildProxyContext extends ProxyContext { constructor(envType, extension, params, xulBrowser) { diff --git a/toolkit/components/extensions/ExtensionChild.jsm b/toolkit/components/extensions/ExtensionChild.jsm index be0f2cbba0a1..0d501282df4e 100644 --- a/toolkit/components/extensions/ExtensionChild.jsm +++ b/toolkit/components/extensions/ExtensionChild.jsm @@ -34,6 +34,7 @@ var { getInnerWindowID, BaseContext, ChildAPIManager, + defineLazyGetter, LocalAPIImplementation, Messenger, SchemaAPIManager, @@ -169,33 +170,22 @@ class ExtensionContext extends BaseContext { if (uri) { sender.url = uri.spec; } + this.sender = sender; - let filter = {extensionId: extension.id}; - let optionalFilter = {}; - // Addon-generated messages (not necessarily from the same process as the - // addon itself) are sent to the main process, which forwards them via the - // parent process message manager. Specific replies can be sent to the frame - // message manager. - this.messenger = new Messenger(this, [Services.cpmm, this.messageManager], sender, filter, optionalFilter); - - let localApis = {}; - apiManager.generateAPIs(this, localApis); - this.childManager = new WannabeChildAPIManager(this, this.messageManager, localApis, { - envType: "addon_parent", - viewType, - url: uri.spec, + Schemas.exportLazyGetter(contentWindow, "browser", () => { + let browserObj = Cu.createObjectIn(contentWindow); + Schemas.inject(browserObj, this.childManager); + return browserObj; }); - let chromeApiWrapper = Object.create(this.childManager); - chromeApiWrapper.isChromeCompat = true; - let browserObj = Cu.createObjectIn(contentWindow, {defineAs: "browser"}); - let chromeObj = Cu.createObjectIn(contentWindow, {defineAs: "chrome"}); - Schemas.inject(browserObj, this.childManager); - Schemas.inject(chromeObj, chromeApiWrapper); + Schemas.exportLazyGetter(contentWindow, "chrome", () => { + let chromeApiWrapper = Object.create(this.childManager); + chromeApiWrapper.isChromeCompat = true; - if (viewType == "background") { - apiManager.global.initializeBackgroundPage(contentWindow); - } + let chromeObj = Cu.createObjectIn(contentWindow); + Schemas.inject(chromeObj, chromeApiWrapper); + return chromeObj; + }); this.extension.views.add(this); } @@ -230,12 +220,45 @@ class ExtensionContext extends BaseContext { return; } + if (this.contentWindow) { + this.contentWindow.close(); + } + super.unload(); - this.childManager.close(); this.extension.views.delete(this); } } +defineLazyGetter(ExtensionContext.prototype, "messenger", function() { + let filter = {extensionId: this.extension.id}; + let optionalFilter = {}; + // Addon-generated messages (not necessarily from the same process as the + // addon itself) are sent to the main process, which forwards them via the + // parent process message manager. Specific replies can be sent to the frame + // message manager. + return new Messenger(this, [Services.cpmm, this.messageManager], this.sender, + filter, optionalFilter); +}); + +defineLazyGetter(ExtensionContext.prototype, "childManager", function() { + let localApis = {}; + apiManager.generateAPIs(this, localApis); + + if (this.viewType == "background") { + apiManager.global.initializeBackgroundPage(this.contentWindow); + } + + let childManager = new WannabeChildAPIManager(this, this.messageManager, localApis, { + envType: "addon_parent", + viewType: this.viewType, + url: this.uri.spec, + }); + + this.callOnClose(childManager); + + return childManager; +}); + // All subframes in a tab, background page, popup, etc. have the same view type. // This class keeps track of such global state. // Note that this is created even for non-extension tabs because at present we diff --git a/toolkit/components/extensions/ExtensionContent.jsm b/toolkit/components/extensions/ExtensionContent.jsm index 8222d346b3ce..a6f08c4773fc 100644 --- a/toolkit/components/extensions/ExtensionContent.jsm +++ b/toolkit/components/extensions/ExtensionContent.jsm @@ -49,6 +49,7 @@ Cu.import("resource://gre/modules/ExtensionChild.jsm"); Cu.import("resource://gre/modules/ExtensionUtils.jsm"); var { runSafeSyncWithoutClone, + defineLazyGetter, BaseContext, LocaleData, Messenger, @@ -274,7 +275,6 @@ class ExtensionContext extends BaseContext { this.scripts = []; - let prin; let contentPrincipal = contentWindow.document.nodePrincipal; let ssm = Services.scriptSecurityManager; @@ -284,12 +284,13 @@ class ExtensionContext extends BaseContext { attrs.addonId = this.extension.id; let extensionPrincipal = ssm.createCodebasePrincipal(this.extension.baseURI, attrs); + let principal; if (ssm.isSystemPrincipal(contentPrincipal)) { // Make sure we don't hand out the system principal by accident. // also make sure that the null principal has the right origin attributes - prin = ssm.createNullPrincipal(attrs); + principal = ssm.createNullPrincipal(attrs); } else { - prin = [contentPrincipal, extensionPrincipal]; + principal = [contentPrincipal, extensionPrincipal]; } if (isExtensionPage) { @@ -314,7 +315,7 @@ class ExtensionContext extends BaseContext { addonId: attrs.addonId, }; - this.sandbox = Cu.Sandbox(prin, { + this.sandbox = Cu.Sandbox(principal, { metadata, sandboxPrototype: contentWindow, wantXrays: true, @@ -337,32 +338,24 @@ class ExtensionContext extends BaseContext { configurable: true, }); - let url = contentWindow.location.href; - // The |sender| parameter is passed directly to the extension. - let sender = {id: this.extension.uuid, frameId, url}; - let filter = {extensionId: this.extension.id}; - let optionalFilter = {frameId}; - this.messenger = new Messenger(this, [this.messageManager], sender, filter, optionalFilter); + this.url = contentWindow.location.href; - this.chromeObj = Cu.createObjectIn(this.sandbox, {defineAs: "browser"}); + defineLazyGetter(this, "chromeObj", () => { + let chromeObj = Cu.createObjectIn(this.sandbox); - // Sandboxes don't get Xrays for some weird compatibility - // reason. However, we waive here anyway in case that changes. - Cu.waiveXrays(this.sandbox).chrome = this.chromeObj; - - let localApis = {}; - apiManager.generateAPIs(this, localApis); - this.childManager = new ChildAPIManager(this, this.messageManager, localApis, { - envType: "content_parent", - url, + Schemas.inject(chromeObj, this.childManager); + return chromeObj; }); - Schemas.inject(this.chromeObj, this.childManager); + Schemas.exportLazyGetter(this.sandbox, "browser", () => this.chromeObj); + Schemas.exportLazyGetter(this.sandbox, "chrome", () => this.chromeObj); - // This is an iframe with content script API enabled. (See Bug 1214658 for rationale) + // This is an iframe with content script API enabled (bug 1214658) if (isExtensionPage) { - Cu.waiveXrays(this.contentWindow).chrome = this.chromeObj; - Cu.waiveXrays(this.contentWindow).browser = this.chromeObj; + Schemas.exportLazyGetter(this.contentWindow, + "browser", () => this.chromeObj); + Schemas.exportLazyGetter(this.contentWindow, + "chrome", () => this.chromeObj); } } @@ -398,8 +391,6 @@ class ExtensionContext extends BaseContext { close() { super.unload(); - this.childManager.close(); - if (this.contentWindow) { for (let script of this.scripts) { if (script.requiresCleanup) { @@ -419,6 +410,29 @@ class ExtensionContext extends BaseContext { } } +defineLazyGetter(ExtensionContext.prototype, "messenger", function() { + // The |sender| parameter is passed directly to the extension. + let sender = {id: this.extension.uuid, frameId: this.frameId, url: this.url}; + let filter = {extensionId: this.extension.id}; + let optionalFilter = {frameId: this.frameId}; + + return new Messenger(this, [this.messageManager], sender, filter, optionalFilter); +}); + +defineLazyGetter(ExtensionContext.prototype, "childManager", function() { + let localApis = {}; + apiManager.generateAPIs(this, localApis); + + let childManager = new ChildAPIManager(this, this.messageManager, localApis, { + envType: "content_parent", + url: this.url, + }); + + this.callOnClose(childManager); + + return childManager; +}); + // Responsible for creating ExtensionContexts and injecting content // scripts into them when new documents are created. DocumentManager = { @@ -708,7 +722,7 @@ DocumentManager = { } else { let contexts = this.contentScriptWindows.get(getInnerWindowID(window)) || new Map(); for (let context of contexts.values()) { - context.triggerScripts(this.getWindowState(window)); + context.triggerScripts(when); } } }, diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index d722e481805b..ff377cb16435 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -204,6 +204,8 @@ class BaseContext { this.messageManager = docShell.QueryInterface(Ci.nsIInterfaceRequestor) .getInterface(Ci.nsIContentFrameMessageManager); + MessageChannel.setupMessageManagers([this.messageManager]); + let onPageShow = event => { if (!event || event.target === document) { this.docShell = docShell; @@ -1439,8 +1441,6 @@ function Messenger(context, messageManagers, sender, filter, optionalFilter) { this.sender = sender; this.filter = filter; this.optionalFilter = optionalFilter; - - MessageChannel.setupMessageManagers(messageManagers); } Messenger.prototype = { @@ -2170,7 +2170,49 @@ const stylesheetMap = new DefaultMap(url => { return styleSheetService.preloadSheet(uri, styleSheetService.AGENT_SHEET); }); +/** + * Defines a lazy getter for the given property on the given object. The + * first time the property is accessed, the return value of the getter + * is defined on the current `this` object with the given property name. + * Importantly, this means that a lazy getter defined on an object + * prototype will be invoked separately for each object instance that + * it's accessed on. + * + * @param {object} object + * The prototype object on which to define the getter. + * @param {string|Symbol} prop + * The property name for which to define the getter. + * @param {function} getter + * The function to call in order to generate the final property + * value. + */ +function defineLazyGetter(object, prop, getter) { + let redefine = (obj, value) => { + Object.defineProperty(obj, prop, { + enumerable: true, + configurable: true, + writable: true, + value, + }); + return value; + }; + + Object.defineProperty(object, prop, { + enumerable: true, + configurable: true, + + get() { + return redefine(this, getter.call(this)); + }, + + set(value) { + redefine(this, value); + }, + }); +} + this.ExtensionUtils = { + defineLazyGetter, detectLanguage, extend, flushJarCache, diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_api_permissions.js b/toolkit/components/extensions/test/xpcshell/test_ext_api_permissions.js index 333cdd145290..85ac798e2848 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_api_permissions.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_api_permissions.js @@ -14,7 +14,10 @@ function getNextContext() { add_task(function* test_storage_api_without_permissions() { let extension = ExtensionTestUtils.loadExtension({ - background() {}, + background() { + // Force API initialization. + void browser.storage; + }, manifest: { permissions: [], @@ -26,6 +29,9 @@ add_task(function* test_storage_api_without_permissions() { let context = yield contextPromise; + // Force API initialization. + void context.apiObj; + ok(!("storage" in context._unwrappedAPIs), "The storage API should not be initialized"); @@ -34,7 +40,9 @@ add_task(function* test_storage_api_without_permissions() { add_task(function* test_storage_api_with_permissions() { let extension = ExtensionTestUtils.loadExtension({ - background() {}, + background() { + void browser.storage; + }, manifest: { permissions: ["storage"], @@ -46,6 +54,9 @@ add_task(function* test_storage_api_with_permissions() { let context = yield contextPromise; + // Force API initialization. + void context.apiObj; + equal(typeof context._unwrappedAPIs.storage, "object", "The storage API should be initialized");