Bug 1462121 - Properly unregister pagehide/pageshow from pages in bfcache
Previously, under these circumstances, the pageshow/pagehide
handlers were not correctly unregistered:
1. Extension has a content script / style.
2. Page moves to the bfcache due to navigation.
3. Extension is unloaded.
After the last step, the context should have been released.
That did not happen, and consequently the extension context cannot be
garbage-collected. This caused
bug 1459404.
This patch fixes the issue by using an event target object
and clearing the handleEvent method when the context unloads.
MozReview-Commit-ID: KF2Vjcn5q9t
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -147,42 +147,61 @@ class BaseContext {
.getInterface(Ci.nsIContentFrameMessageManager);
if (this.incognito == null) {
this.incognito = PrivateBrowsingUtils.isContentWindowPrivate(contentWindow);
}
MessageChannel.setupMessageManagers([this.messageManager]);
- let onPageShow = event => {
- if (!event || event.target === document) {
- this.contentWindow = contentWindow;
- this.active = true;
- }
+ let isValidInnerWindow = () => {
+ return !Cu.isDeadWrapper(contentWindow) &&
+ getInnerWindowID(contentWindow) === this.innerWindowID;
};
- let onPageHide = event => {
- if (!event || event.target === document) {
- // Put this off until the next tick.
+
+ // Use an event target object (instead of a function) so that the closure
+ // can be released upon closing this context, even if the document is in the
+ // bfcache.
+ let handler = {
+ handleEvent: (event) => {
+ if (event.target !== document) {
+ return;
+ }
+ if (event.type === "pageshow") {
+ this.contentWindow = contentWindow;
+ this.active = true;
+ }
+ if (event.type === "pagehide") {
+ this.contentWindow = null;
+ this.active = false;
+ }
+ },
+ };
+
+ this.contentWindow = contentWindow;
+ this.active = true;
+ contentWindow.addEventListener("pagehide", handler, {mozSystemGroup: true});
+ contentWindow.addEventListener("pageshow", handler, {mozSystemGroup: true});
+ this.callOnClose({
+ close: () => {
+ if (isValidInnerWindow()) {
+ contentWindow.removeEventListener("pagehide", handler, {mozSystemGroup: true});
+ contentWindow.removeEventListener("pageshow", handler, {mozSystemGroup: true});
+ }
+ // Replace handler to release the original handleEvent closure.
+ handler.handleEvent = function(event) {
+ event.currentTarget.removeEventListener("pagehide", this, {mozSystemGroup: true});
+ event.currentTarget.removeEventListener("pageshow", this, {mozSystemGroup: true});
+ };
+
+ // Allow other "close" handlers to use these properties, until the next tick.
Promise.resolve().then(() => {
this.contentWindow = null;
this.active = false;
});
- }
- };
-
- onPageShow();
- contentWindow.addEventListener("pagehide", onPageHide, {mozSystemGroup: true});
- contentWindow.addEventListener("pageshow", onPageShow, {mozSystemGroup: true});
- this.callOnClose({
- close: () => {
- onPageHide();
- if (this.active) {
- contentWindow.removeEventListener("pagehide", onPageHide, {mozSystemGroup: true});
- contentWindow.removeEventListener("pageshow", onPageShow, {mozSystemGroup: true});
- }
},
});
}
get cloneScope() {
throw new Error("Not implemented");
}
@@ -487,16 +506,17 @@ class BaseContext {
MessageChannel.abortResponses({
extensionId: this.extension.id,
contextId: this.contextId,
});
for (let obj of this.onClose) {
obj.close();
}
+ this.onClose.clear();
}
/**
* A simple proxy for unload(), for use with callOnClose().
*/
close() {
this.unload();
}
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_context.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_context.js
@@ -5,42 +5,45 @@
const server = createHttpServer({hosts: ["example.com", "example.org"]});
server.registerPathHandler("/dummy", (request, response) => {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Content-Type", "text/html", false);
response.write("<!DOCTYPE html><html></html>");
});
-add_task(async function test_contentscript_context() {
+function loadExtension() {
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({
+ return ExtensionTestUtils.loadExtension({
manifest: {
content_scripts: [{
"matches": ["http://example.com/dummy"],
"js": ["content_script.js"],
"run_at": "document_start",
}],
},
files: {
"content_script.js": contentScript,
},
});
+}
+add_task(async function test_contentscript_context() {
+ let extension = loadExtension();
await extension.startup();
let contentPage = await ExtensionTestUtils.loadContentPage("http://example.com/dummy");
await extension.awaitMessage("content-script-ready");
await extension.awaitMessage("content-script-show");
// Get the content script context and check that it points to the correct window.
await contentPage.spawn(extension.id, async extensionId => {
@@ -70,8 +73,59 @@ add_task(async function test_contentscri
await contentPage.spawn(null, async () => {
Assert.equal(this.context.contentWindow, this.content, "Context's contentWindow property is correct");
});
await contentPage.close();
await extension.awaitMessage("content-script-hide");
await extension.unload();
});
+
+add_task(async function test_contentscript_context_unload_while_in_bfcache() {
+ let contentPage = await ExtensionTestUtils.loadContentPage("http://example.com/dummy");
+ let extension = loadExtension();
+ await extension.startup();
+ await extension.awaitMessage("content-script-ready");
+
+ // Get the content script context and check that it points to the correct window.
+ await contentPage.spawn(extension.id, async extensionId => {
+ let {DocumentManager} = ChromeUtils.import("resource://gre/modules/ExtensionContent.jsm", {});
+ this.context = DocumentManager.getContext(extensionId, this.content);
+
+ Assert.equal(this.context.contentWindow, this.content, "Context's contentWindow property is correct");
+
+ this.contextUnloadedPromise = new Promise(resolve => {
+ this.context.callOnClose({close: resolve});
+ });
+ this.pageshownPromise = new Promise(resolve => {
+ this.content.addEventListener("pageshow", () => {
+ // Yield to the event loop once more to ensure that all pageshow event
+ // handlers have been dispatched before fulfilling the promise.
+ let {setTimeout} = ChromeUtils.import("resource://gre/modules/Timer.jsm", {});
+ setTimeout(resolve, 0);
+ }, {once: true, mozSystemGroup: true});
+ });
+
+ // Navigate so that the content page is hidden in the bfcache.
+ this.content.location = "http://example.org/dummy";
+ });
+
+ await extension.awaitMessage("content-script-hide");
+
+ await extension.unload();
+ await contentPage.spawn(null, async () => {
+ await this.contextUnloadedPromise;
+ Assert.equal(this.context.unloaded, true, "Context has been unloaded");
+ });
+
+ await contentPage.spawn(null, async () => {
+ Assert.equal(this.context.contentWindow, null, "Context's contentWindow property is null");
+
+ // Navigate back so the content page is resurrected from the bfcache.
+ this.content.history.back();
+
+ await this.pageshownPromise;
+
+ Assert.equal(this.context.contentWindow, null, "Context's contentWindow property is null after restore from bfcache");
+ });
+
+ await contentPage.close();
+});