Bug 1369577 Part 2 Propagate isHandlingUserInput for browserAction, pageAction, and menus draft
authorAndrew Swan <aswan@mozilla.com>
Thu, 15 Jun 2017 12:48:40 -0700
changeset 602422 1d7ea23c16063d699b7b99fe7f2f23214a154a0e
parent 602421 29e0a5ff6c1c32a3fac12f1509ecbddccf0fc2e6
child 635600 b6a6480210077572a5ec0dd8ba892fe3f66e5d4a
push id66431
push useraswan@mozilla.com
push dateFri, 30 Jun 2017 00:12:09 +0000
bugs1369577
milestone56.0a1
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
browser/components/extensions/ext-browserAction.js
browser/components/extensions/ext-c-menus.js
browser/components/extensions/ext-menus.js
browser/components/extensions/ext-pageAction.js
browser/components/extensions/test/browser/browser-common.ini
browser/components/extensions/test/browser/browser_ext_user_events.js
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionCommon.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/ext-permissions.js
toolkit/components/extensions/ext-toolkit.js
--- 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) {