Bug 1451748: Always remove unloaded views from views list. r?mixedpuppy
Contexts for active extension views are kept in a Set on the owning extension.
That list is meant to be kept current, with views added and removed as they're
created and unloaded. A refactoring at some point in the past, though, changed
that so that we only cleaned up parent views at extension shutdown, not at
view shutdown.
MozReview-Commit-ID: FW8KHPOD9qc
--- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
+++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js
@@ -1,19 +1,26 @@
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
+const {GlobalManager} = ChromeUtils.import("resource://gre/modules/Extension.jsm", null);
+
function getBrowserAction(extension) {
- const {GlobalManager, Management: {global: {browserActionFor}}} = ChromeUtils.import("resource://gre/modules/Extension.jsm", {});
+ const {global: {browserActionFor}} = Management;
let ext = GlobalManager.extensionMap.get(extension.id);
return browserActionFor(ext);
}
+function assertViewCount(extension, count) {
+ let ext = GlobalManager.extensionMap.get(extension.id);
+ is(ext.views.size, count, "Should have the expected number of extension views");
+}
+
let scriptPage = url => `<html><head><meta charset="utf-8"><script src="${url}"></script></head><body>${url}</body></html>`;
async function testInArea(area) {
let extension = ExtensionTestUtils.loadExtension({
manifest: {
"background": {
"page": "data/background.html",
},
@@ -199,33 +206,38 @@ async function testInArea(area) {
if (expecting.waitUntilClosed) {
await new Promise(resolve => setTimeout(resolve, 0));
let panel = getBrowserActionPopup(extension);
if (panel && panel.state != "closed") {
info("Popup is open. Waiting for close");
await promisePopupHidden(panel);
}
+
+ assertViewCount(extension, 1);
} else if (expecting.closePopupUsingWindow) {
let panel = getBrowserActionPopup(extension);
ok(panel, "Expect panel to exist");
await promisePopupShown(panel);
extension.sendMessage("close-popup-using-window.close");
await promisePopupHidden(panel);
ok(true, "Panel is closed");
+
+ assertViewCount(extension, 1);
} else if (expecting.closePopup) {
if (!getBrowserActionPopup(extension)) {
info("Waiting for panel");
await awaitExtensionPanel(extension);
}
info("Closing for panel");
await closeBrowserAction(extension);
+ assertViewCount(extension, 1);
}
if (area == getCustomizableUIPanelID() && expecting.containingPopupShouldClose) {
let {node} = getBrowserActionWidget(extension).forWindow(window);
let panel = node.closest("panel");
info(`State of panel ${panel.id} is: ${panel.state}`);
ok(!["open", "showing"].includes(panel.state),
"Panel containing the action should be closed");
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_popup.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_popup.js
@@ -1,12 +1,19 @@
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
+const {GlobalManager} = ChromeUtils.import("resource://gre/modules/Extension.jsm", null);
+
+function assertViewCount(extension, count) {
+ let ext = GlobalManager.extensionMap.get(extension.id);
+ is(ext.views.size, count, "Should have the expected number of extension views");
+}
+
add_task(async function testPageActionPopup() {
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "http://example.com/");
let scriptPage = url => `<html><head><meta charset="utf-8"><script src="${url}"></script></head></html>`;
let extension = ExtensionTestUtils.loadExtension({
manifest: {
"background": {
@@ -173,16 +180,18 @@ add_task(async function testPageActionPo
info("Panel closed");
gBrowser.selectedTab = oldTab;
} else if (panel) {
await promisePopupShown(panel);
panel.hidePopup();
}
+ assertViewCount(extension, 1);
+
if (panel) {
panel = document.getElementById(panelId);
is(panel, null, "panel successfully removed from document after hiding");
}
extension.sendMessage("next-test");
});
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -582,19 +582,23 @@ class ExtensionPageContextParent extends
}
}
onBrowserChange(browser) {
super.onBrowserChange(browser);
this.xulBrowser = browser;
}
+ unload() {
+ super.unload();
+ this.extension.views.delete(this);
+ }
+
shutdown() {
apiManager.emit("page-shutdown", this);
- this.extension.views.delete(this);
super.shutdown();
}
}
/**
* The parent side of proxied API context for devtools extension page, such as a
* devtools pages and panels running in ExtensionChild.jsm.
*/