From 5b4992f05ad7579eed7f78de632ba49f1eaf6fd0 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 30 May 2014 15:11:06 -0700 Subject: [PATCH] Bug 1015396 - Get rid of apiSandbox. r=irakli Currently, Jetpack uses two sandboxes for each content script - one for the content script itself, and another for SDK machinery. And we end up getting Xrays between the sandboxes, even though they're same-origin (generally with an nsEP) because Jetpack sandboxes use |wantXrays: true| to get same-origin Xrays. Object Xrays cause all sorts of communication problems between the sandboxes. Irakli says we don't actually need the separate sandbox anymore since we've removed the proxy layer, so let's just get rid of it. --- addon-sdk/source/lib/sdk/content/content-worker.js | 5 ++++- addon-sdk/source/lib/sdk/content/sandbox.js | 12 ++---------- addon-sdk/source/lib/sdk/deprecated/traits-worker.js | 12 ++---------- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/addon-sdk/source/lib/sdk/content/content-worker.js b/addon-sdk/source/lib/sdk/content/content-worker.js index 65722aadce2..2c0159fe90b 100644 --- a/addon-sdk/source/lib/sdk/content/content-worker.js +++ b/addon-sdk/source/lib/sdk/content/content-worker.js @@ -2,7 +2,7 @@ * 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/. */ -const ContentWorker = Object.freeze({ +Object.freeze({ // TODO: Bug 727854 Use same implementation than common JS modules, // i.e. EventEmitter module @@ -70,6 +70,7 @@ const ContentWorker = Object.freeze({ * onChromeEvent --> callback registered through pipe.on */ createPipe: function createPipe(emitToChrome) { + let ContentWorker = this; function onEvent(type, ...args) { // JSON.stringify is buggy with cross-sandbox values, // it may return "{}" on functions. Use a replacer to match them correctly. @@ -271,6 +272,7 @@ const ContentWorker = Object.freeze({ injectMessageAPI: function injectMessageAPI(exports, pipe, console) { + let ContentWorker = this; let { eventEmitter: port, emit : portEmit } = ContentWorker.createEventEmitter(pipe.emit.bind(null, "event")); pipe.on("event", portEmit); @@ -322,6 +324,7 @@ const ContentWorker = Object.freeze({ }, inject: function (exports, chromeAPI, emitToChrome, options) { + let ContentWorker = this; let { pipe, onChromeEvent, hasListenerFor } = ContentWorker.createPipe(emitToChrome); diff --git a/addon-sdk/source/lib/sdk/content/sandbox.js b/addon-sdk/source/lib/sdk/content/sandbox.js index bcfdac15a87..6b16b7e8bb0 100644 --- a/addon-sdk/source/lib/sdk/content/sandbox.js +++ b/addon-sdk/source/lib/sdk/content/sandbox.js @@ -132,11 +132,6 @@ const WorkerSandbox = Class({ principals = EXPANDED_PRINCIPALS.concat(window); } - // Instantiate trusted code in another Sandbox in order to prevent content - // script from messing with standard classes used by proxy and API code. - let apiSandbox = sandbox(principals, { wantXrays: true, sameZoneAs: window }); - apiSandbox.console = console; - // Create the sandbox and bind it to window in order for content scripts to // have access to all standard globals (window, document, ...) let content = sandbox(principals, { @@ -171,9 +166,7 @@ const WorkerSandbox = Class({ }); // Load trusted code that will inject content script API. - // We need to expose JS objects defined in same principal in order to - // avoid having any kind of wrapper. - load(apiSandbox, CONTENT_WORKER_URL); + let ContentWorker = load(content, CONTENT_WORKER_URL); // prepare a clean `self.options` let options = 'contentScriptOptions' in worker ? @@ -189,9 +182,8 @@ const WorkerSandbox = Class({ // content priviledges // https://developer.mozilla.org/en/XPConnect_wrappers#Other_security_wrappers let onEvent = onContentEvent.bind(null, this); - // `ContentWorker` is defined in CONTENT_WORKER_URL file let chromeAPI = createChromeAPI(); - let result = apiSandbox.ContentWorker.inject(content, chromeAPI, onEvent, options); + let result = Cu.waiveXrays(ContentWorker).inject(content, chromeAPI, onEvent, options); // Merge `emitToContent` and `hasListenerFor` into our private // model of the WorkerSandbox so we can communicate with content diff --git a/addon-sdk/source/lib/sdk/deprecated/traits-worker.js b/addon-sdk/source/lib/sdk/deprecated/traits-worker.js index aa61a68e9ea..8816f7220c6 100644 --- a/addon-sdk/source/lib/sdk/deprecated/traits-worker.js +++ b/addon-sdk/source/lib/sdk/deprecated/traits-worker.js @@ -145,11 +145,6 @@ const WorkerSandbox = EventEmitter.compose({ wantGlobalProperties.push("XMLHttpRequest"); } - // Instantiate trusted code in another Sandbox in order to prevent content - // script from messing with standard classes used by proxy and API code. - let apiSandbox = sandbox(principals, { wantXrays: true, sameZoneAs: window }); - apiSandbox.console = console; - // Create the sandbox and bind it to window in order for content scripts to // have access to all standard globals (window, document, ...) let content = this._sandbox = sandbox(principals, { @@ -181,9 +176,7 @@ const WorkerSandbox = EventEmitter.compose({ }); // Load trusted code that will inject content script API. - // We need to expose JS objects defined in same principal in order to - // avoid having any kind of wrapper. - load(apiSandbox, CONTENT_WORKER_URL); + let ContentWorker = load(content, CONTENT_WORKER_URL); // prepare a clean `self.options` let options = 'contentScriptOptions' in worker ? @@ -223,8 +216,7 @@ const WorkerSandbox = EventEmitter.compose({ } }; let onEvent = this._onContentEvent.bind(this); - // `ContentWorker` is defined in CONTENT_WORKER_URL file - let result = apiSandbox.ContentWorker.inject(content, chromeAPI, onEvent, options); + let result = Cu.waiveXrays(ContentWorker).inject(content, chromeAPI, onEvent, options); this._emitToContent = result.emitToContent; this._hasListenerFor = result.hasListenerFor;