Bug 1369577 Part 2 Propagate isHandlingUserInput for browserAction, pageAction, and menus r=kmag

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

--HG--
extra : rebase_source : 3d9cba03d2853ef8d71b6c3e3a1fd0aba400b39c

browser/components/extensions/ext-browserAction.js

@@ -205,7 +205,7 @@ this.browserAction = class extends ExtensionAPI {
           // 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);
         }
       },
     });
@@ -566,9 +566,10 @@ this.browserAction = class extends ExtensionAPI {
 
     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 () => {

browser/components/extensions/ext-c-menus.js

@@ -5,6 +5,10 @@
 // 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.
@@ -160,7 +164,8 @@ this.menusInternal = class extends ExtensionAPI {
 
         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");

browser/components/extensions/ext-menus.js

@@ -675,7 +675,8 @@ this.menusInternal = class extends ExtensionAPI {
 
         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);

browser/components/extensions/ext-pageAction.js

@@ -269,9 +269,10 @@ this.pageAction = class extends ExtensionAPI {
 
     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);

browser/components/extensions/test/browser/browser-common.ini

@@ -143,6 +143,7 @@ skip-if = debug || asan # Bug 1354681
 [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/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();
+});
+

toolkit/components/extensions/ExtensionChild.jsm

@@ -43,6 +43,7 @@ const {
   defineLazyGetter,
   getMessageManager,
   getUniqueId,
+  withHandlingUserInput,
 } = ExtensionUtils;
 
 const {
@@ -759,8 +760,9 @@ class ChildAPIManager {
 
         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(

toolkit/components/extensions/ExtensionCommon.jsm

@@ -1383,6 +1383,7 @@ function EventManager(context, name, register) {
   this.name = name;
   this.register = register;
   this.unregister = new Map();
+  this.inputHandling = false;
 }
 
 EventManager.prototype = {
@@ -1472,6 +1473,7 @@ EventManager.prototype = {
       addListener: (...args) => this.addListener(...args),
       removeListener: (...args) => this.removeListener(...args),
       hasListener: (...args) => this.hasListener(...args),
+      setUserInput: this.inputHandling,
       [Schemas.REVOKE]: () => this.revoke(),
     };
   },

toolkit/components/extensions/ExtensionParent.jsm

@@ -322,9 +322,21 @@ class ProxyContextParent extends BaseContext {
 
     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;
   }
@@ -614,8 +626,8 @@ ParentAPIManager = {
     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();
 
@@ -647,6 +659,7 @@ ParentAPIManager = {
     }
 
     let {childId} = data;
+    let handlingUserInput = false;
 
     function listener(...listenerArgs) {
       return context.sendMessage(
@@ -654,6 +667,7 @@ ParentAPIManager = {
         "API:RunListener",
         {
           childId,
+          handlingUserInput,
           listenerId: data.listenerId,
           path: data.path,
           args: new StructuredCloneHolder(listenerArgs),
@@ -678,6 +692,9 @@ ParentAPIManager = {
     }
 
     let handler = await promise;
+    if (handler.setUserInput) {
+      handlingUserInput = true;
+    }
     handler.addListener(listener, ...args);
   },
 

toolkit/components/extensions/ExtensionUtils.jsm

@@ -149,6 +149,15 @@ 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");
 
@@ -629,6 +638,7 @@ this.ExtensionUtils = {
   runSafeSync,
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
+  withHandlingUserInput,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,

toolkit/components/extensions/ext-permissions.js

@@ -36,10 +36,11 @@ this.permissions = class extends ExtensionAPI {
           }
 
           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},

toolkit/components/extensions/ext-toolkit.js

@@ -5,12 +5,12 @@
 /* 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");
@@ -18,6 +18,12 @@ XPCOMUtils.defineLazyModuleGetter(this, "ContextualIdentityService",
 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 */