From 602a0147b19ad2da230c78880609d3b5228e43b5 Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Mon, 15 Aug 2016 21:36:34 -0700 Subject: [PATCH] Backed out 3 changesets (bug 1246034) for Win e10s timeouts in browser_ext_commands_execute_browser_action.js Backed out changeset f1f24546c26c (bug 1246034) Backed out changeset 2c396099a21d (bug 1246034) Backed out changeset 8cce25ece209 (bug 1246034) --- browser/components/extensions/.eslintrc | 3 - .../extensions/ext-browserAction.js | 46 +------ browser/components/extensions/ext-commands.js | 3 - browser/components/extensions/ext-utils.js | 16 --- .../extensions/test/browser/browser.ini | 1 - .../browser_ext_browserAction_popup.js | 74 +++--------- ...ser_ext_commands_execute_browser_action.js | 114 ------------------ .../components/extensions/ExtensionUtils.jsm | 39 +++--- 8 files changed, 46 insertions(+), 250 deletions(-) delete mode 100644 browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js diff --git a/browser/components/extensions/.eslintrc b/browser/components/extensions/.eslintrc index 8813510adf75..b446fe45d7f3 100644 --- a/browser/components/extensions/.eslintrc +++ b/browser/components/extensions/.eslintrc @@ -3,7 +3,6 @@ "globals": { "AllWindowEvents": true, - "browserActionFor": true, "currentWindow": true, "EventEmitter": true, "IconDetails": true, @@ -17,5 +16,3 @@ "WindowManager": true, }, } - - diff --git a/browser/components/extensions/ext-browserAction.js b/browser/components/extensions/ext-browserAction.js index f0f477984372..94406faefe71 100644 --- a/browser/components/extensions/ext-browserAction.js +++ b/browser/components/extensions/ext-browserAction.js @@ -10,9 +10,8 @@ XPCOMUtils.defineLazyGetter(this, "colorUtils", () => { }); Cu.import("resource://devtools/shared/event-emitter.js"); -Cu.import("resource://gre/modules/ExtensionUtils.jsm"); -Cu.import("resource://gre/modules/Task.jsm"); +Cu.import("resource://gre/modules/ExtensionUtils.jsm"); var { EventManager, IconDetails, @@ -98,9 +97,10 @@ BrowserAction.prototype = { let popupURL = this.getProperty(tab, "popup"); this.tabManager.addActiveTabPermission(tab); - // Popups are shown only if a popup URL is defined; otherwise - // a "click" event is dispatched. This is done for compatibility with the - // Google Chrome onClicked extension API. + // If the widget has a popup URL defined, we open a popup, but do not + // dispatch a click event to the extension. + // If it has no popup URL defined, we dispatch a click event, but do not + // open a popup. if (popupURL) { try { new ViewPopup(this.extension, event.target, popupURL, this.browserStyle); @@ -123,42 +123,6 @@ BrowserAction.prototype = { this.widget = widget; }, - /** - * Triggers this browser action for the given window, with the same effects as - * if it were clicked by a user. - * - * This has no effect if the browser action is disabled for, or not - * present in, the given window. - */ - triggerAction: Task.async(function* (window) { - let popup = ViewPopup.for(this.extension, window); - if (popup) { - popup.closePopup(); - return; - } - - let widget = this.widget.forWindow(window); - let tab = window.gBrowser.selectedTab; - - if (!widget || !this.getProperty(tab, "enabled")) { - return; - } - - // Popups are shown only if a popup URL is defined; otherwise - // a "click" event is dispatched. This is done for compatibility with the - // Google Chrome onClicked extension API. - if (this.getProperty(tab, "popup")) { - if (this.widget.areaType == CustomizableUI.TYPE_MENU_PANEL) { - yield window.PanelUI.show(); - } - - let event = new window.CustomEvent("command", {bubbles: true, cancelable: true}); - widget.node.dispatchEvent(event); - } else { - this.emit("click"); - } - }), - // Update the toolbar button |node| with the tab context data // in |tabData|. updateButton(node, tabData) { diff --git a/browser/components/extensions/ext-commands.js b/browser/components/extensions/ext-commands.js index e756b682abe6..3fca3dba8e3a 100644 --- a/browser/components/extensions/ext-commands.js +++ b/browser/components/extensions/ext-commands.js @@ -128,9 +128,6 @@ CommandList.prototype = { if (name == "_execute_page_action") { let win = event.target.ownerDocument.defaultView; pageActionFor(this.extension).triggerAction(win); - } else if (name == "_execute_browser_action") { - let win = event.target.ownerDocument.defaultView; - browserActionFor(this.extension).triggerAction(win); } else { TabManager.for(this.extension) .addActiveTabPermission(TabManager.activeTab); diff --git a/browser/components/extensions/ext-utils.js b/browser/components/extensions/ext-utils.js index fc6cc1ec31b9..99eb20ac0e71 100644 --- a/browser/components/extensions/ext-utils.js +++ b/browser/components/extensions/ext-utils.js @@ -26,7 +26,6 @@ const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"; const RESIZE_TIMEOUT = 100; var { - DefaultWeakMap, EventManager, } = ExtensionUtils; @@ -112,12 +111,6 @@ class BasePopup { this.browser = null; this.browserReady = this.createBrowser(viewNode, popupURI); - - BasePopup.instances.get(this.window).set(extension, this); - } - - static for(extension, window) { - return BasePopup.instances.get(window).get(extension); } destroy() { @@ -134,8 +127,6 @@ class BasePopup { this.panel.style.setProperty("--panel-arrowcontent-background", ""); this.panel.style.setProperty("--panel-arrow-image-vertical", ""); - BasePopup.instances.get(this.window).delete(this.extension); - this.browser = null; this.viewNode = null; }); @@ -388,13 +379,6 @@ class BasePopup { } } -/** - * A map of active popups for a given browser window. - * - * WeakMap[window -> WeakMap[Extension -> BasePopup]] - */ -BasePopup.instances = new DefaultWeakMap(() => new WeakMap()); - global.PanelPopup = class PanelPopup extends BasePopup { constructor(extension, imageNode, popupURL, browserStyle) { let document = imageNode.ownerDocument; diff --git a/browser/components/extensions/test/browser/browser.ini b/browser/components/extensions/test/browser/browser.ini index 0e056a4ca5b5..9ffb3b318f08 100644 --- a/browser/components/extensions/test/browser/browser.ini +++ b/browser/components/extensions/test/browser/browser.ini @@ -24,7 +24,6 @@ support-files = [browser_ext_browserAction_popup.js] [browser_ext_browserAction_popup_resize.js] [browser_ext_browserAction_simple.js] -[browser_ext_commands_execute_browser_action.js] [browser_ext_commands_execute_page_action.js] [browser_ext_commands_getAll.js] [browser_ext_commands_onCommand.js] 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 8a321066dc37..bde7c53e4ac2 100644 --- a/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js +++ b/browser/components/extensions/test/browser/browser_ext_browserAction_popup.js @@ -2,13 +2,6 @@ /* vim: set sts=2 sw=2 et tw=80: */ "use strict"; -function getBrowserAction(extension) { - let {GlobalManager, browserActionFor} = Cu.import("resource://gre/modules/Extension.jsm", {}); - - let ext = GlobalManager.extensionMap.get(extension.id); - return browserActionFor(ext); -} - function* testInArea(area) { let scriptPage = url => `${url}`; @@ -32,7 +25,7 @@ function* testInArea(area) { browser.runtime.sendMessage("from-popup-a"); }; browser.runtime.onMessage.addListener(msg => { - if (msg == "close-popup-using-window.close") { + if (msg == "close-popup") { window.close(); } }); @@ -49,67 +42,42 @@ function* testInArea(area) { let sendClick; let tests = [ () => { - browser.test.log(`Click browser action, expect popup "a".`); sendClick({expectEvent: false, expectPopup: "a"}); }, () => { - browser.test.log(`Click browser action again, expect popup "a".`); sendClick({expectEvent: false, expectPopup: "a"}); }, () => { - browser.test.log(`Call triggerAction, expect popup "a" again. Leave popup open.`); - sendClick({expectEvent: false, expectPopup: "a", leaveOpen: true}, "trigger-action"); - }, - () => { - browser.test.log(`Call triggerAction again. Expect remaining popup closed.`); - sendClick({expectEvent: false, expectPopup: null}, "trigger-action"); - browser.test.sendMessage("next-test", {waitUntilClosed: true}); - }, - () => { - browser.test.log(`Call triggerAction again. Expect popup "a" again.`); - sendClick({expectEvent: false, expectPopup: "a"}, "trigger-action"); - }, - () => { - browser.test.log(`Change popup URL. Click browser action. Expect popup "b".`); browser.browserAction.setPopup({popup: "popup-b.html"}); sendClick({expectEvent: false, expectPopup: "b"}); }, () => { - browser.test.log(`Click browser action again. Expect popup "b" again.`); sendClick({expectEvent: false, expectPopup: "b"}); }, () => { - browser.test.log(`Clear popup URL. Click browser action. Expect click event.`); browser.browserAction.setPopup({popup: ""}); sendClick({expectEvent: true, expectPopup: null}); }, () => { - browser.test.log(`Click browser action again. Expect another click event.`); sendClick({expectEvent: true, expectPopup: null}); }, () => { - browser.test.log(`Call triggerAction. Expect click event.`); - sendClick({expectEvent: true, expectPopup: null}, "trigger-action"); - }, - () => { - browser.test.log(`Change popup URL. Click browser action. Expect popup "a", and leave open.`); browser.browserAction.setPopup({popup: "/popup-a.html"}); - sendClick({expectEvent: false, expectPopup: "a", leaveOpen: true}); + sendClick({expectEvent: false, expectPopup: "a", runNextTest: true}); }, () => { - browser.test.log(`Call window.close(). Expect popup closed.`); - browser.test.sendMessage("next-test", {closePopupUsingWindow: true}); + browser.test.sendMessage("next-test", {expectClosed: true}); }, ]; let expect = {}; - sendClick = ({expectEvent, expectPopup, runNextTest, waitUntilClosed, leaveOpen}, message = "send-click") => { - expect = {event: expectEvent, popup: expectPopup, runNextTest, waitUntilClosed, leaveOpen}; - browser.test.sendMessage(message); + sendClick = ({expectEvent, expectPopup, runNextTest}) => { + expect = {event: expectEvent, popup: expectPopup, runNextTest}; + browser.test.sendMessage("send-click"); }; browser.runtime.onMessage.addListener(msg => { - if (msg == "close-popup-using-window.close") { + if (msg == "close-popup") { return; } else if (expect.popup) { browser.test.assertEq(msg, `from-popup-${expect.popup}`, @@ -119,7 +87,12 @@ function* testInArea(area) { } expect.popup = null; - browser.test.sendMessage("next-test", expect); + if (expect.runNextTest) { + expect.runNextTest = false; + tests.shift()(); + } else { + browser.test.sendMessage("next-test"); + } }); browser.browserAction.onClicked.addListener(() => { @@ -130,12 +103,12 @@ function* testInArea(area) { } expect.event = false; - browser.test.sendMessage("next-test", expect); + browser.test.sendMessage("next-test"); }); browser.test.onMessage.addListener((msg) => { - if (msg == "close-popup-using-window.close") { - browser.runtime.sendMessage("close-popup-using-window.close"); + if (msg == "close-popup") { + browser.runtime.sendMessage("close-popup"); return; } @@ -160,31 +133,22 @@ function* testInArea(area) { clickBrowserAction(extension); }); - extension.onMessage("trigger-action", () => { - getBrowserAction(extension).triggerAction(window); - }); - let widget; extension.onMessage("next-test", Task.async(function* (expecting = {}) { if (!widget) { widget = getBrowserActionWidget(extension); CustomizableUI.addWidgetToArea(widget.id, area); } - if (expecting.waitUntilClosed) { - let panel = getBrowserActionPopup(extension); - if (panel && panel.state != "closed") { - yield promisePopupHidden(panel); - } - } else if (expecting.closePopupUsingWindow) { + if (expecting.expectClosed) { let panel = getBrowserActionPopup(extension); ok(panel, "Expect panel to exist"); yield promisePopupShown(panel); - extension.sendMessage("close-popup-using-window.close"); + extension.sendMessage("close-popup"); yield promisePopupHidden(panel); ok(true, "Panel is closed"); - } else if (!expecting.leaveOpen) { + } else { yield closeBrowserAction(extension); } 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 deleted file mode 100644 index dcb18c72617f..000000000000 --- a/browser/components/extensions/test/browser/browser_ext_commands_execute_browser_action.js +++ /dev/null @@ -1,114 +0,0 @@ -/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */ -/* vim: set sts=2 sw=2 et tw=80: */ -"use strict"; - -function* testExecuteBrowserActionWithOptions(options = {}) { - let extensionOptions = {}; - - extensionOptions.manifest = { - "commands": { - "_execute_browser_action": { - "suggested_key": { - "default": "Alt+Shift+J", - }, - }, - }, - "browser_action": { - "browser_style": true, - }, - }; - - if (options.withPopup) { - extensionOptions.manifest.browser_action.default_popup = "popup.html"; - - extensionOptions.files = { - "popup.html": ` - - - - - - - - `, - - "popup.js": function() { - browser.runtime.sendMessage("from-browser-action-popup"); - }, - }; - } - - extensionOptions.background = () => { - browser.test.onMessage.addListener((message, withPopup) => { - browser.commands.onCommand.addListener((commandName) => { - if (commandName == "_execute_browser_action") { - browser.test.fail("The onCommand listener should never fire for _execute_browser_action."); - } - }); - - browser.browserAction.onClicked.addListener(() => { - if (withPopup) { - browser.test.fail("The onClick listener should never fire if the browserAction has a popup."); - browser.test.notifyFail("execute-browser-action-on-clicked-fired"); - } else { - browser.test.notifyPass("execute-browser-action-on-clicked-fired"); - } - }); - - browser.runtime.onMessage.addListener(msg => { - if (msg == "from-browser-action-popup") { - browser.test.notifyPass("execute-browser-action-popup-opened"); - } - }); - - browser.test.sendMessage("send-keys"); - }); - }; - - let extension = ExtensionTestUtils.loadExtension(extensionOptions); - - extension.onMessage("send-keys", () => { - EventUtils.synthesizeKey("j", {altKey: true, shiftKey: true}); - }); - - yield extension.startup(); - - if (options.inArea) { - let widget = getBrowserActionWidget(extension); - CustomizableUI.addWidgetToArea(widget.id, options.inArea); - } - - extension.sendMessage("withPopup", options.withPopup); - - if (options.withPopup) { - yield extension.awaitFinish("execute-browser-action-popup-opened"); - yield closeBrowserAction(extension); - } else { - yield extension.awaitFinish("execute-browser-action-on-clicked-fired"); - } - yield extension.unload(); -} - -add_task(function* test_execute_browser_action_with_popup() { - yield testExecuteBrowserActionWithOptions({ - withPopup: true, - }); -}); - -add_task(function* test_execute_browser_action_without_popup() { - yield testExecuteBrowserActionWithOptions(); -}); - -add_task(function* test_execute_browser_action_in_hamburger_menu_with_popup() { - yield testExecuteBrowserActionWithOptions({ - withPopup: true, - inArea: CustomizableUI.AREA_PANEL, - }); -}); - -add_task(function* test_execute_browser_action_in_hamburger_menu_without_popup() { - yield testExecuteBrowserActionWithOptions({ - inArea: CustomizableUI.AREA_PANEL, - }); -}); - diff --git a/toolkit/components/extensions/ExtensionUtils.jsm b/toolkit/components/extensions/ExtensionUtils.jsm index 36f169a4dfe0..97f4e07bc210 100644 --- a/toolkit/components/extensions/ExtensionUtils.jsm +++ b/toolkit/components/extensions/ExtensionUtils.jsm @@ -128,25 +128,30 @@ function extend(obj, ...args) { return obj; } -/** - * Similar to a WeakMap, but creates a new key with the given - * constructor if one is not present. - */ -class DefaultWeakMap extends WeakMap { - constructor(defaultConstructor, init) { - super(init); - this.defaultConstructor = defaultConstructor; - } - - get(key) { - if (!this.has(key)) { - this.set(key, this.defaultConstructor()); - } - - return super.get(key); - } +// Similar to a WeakMap, but returns a particular default value for +// |get| if a key is not present. +function DefaultWeakMap(defaultValue) { + this.defaultValue = defaultValue; + this.weakmap = new WeakMap(); } +DefaultWeakMap.prototype = { + get(key) { + if (this.weakmap.has(key)) { + return this.weakmap.get(key); + } + return this.defaultValue; + }, + + set(key, value) { + if (key) { + this.weakmap.set(key, value); + } else { + this.defaultValue = value; + } + }, +}; + class SpreadArgs extends Array { constructor(args) { super();