Bug 1369577 Part 2 Propagate isHandlingUserInput for browserAction, pageAction, and menus
The implementations of browserAction, pageAction, and menu onClick
handlers now stash the current <browser> until we get a reply from
the extension process indicating that the handler has finished running.
We also have to take care to keep that <browser> around even if the
permissions api has to be loaded asynchronously.
MozReview-Commit-ID: BYJaiwdj40u
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -197,17 +197,17 @@ this.browserAction = class extends Exten
Cu.reportError(e);
event.preventDefault();
}
} else {
TelemetryStopwatch.cancel(POPUP_OPEN_MS_HISTOGRAM, this);
// This isn't not a hack, but it seems to provide the correct behavior
// with the fewest complications.
event.preventDefault();
- this.emit("click");
+ this.emit("click", tabbrowser.selectedBrowser);
}
},
});
this.tabContext.on("tab-select", // eslint-disable-line mozilla/balanced-listeners
(evt, tab) => { this.updateWindow(tab.ownerGlobal); });
this.widget = widget;
@@ -545,19 +545,20 @@ this.browserAction = class extends Exten
if (tabId !== null) {
return tabTracker.getTab(tabId);
}
return null;
}
return {
browserAction: {
- onClicked: new EventManager(context, "browserAction.onClicked", fire => {
- let listener = () => {
- fire.async(tabManager.convert(tabTracker.activeTab));
+ onClicked: new InputEventManager(context, "browserAction.onClicked", fire => {
+ let listener = (event, browser) => {
+ context.withPendingBrowser(browser, () =>
+ fire.sync(tabManager.convert(tabTracker.activeTab)));
};
browserAction.on("click", listener);
return () => {
browserAction.off("click", listener);
};
}).api(),
enable: function(tabId) {
--- a/browser/components/extensions/ext-c-menus.js
+++ b/browser/components/extensions/ext-c-menus.js
@@ -1,15 +1,19 @@
/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
/* vim: set sts=2 sw=2 et tw=80: */
"use strict";
// The ext-* files are imported into the same scopes.
/* import-globals-from ../../../toolkit/components/extensions/ext-c-toolkit.js */
+var {
+ withHandlingUserInput,
+} = ExtensionUtils;
+
// If id is not specified for an item we use an integer.
// This ID need only be unique within a single addon. Since all addon code that
// can use this API runs in the same process, this local variable suffices.
var gNextMenuItemID = 0;
// Map[Extension -> Map[string or id, ContextMenusClickPropHandler]]
var gPropHandlers = new Map();
@@ -155,17 +159,18 @@ this.menusInternal = class extends Exten
removeAll() {
onClickedProp.deleteAllListenersFromExtension();
return context.childManager.callParentAsyncFunction("menusInternal.removeAll", []);
},
onClicked: new EventManager(context, "menus.onClicked", fire => {
let listener = (info, tab) => {
- fire.async(info, tab);
+ withHandlingUserInput(context.contentWindow,
+ () => fire.sync(info, tab));
};
let event = context.childManager.getParentEvent("menusInternal.onClicked");
event.addListener(listener);
return () => {
event.removeListener(listener);
};
}).api(),
--- a/browser/components/extensions/ext-menus.js
+++ b/browser/components/extensions/ext-menus.js
@@ -669,17 +669,18 @@ this.menusInternal = class extends Exten
let root = gRootItems.get(extension);
if (root) {
root.remove();
}
},
onClicked: new EventManager(context, "menusInternal.onClicked", fire => {
let listener = (event, info, tab) => {
- fire.async(info, tab);
+ context.withPendingBrowser(tab.linkedBrowser,
+ () => fire.sync(info, tab));
};
extension.on("webext-menu-menuitem-click", listener);
return () => {
extension.off("webext-menu-menuitem-click", listener);
};
}).api(),
},
--- a/browser/components/extensions/ext-pageAction.js
+++ b/browser/components/extensions/ext-pageAction.js
@@ -263,19 +263,20 @@ this.pageAction = class extends Extensio
getAPI(context) {
let {extension} = context;
const {tabManager} = extension;
const pageAction = this;
return {
pageAction: {
- onClicked: new EventManager(context, "pageAction.onClicked", fire => {
+ onClicked: new InputEventManager(context, "pageAction.onClicked", fire => {
let listener = (evt, tab) => {
- fire.async(tabManager.convert(tab));
+ context.withPendingBrowser(tab.linkedBrowser, () =>
+ fire.sync(tabManager.convert(tab)));
};
pageAction.on("click", listener);
return () => {
pageAction.off("click", listener);
};
}).api(),
--- a/browser/components/extensions/test/browser/browser-common.ini
+++ b/browser/components/extensions/test/browser/browser-common.ini
@@ -137,16 +137,17 @@ skip-if = debug || asan # Bug 1354681
[browser_ext_tabs_cookieStoreId.js]
[browser_ext_tabs_update.js]
[browser_ext_tabs_zoom.js]
[browser_ext_tabs_update_url.js]
[browser_ext_themes_icons.js]
[browser_ext_themes_validation.js]
[browser_ext_url_overrides_newtab.js]
[browser_ext_url_overrides_home.js]
+[browser_ext_user_events.js]
[browser_ext_webRequest.js]
[browser_ext_webNavigation_frameId0.js]
[browser_ext_webNavigation_getFrames.js]
[browser_ext_webNavigation_onCreatedNavigationTarget.js]
[browser_ext_webNavigation_onCreatedNavigationTarget_contextmenu.js]
[browser_ext_webNavigation_onCreatedNavigationTarget_window_open.js]
[browser_ext_webNavigation_urlbar_transitions.js]
[browser_ext_windows.js]
new file mode 100644
--- /dev/null
+++ b/browser/components/extensions/test/browser/browser_ext_user_events.js
@@ -0,0 +1,70 @@
+"use strict";
+
+// Test that different types of events are all considered
+// "handling user input".
+add_task(async function testSources() {
+ let extension = ExtensionTestUtils.loadExtension({
+ async background() {
+ async function request() {
+ try {
+ let result = await browser.permissions.request({
+ permissions: ["cookies"],
+ });
+ browser.test.sendMessage("request", {success: true, result});
+ } catch (err) {
+ browser.test.sendMessage("request", {success: false, errmsg: err.message});
+ }
+ }
+
+ let tabs = await browser.tabs.query({active: true, currentWindow: true});
+ await browser.pageAction.show(tabs[0].id);
+ browser.test.sendMessage("page-action-shown");
+
+ browser.pageAction.onClicked.addListener(request);
+ browser.browserAction.onClicked.addListener(request);
+
+ browser.contextMenus.create({
+ id: "menu",
+ title: "test user events",
+ contexts: ["page"],
+ });
+ browser.contextMenus.onClicked.addListener(request);
+ },
+
+ manifest: {
+ browser_action: {default_title: "test"},
+ page_action: {default_title: "test"},
+ permissions: ["contextMenus"],
+ optional_permissions: ["cookies"],
+ },
+ });
+
+ async function check(what) {
+ let result = await extension.awaitMessage("request");
+ ok(result.success, `request() did not throw when called from ${what}`);
+ is(result.result, true, `request() succeeded when called from ${what}`);
+ }
+
+ await extension.startup();
+
+ await extension.awaitMessage("page-action-shown");
+ clickPageAction(extension);
+ await check("page action click");
+
+ clickBrowserAction(extension);
+ await check("browser action click");
+
+ let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
+ gBrowser.selectedTab = tab;
+
+ let menu = await openContextMenu("body");
+ let items = menu.getElementsByAttribute("label", "test user events");
+ is(items.length, 1, "Found context menu item");
+ EventUtils.synthesizeMouseAtCenter(items[0], {});
+ await check("context menu click");
+
+ await BrowserTestUtils.removeTab(tab);
+
+ await extension.unload();
+});
+
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -38,16 +38,17 @@ Cu.import("resource://gre/modules/Extens
const {
DefaultMap,
EventEmitter,
LimitedSet,
defineLazyGetter,
getMessageManager,
getUniqueId,
+ withHandlingUserInput,
} = ExtensionUtils;
const {
EventManager,
LocalAPIImplementation,
LocaleData,
NoCloneSpreadArgs,
SchemaAPIInterface,
@@ -754,18 +755,19 @@ class ChildAPIManager {
switch (name || messageName) {
case "API:RunListener":
let map = this.listeners.get(data.path);
let listener = map.ids.get(data.listenerId);
if (listener) {
let args = data.args.deserialize(this.context.cloneScope);
-
- return this.context.runSafeWithoutClone(listener, ...args);
+ let fire = () => this.context.runSafeWithoutClone(listener, ...args);
+ return (data.handlingUserInput) ?
+ withHandlingUserInput(this.context.contentWindow, fire) : fire();
}
if (!map.removedIds.has(data.listenerId)) {
Services.console.logStringMessage(
`Unknown listener at childId=${data.childId} path=${data.path} listenerId=${data.listenerId}\n`);
}
break;
case "API:CallResult":
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1378,16 +1378,17 @@ defineLazyGetter(LocaleData.prototype, "
// content process). |name| is for debugging. |register| is a function
// to register the listener. |register| should return an
// unregister function that will unregister the listener.
function EventManager(context, name, register) {
this.context = context;
this.name = name;
this.register = register;
this.unregister = new Map();
+ this.inputHandling = false;
}
EventManager.prototype = {
addListener(callback, ...args) {
if (this.unregister.has(callback)) {
return;
}
@@ -1467,16 +1468,17 @@ EventManager.prototype = {
this.revoke();
},
api() {
return {
addListener: (...args) => this.addListener(...args),
removeListener: (...args) => this.removeListener(...args),
hasListener: (...args) => this.hasListener(...args),
+ setUserInput: this.inputHandling,
[Schemas.REVOKE]: () => this.revoke(),
};
},
};
// Simple API for event listeners where events never fire.
function ignoreEvent(context, name) {
return {
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -317,19 +317,31 @@ class ProxyContextParent extends BaseCon
this.messageManagerProxy = new MessageManagerProxy(xulBrowser);
Object.defineProperty(this, "principal", {
value: principal, enumerable: true, configurable: true,
});
this.listenerProxies = new Map();
+ this.pendingEventBrowser = null;
+
apiManager.emit("proxy-context-load", this);
}
+ withPendingBrowser(browser, callable) {
+ let savedBrowser = this.pendingEventBrowser;
+ this.pendingEventBrowser = browser;
+ try {
+ return callable();
+ } finally {
+ this.pendingEventBrowser = savedBrowser;
+ }
+ }
+
get cloneScope() {
return this.sandbox;
}
get xulBrowser() {
return this.messageManagerProxy.eventTarget;
}
@@ -609,18 +621,18 @@ ParentAPIManager = {
childId: data.childId,
callId: data.callId,
}, result));
};
try {
let args = Cu.cloneInto(data.args, context.sandbox);
let fun = await context.apiCan.asyncFindAPIPath(data.path);
- let result = fun(...args);
-
+ let result = context.withPendingBrowser(context.pendingEventBrowser,
+ () => fun(...args));
if (data.callId) {
result = result || Promise.resolve();
result.then(result => {
result = result instanceof SpreadArgs ? [...result] : [result];
let holder = new StructuredCloneHolder(result);
@@ -642,23 +654,25 @@ ParentAPIManager = {
async addListener(data, target) {
let context = this.getContextById(data.childId);
if (context.parentMessageManager !== target.messageManager) {
throw new Error("Got message on unexpected message manager");
}
let {childId} = data;
+ let handlingUserInput = false;
function listener(...listenerArgs) {
return context.sendMessage(
context.parentMessageManager,
"API:RunListener",
{
childId,
+ handlingUserInput,
listenerId: data.listenerId,
path: data.path,
args: new StructuredCloneHolder(listenerArgs),
},
{
recipient: {childId},
});
}
@@ -673,16 +687,19 @@ ParentAPIManager = {
if (context.viewType === "background" && context.listenerPromises) {
const {listenerPromises} = context;
listenerPromises.add(promise);
let remove = () => { listenerPromises.delete(promise); };
promise.then(remove, remove);
}
let handler = await promise;
+ if (handler.setUserInput) {
+ handlingUserInput = true;
+ }
handler.addListener(listener, ...args);
},
async removeListener(data) {
let context = this.getContextById(data.childId);
let listener = context.listenerProxies.get(data.listenerId);
let handler = await context.apiCan.asyncFindAPIPath(data.path);
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -144,16 +144,25 @@ const _winUtils = new DefaultWeakMap(win
.getInterface(Ci.nsIDOMWindowUtils);
});
const getWinUtils = win => _winUtils.get(win);
function getInnerWindowID(window) {
return getWinUtils(window).currentInnerWindowID;
}
+function withHandlingUserInput(window, callable) {
+ let handle = getWinUtils(window).setHandlingUserInput(true);
+ try {
+ return callable();
+ } finally {
+ handle.destruct();
+ }
+}
+
const LISTENERS = Symbol("listeners");
const ONCE_MAP = Symbol("onceMap");
class EventEmitter {
constructor() {
this[LISTENERS] = new Map();
this[ONCE_MAP] = new WeakMap();
}
@@ -624,15 +633,16 @@ this.ExtensionUtils = {
promiseDocumentLoaded,
promiseDocumentReady,
promiseEvent,
promiseObserved,
runSafe,
runSafeSync,
runSafeSyncWithoutClone,
runSafeWithoutClone,
+ withHandlingUserInput,
DefaultMap,
DefaultWeakMap,
EventEmitter,
ExtensionError,
LimitedSet,
MessageManagerProxy,
};
--- a/toolkit/components/extensions/ext-permissions.js
+++ b/toolkit/components/extensions/ext-permissions.js
@@ -31,20 +31,21 @@ this.permissions = class extends Extensi
let optionalOrigins = context.extension.optionalOrigins;
for (let origin of origins) {
if (!optionalOrigins.subsumes(new MatchPattern(origin))) {
throw new ExtensionError(`Cannot request origin permission for ${origin} since it was not declared in optional_permissions`);
}
}
if (promptsEnabled) {
+ let browser = context.pendingEventBrowser || context.xulBrowser;
let allow = await new Promise(resolve => {
let subject = {
wrappedJSObject: {
- browser: context.xulBrowser,
+ browser,
name: context.extension.name,
icon: context.extension.iconURL,
permissions: {permissions, origins},
resolve,
},
};
Services.obs.notifyObservers(subject, "webextension-optional-permission-prompt");
});
--- a/toolkit/components/extensions/ext-toolkit.js
+++ b/toolkit/components/extensions/ext-toolkit.js
@@ -1,28 +1,34 @@
"use strict";
// These are defined on "global" which is used for the same scopes as the other
// ext-*.js files.
/* exported getCookieStoreIdForTab, getCookieStoreIdForContainer,
getContainerForCookieStoreId,
isValidCookieStoreId, isContainerCookieStoreId,
- EventManager */
+ EventManager, InputEventManager */
/* global getCookieStoreIdForTab:false, getCookieStoreIdForContainer:false,
getContainerForCookieStoreId: false,
isValidCookieStoreId:false, isContainerCookieStoreId:false,
isDefaultCookieStoreId: false, isPrivateCookieStoreId:false,
- EventManager: false */
+ EventManager: false, InputEventManager: false */
XPCOMUtils.defineLazyModuleGetter(this, "ContextualIdentityService",
"resource://gre/modules/ContextualIdentityService.jsm");
Cu.import("resource://gre/modules/ExtensionCommon.jsm");
global.EventManager = ExtensionCommon.EventManager;
+global.InputEventManager = class extends EventManager {
+ constructor(...args) {
+ super(...args);
+ this.inputHandling = true;
+ }
+};
/* globals DEFAULT_STORE, PRIVATE_STORE, CONTAINER_STORE */
global.DEFAULT_STORE = "firefox-default";
global.PRIVATE_STORE = "firefox-private";
global.CONTAINER_STORE = "firefox-container-";
global.getCookieStoreIdForTab = function(data, tab) {