Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to. r?billm draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 04 Aug 2016 16:18:25 -0700
changeset 396965 88aa9e5eb14d22696126c26f960edbc40e923b89
parent 396948 3ef7e68c6d0e9735eb9aa5e991ba99e848bc98e5
child 527337 1c22233d57bb0da31e51c4424e830b1e63dd9651
push id25160
push usermaglione.k@gmail.com
push dateThu, 04 Aug 2016 23:18:40 +0000
reviewersbillm
bugs1292369
milestone51.0a1
Bug 1292369: Null out contentWindow properties when they point to a different inner window than the context belongs to. r?billm MozReview-Commit-ID: LYQRxpU9vI8
toolkit/components/extensions/Extension.jsm
toolkit/components/extensions/ExtensionContent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_contentscript_context.html
--- a/toolkit/components/extensions/Extension.jsm
+++ b/toolkit/components/extensions/Extension.jsm
@@ -219,23 +219,26 @@ var Management = {
 // |contentWindow| is the DOM window the content runs in.
 // |uri| is the URI of the content (optional).
 // |docShell| is the docshell the content runs in (optional).
 // |incognito| is the content running in a private context (default: false).
 ExtensionContext = class extends BaseContext {
   constructor(extension, params) {
     super(extension.id);
 
-    let {type, contentWindow, uri} = params;
+    let {type, uri} = params;
     this.extension = extension;
     this.type = type;
-    this.contentWindow = contentWindow || null;
     this.uri = uri || extension.baseURI;
     this.incognito = params.incognito || false;
 
+    if (params.contentWindow) {
+      this.setContentWindow(params.contentWindow);
+    }
+
     // This is the MessageSender property passed to extension.
     // It can be augmented by the "page-open" hook.
     let sender = {id: extension.uuid};
     if (uri) {
       sender.url = uri.spec;
     }
     let delegate = {
       getSender() {},
@@ -247,21 +250,16 @@ ExtensionContext = class extends BaseCon
     let filter = {extensionId: extension.id};
     this.messenger = new Messenger(this, [Services.mm, Services.ppmm], sender, filter, delegate);
 
     if (this.externallyVisible) {
       this.extension.views.add(this);
     }
   }
 
-  get docShell() {
-    return this.contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
-               .getInterface(Ci.nsIDocShell);
-  }
-
   get cloneScope() {
     return this.contentWindow;
   }
 
   get principal() {
     return this.contentWindow.document.nodePrincipal;
   }
 
--- a/toolkit/components/extensions/ExtensionContent.jsm
+++ b/toolkit/components/extensions/ExtensionContent.jsm
@@ -46,33 +46,28 @@ Cu.import("resource://gre/modules/Extens
 var {
   runSafeSyncWithoutClone,
   BaseContext,
   LocaleData,
   Messenger,
   injectAPI,
   flushJarCache,
   detectLanguage,
+  getInnerWindowID,
   promiseDocumentReady,
   ChildAPIManager,
 } = ExtensionUtils;
 
 function isWhenBeforeOrSame(when1, when2) {
   let table = {"document_start": 0,
                "document_end": 1,
                "document_idle": 2};
   return table[when1] <= table[when2];
 }
 
-function getInnerWindowID(window) {
-  return window.QueryInterface(Ci.nsIInterfaceRequestor)
-    .getInterface(Ci.nsIDOMWindowUtils)
-    .currentInnerWindowID;
-}
-
 // This is the fairly simple API that we inject into content
 // scripts.
 var api = context => {
   return {
     runtime: {
       connect: function(extensionId, connectInfo) {
         if (!connectInfo) {
           connectInfo = extensionId;
@@ -318,21 +313,18 @@ class ExtensionContext extends BaseConte
   constructor(extensionId, contentWindow, contextOptions = {}) {
     super(extensionId);
 
     let {isExtensionPage} = contextOptions;
 
     this.isExtensionPage = isExtensionPage;
     this.extension = ExtensionManager.get(extensionId);
     this.extensionId = extensionId;
-    this.contentWindow = contentWindow;
-    this.windowId = getInnerWindowID(contentWindow);
 
-    contentWindow.addEventListener("pageshow", this, true);
-    contentWindow.addEventListener("pagehide", this, true);
+    this.setContentWindow(contentWindow);
 
     let frameId = WebNavigationFrames.getFrameId(contentWindow);
     this.frameId = frameId;
 
     this.scripts = [];
 
     let mm = getWindowMessageManager(contentWindow);
     this.messageManager = mm;
@@ -370,17 +362,17 @@ class ExtensionContext extends BaseConte
         wantXrays: false,
         isWebExtensionContentScript: true,
       });
     } else {
       // This metadata is required by the Developer Tools, in order for
       // the content script to be associated with both the extension and
       // the tab holding the content page.
       let metadata = {
-        "inner-window-id": this.windowId,
+        "inner-window-id": this.innerWindowID,
         addonId: attrs.addonId,
       };
 
       this.sandbox = Cu.Sandbox(prin, {
         metadata,
         sandboxPrototype: contentWindow,
         wantXrays: true,
         isWebExtensionContentScript: true,
@@ -436,24 +428,16 @@ class ExtensionContext extends BaseConte
 
     // This is an iframe with content script API enabled. (See Bug 1214658 for rationale)
     if (isExtensionPage) {
       Cu.waiveXrays(this.contentWindow).chrome = this.chromeObj;
       Cu.waiveXrays(this.contentWindow).browser = this.chromeObj;
     }
   }
 
-  handleEvent(event) {
-    if (event.type == "pageshow") {
-      this.active = true;
-    } else if (event.type == "pagehide") {
-      this.active = false;
-    }
-  }
-
   get cloneScope() {
     return this.sandbox;
   }
 
   execute(script, shouldRun) {
     script.tryInject(this.contentWindow, this.sandbox, shouldRun);
   }
 
@@ -476,35 +460,31 @@ class ExtensionContext extends BaseConte
       // Don't bother saving scripts after document_idle.
       this.scripts = this.scripts.filter(script => script.requiresCleanup);
     }
   }
 
   close() {
     super.unload();
 
-    if (this.windowId === getInnerWindowID(this.contentWindow)) {
-      this.contentWindow.removeEventListener("pageshow", this, true);
-      this.contentWindow.removeEventListener("pagehide", this, true);
-    }
-
-    for (let script of this.scripts) {
-      if (script.requiresCleanup) {
-        script.cleanup(this.contentWindow);
-      }
-    }
-
     this.childManager.close();
 
-    // Overwrite the content script APIs with an empty object if the APIs objects are still
-    // defined in the content window (See Bug 1214658 for rationale).
-    if (this.isExtensionPage && !Cu.isDeadWrapper(this.contentWindow) &&
-        Cu.waiveXrays(this.contentWindow).browser === this.chromeObj) {
-      Cu.createObjectIn(this.contentWindow, {defineAs: "browser"});
-      Cu.createObjectIn(this.contentWindow, {defineAs: "chrome"});
+    if (this.contentWindow) {
+      for (let script of this.scripts) {
+        if (script.requiresCleanup) {
+          script.cleanup(this.contentWindow);
+        }
+      }
+
+      // Overwrite the content script APIs with an empty object if the APIs objects are still
+      // defined in the content window (bug 1214658).
+      if (this.isExtensionPage) {
+        Cu.createObjectIn(this.contentWindow, {defineAs: "browser"});
+        Cu.createObjectIn(this.contentWindow, {defineAs: "chrome"});
+      }
     }
     Cu.nukeSandbox(this.sandbox);
     this.sandbox = null;
   }
 }
 
 // Responsible for creating ExtensionContexts and injecting content
 // scripts into them when new documents are created.
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -85,16 +85,22 @@ function runSafe(context, f, ...args) {
   }
   if (context.unloaded) {
     dump(`runSafe failure: context is already unloaded ${filterStack(new Error())}\n`);
     return undefined;
   }
   return runSafeWithoutClone(f, ...args);
 }
 
+function getInnerWindowID(window) {
+  return window.QueryInterface(Ci.nsIInterfaceRequestor)
+    .getInterface(Ci.nsIDOMWindowUtils)
+    .currentInnerWindowID;
+}
+
 // Return true if the given value is an instance of the given
 // native type.
 function instanceOf(value, type) {
   return {}.toString.call(value) == `[object ${type}]`;
 }
 
 // Extend the object |obj| with the property descriptors of each object in
 // |args|.
@@ -149,16 +155,59 @@ class BaseContext {
     this.onClose = new Set();
     this.checkedLastError = false;
     this._lastError = null;
     this.contextId = ++gContextId;
     this.unloaded = false;
     this.extensionId = extensionId;
     this.jsonSandbox = null;
     this.active = true;
+
+    this.docShell = null;
+    this.contentWindow = null;
+    this.innerWindowID = null;
+  }
+
+  setContentWindow(contentWindow) {
+    let {document} = contentWindow;
+    let docShell = contentWindow.QueryInterface(Ci.nsIInterfaceRequestor)
+                                .getInterface(Ci.nsIDocShell);
+
+    this.innerWindowID = getInnerWindowID(contentWindow);
+
+    let onPageShow = event => {
+      if (!event || event.target === document) {
+        this.docShell = docShell;
+        this.contentWindow = contentWindow;
+        this.active = true;
+      }
+    };
+    let onPageHide = event => {
+      if (!event || event.target === document) {
+        // Put this off until the next tick.
+        Promise.resolve().then(() => {
+          this.docShell = null;
+          this.contentWindow = null;
+          this.active = false;
+        });
+      }
+    };
+
+    onPageShow();
+    contentWindow.addEventListener("pagehide", onPageHide, true);
+    contentWindow.addEventListener("pageshow", onPageShow, true);
+    this.callOnClose({
+      close: () => {
+        onPageHide();
+        if (this.active) {
+          contentWindow.removeEventListener("pagehide", onPageHide, true);
+          contentWindow.removeEventListener("pageshow", onPageShow, true);
+        }
+      },
+    });
   }
 
   get cloneScope() {
     throw new Error("Not implemented");
   }
 
   get principal() {
     throw new Error("Not implemented");
@@ -1516,16 +1565,17 @@ function normalizeTime(date) {
   return new Date((typeof date == "string" && /^\d+$/.test(date))
                         ? parseInt(date, 10) : date);
 }
 
 this.ExtensionUtils = {
   detectLanguage,
   extend,
   flushJarCache,
+  getInnerWindowID,
   ignoreEvent,
   injectAPI,
   instanceOf,
   normalizeTime,
   promiseDocumentLoaded,
   promiseDocumentReady,
   promiseObserved,
   runSafe,
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -38,16 +38,17 @@ support-files =
 [test_ext_inIncognitoContext_window.html]
 skip-if = os == 'android' # Android does not currently support windows.
 [test_ext_geturl.html]
 [test_ext_background_canvas.html]
 [test_ext_content_security_policy.html]
 [test_ext_contentscript.html]
 skip-if = buildapp == 'b2g' # runat != document_idle is not supported.
 [test_ext_contentscript_api_injection.html]
+[test_ext_contentscript_context.html]
 [test_ext_contentscript_create_iframe.html]
 [test_ext_contentscript_devtools_metadata.html]
 [test_ext_contentscript_exporthelpers.html]
 [test_ext_contentscript_css.html]
 [test_ext_exclude_include_globs.html]
 [test_ext_i18n_css.html]
 [test_ext_generate.html]
 [test_ext_notifications.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_contentscript_context.html
@@ -0,0 +1,78 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <title>Test for content script contexts</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+/* eslint-disable mozilla/balanced-listeners */
+
+add_task(function* test_contentscript_context() {
+  function contentScript() {
+    browser.test.sendMessage("content-script-ready");
+
+    window.addEventListener("pagehide", () => {
+      browser.test.sendMessage("content-script-hide");
+    }, true);
+    window.addEventListener("pageshow", () => {
+      browser.test.sendMessage("content-script-show");
+    });
+  }
+
+  let extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      content_scripts: [{
+        "matches": ["http://example.com/"],
+        "js": ["content_script.js"],
+      }],
+    },
+
+    files: {
+      "content_script.js": `(${contentScript})()`,
+    },
+  });
+
+  yield extension.startup();
+
+  let win = window.open("http://example.com/");
+  yield extension.awaitMessage("content-script-ready");
+  yield extension.awaitMessage("content-script-show");
+
+  // Get the content script context and check that it points to the correct window.
+
+  let {DocumentManager} = SpecialPowers.Cu.import("resource://gre/modules/ExtensionContent.jsm", {});
+  let context = DocumentManager.getContentScriptContext(extension.id, win);
+  ok(context != null, "Got content script context");
+
+  is(SpecialPowers.unwrap(context.contentWindow), win, "Context's contentWindow property is correct");
+
+  // Navigate so that the content page is hidden in the bfcache.
+
+  win.location = "http://example.org/";
+  yield extension.awaitMessage("content-script-hide");
+
+  is(context.contentWindow, null, "Context's contentWindow property is null");
+
+  // Navigate back so the content page is resurrected from the bfcache.
+
+  SpecialPowers.wrap(win).history.back();
+  yield extension.awaitMessage("content-script-show");
+
+  is(SpecialPowers.unwrap(context.contentWindow), win, "Context's contentWindow property is correct");
+
+
+  win.close();
+  yield extension.unload();
+});
+</script>
+
+</body>
+</html>