From 73f6cab40353efdad1b96db996c53959b0d252a5 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Thu, 17 Nov 2016 11:28:40 -0800 Subject: [PATCH] Bug 1317101 - Part 7c: Run extension popups in a remote browser. r=aswan MozReview-Commit-ID: CATeESBwj1J --HG-- extra : rebase_source : c83dbcabbafbe2884746d282de78d23f5e0edff6 extra : source : b7892d3fb0ca5268a252377ecbb44dfb1d289500 --- browser/components/extensions/ext-utils.js | 95 +++++++++++++------ .../browser_ext_browserAction_popup.js | 14 ++- ...ser_ext_commands_execute_browser_action.js | 4 + 3 files changed, 83 insertions(+), 30 deletions(-) diff --git a/browser/components/extensions/ext-utils.js b/browser/components/extensions/ext-utils.js index 2bced95a320d..f546709d0a6d 100644 --- a/browser/components/extensions/ext-utils.js +++ b/browser/components/extensions/ext-utils.js @@ -123,11 +123,13 @@ class BasePopup { this.destroyed = true; this.browserLoadedDeferred.reject(new Error("Popup destroyed")); return this.browserReady.then(() => { - this.destroyBrowser(this.browser); + this.destroyBrowser(this.browser, true); this.browser.remove(); - this.viewNode.removeEventListener(this.DESTROY_EVENT, this); - this.viewNode.style.maxHeight = ""; + if (this.viewNode) { + this.viewNode.removeEventListener(this.DESTROY_EVENT, this); + this.viewNode.style.maxHeight = ""; + } if (this.panel) { this.panel.style.removeProperty("--arrowpanel-background"); @@ -141,16 +143,19 @@ class BasePopup { }); } - destroyBrowser(browser) { + destroyBrowser(browser, finalize = false) { let mm = browser.messageManager; // If the browser has already been removed from the document, because the - // popup was closed externally, there will be no message manager here. + // popup was closed externally, there will be no message manager here, so + // just replace our receiveMessage method with a stub. if (mm) { mm.removeMessageListener("DOMTitleChanged", this); mm.removeMessageListener("Extension:BrowserBackgroundChanged", this); mm.removeMessageListener("Extension:BrowserContentLoaded", this); mm.removeMessageListener("Extension:BrowserResized", this); mm.removeMessageListener("Extension:DOMWindowClose", this); + } else if (finalize) { + this.receiveMessage = () => {}; } } @@ -213,53 +218,73 @@ class BasePopup { handleEvent(event) { switch (event.type) { case this.DESTROY_EVENT: - this.destroy(); + if (!this.destroyed) { + this.destroy(); + } break; } } createBrowser(viewNode, popupURL = null) { let document = viewNode.ownerDocument; - this.browser = document.createElementNS(XUL_NS, "browser"); - this.browser.setAttribute("type", "content"); - this.browser.setAttribute("disableglobalhistory", "true"); - this.browser.setAttribute("transparent", "true"); - this.browser.setAttribute("class", "webextension-popup-browser"); - this.browser.setAttribute("webextension-view-type", "popup"); - this.browser.setAttribute("tooltip", "aHTMLTooltip"); + let browser = document.createElementNS(XUL_NS, "browser"); + browser.setAttribute("type", "content"); + browser.setAttribute("disableglobalhistory", "true"); + browser.setAttribute("transparent", "true"); + browser.setAttribute("class", "webextension-popup-browser"); + browser.setAttribute("webextension-view-type", "popup"); + browser.setAttribute("tooltip", "aHTMLTooltip"); + + if (this.extension.remote) { + browser.setAttribute("remote", "true"); + } // We only need flex sizing for the sake of the slide-in sub-views of the // main menu panel, so that the browser occupies the full width of the view, // and also takes up any extra height that's available to it. - this.browser.setAttribute("flex", "1"); + browser.setAttribute("flex", "1"); // Note: When using noautohide panels, the popup manager will add width and // height attributes to the panel, breaking our resize code, if the browser // starts out smaller than 30px by 10px. This isn't an issue now, but it // will be if and when we popup debugging. - viewNode.appendChild(this.browser); + this.browser = browser; - extensions.emit("extension-browser-inserted", this.browser); + let readyPromise; + if (this.extension.remote) { + readyPromise = promiseEvent(browser, "XULFrameLoaderCreated"); + } else { + readyPromise = promiseEvent(browser, "load"); + } - let initBrowser = browser => { + viewNode.appendChild(browser); + + extensions.emit("extension-browser-inserted", browser); + + let setupBrowser = browser => { let mm = browser.messageManager; mm.addMessageListener("DOMTitleChanged", this); mm.addMessageListener("Extension:BrowserBackgroundChanged", this); mm.addMessageListener("Extension:BrowserContentLoaded", this); mm.addMessageListener("Extension:BrowserResized", this); mm.addMessageListener("Extension:DOMWindowClose", this, true); + return browser; }; if (!popupURL) { - initBrowser(this.browser); - return this.browser; + // For remote browsers, we can't do any setup until the frame loader is + // created. Non-remote browsers get a message manager immediately, so + // there's no need to wait for the load event. + if (this.extension.remote) { + return readyPromise.then(() => setupBrowser(browser)); + } + return setupBrowser(browser); } - return promiseEvent(this.browser, "load").then(() => { - initBrowser(this.browser); - - let mm = this.browser.messageManager; + return readyPromise.then(() => { + setupBrowser(browser); + let mm = browser.messageManager; mm.loadFrameScript( "chrome://extensions/content/ext-browser-content.js", false); @@ -272,7 +297,7 @@ class BasePopup { stylesheets: this.STYLESHEETS, }); - this.browser.loadURI(popupURL); + browser.loadURI(popupURL); }); } @@ -363,12 +388,13 @@ class PanelPopup extends BasePopup { destroy() { super.destroy(); this.viewNode.remove(); + this.viewNode = null; } closePopup() { promisePopupShown(this.viewNode).then(() => { - // Make sure we're not already destroyed. - if (this.viewNode) { + // Make sure we're not already destroyed, or removed from the DOM. + if (this.viewNode && this.viewNode.hidePopup) { this.viewNode.hidePopup(); } }); @@ -391,6 +417,7 @@ class ViewPopup extends BasePopup { this.ignoreResizes = true; this.attached = false; + this.shown = false; this.tempPanel = panel; this.browser.classList.add("webextension-preload-browser"); @@ -432,11 +459,13 @@ class ViewPopup extends BasePopup { } if (this.destroyed) { + CustomizableUI.hidePanelForNode(viewNode); return false; } this.attached = true; + // Store the initial height of the view, so that we never resize menu panel // sub-views smaller than the initial height of the menu. this.viewHeight = this.viewNode.boxObject.height; @@ -457,7 +486,7 @@ class ViewPopup extends BasePopup { // Create a new browser in the real popup. let browser = this.browser; - this.createBrowser(this.viewNode); + yield this.createBrowser(this.viewNode); this.browser.swapDocShells(browser); this.destroyBrowser(browser); @@ -470,6 +499,14 @@ class ViewPopup extends BasePopup { this.tempPanel.remove(); this.tempPanel = null; + this.shown = true; + + if (this.destroyed) { + this.closePopup(); + this.destroy(); + return false; + } + let event = new this.window.CustomEvent("WebExtPopupLoaded", { bubbles: true, detail: {extension: this.extension}, @@ -494,8 +531,10 @@ class ViewPopup extends BasePopup { } closePopup() { - if (this.attached) { + if (this.shown) { CustomizableUI.hidePanelForNode(this.viewNode); + } else if (this.attached) { + this.destroyed = true; } else { this.destroy(); } diff --git a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js index 9f04b3c115d3..070065c0949d 100644 --- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js +++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js @@ -82,7 +82,7 @@ function* testInArea(area) { () => { browser.test.log(`Set popup to "c" and click browser action. Expect popup "c".`); browser.browserAction.setPopup({popup: "popup-c.html"}); - sendClick({expectEvent: false, expectPopup: "c", closePopup: false}); + sendClick({expectEvent: false, expectPopup: "c", waitUntilClosed: true}); }, () => { browser.test.log(`Set popup to "b" and click browser action. Expect popup "b".`); @@ -120,7 +120,7 @@ function* testInArea(area) { let expect = {}; sendClick = ({expectEvent, expectPopup, runNextTest, waitUntilClosed, closePopup}, message = "send-click") => { if (closePopup == undefined) { - closePopup = true; + closePopup = !expectEvent; } expect = {event: expectEvent, popup: expectPopup, runNextTest, waitUntilClosed, closePopup}; @@ -190,8 +190,11 @@ function* testInArea(area) { CustomizableUI.addWidgetToArea(widget.id, area); } if (expecting.waitUntilClosed) { + yield new Promise(resolve => setTimeout(resolve, 0)); + let panel = getBrowserActionPopup(extension); if (panel && panel.state != "closed") { + info("Popup is open. Waiting for close"); yield promisePopupHidden(panel); } } else if (expecting.closePopupUsingWindow) { @@ -204,9 +207,16 @@ function* testInArea(area) { yield promisePopupHidden(panel); ok(true, "Panel is closed"); } else if (expecting.closePopup) { + if (!getBrowserActionPopup(extension)) { + info("Waiting for panel"); + yield awaitExtensionPanel(extension); + } + + info("Closing for panel"); yield closeBrowserAction(extension); } + info("Starting next test"); extension.sendMessage("next-test"); })); diff --git a/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js b/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js index f97a735d4317..04f8751de61a 100644 --- a/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js +++ b/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js @@ -82,6 +82,10 @@ function* testExecuteBrowserActionWithOptions(options = {}) { if (options.withPopup) { yield extension.awaitFinish("execute-browser-action-popup-opened"); + + if (!getBrowserActionPopup(extension)) { + yield awaitExtensionPanel(extension); + } yield closeBrowserAction(extension); } else { yield extension.awaitFinish("execute-browser-action-on-clicked-fired");