Bug 1333403 - Part 3: Fix using browser.menus from multiple contexts r=kmag
authorTomislav Jovanovic <tomica@gmail.com>
Tue, 25 Apr 2017 23:51:26 +0200
changeset 411472 5cf9d6d03da7bd891baa8b27e2fe35fcfc9ebe57
parent 411471 d9488457b8debef552cc690da9c8deeb8352685a
child 411473 1a3550df404396cd1bb5ea12703f2979b2e3c3e3
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1333403
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1333403 - Part 3: Fix using browser.menus from multiple contexts r=kmag MozReview-Commit-ID: XlP72cr0VT
browser/components/extensions/ext-menus.js
browser/components/extensions/test/browser/browser_ext_menus.js
browser/components/extensions/test/browser/head.js
toolkit/components/extensions/ExtensionCommon.jsm
--- a/browser/components/extensions/ext-menus.js
+++ b/browser/components/extensions/ext-menus.js
@@ -611,39 +611,41 @@ const menuTracker = {
       const trigger = menu.triggerNode;
       const tab = trigger.localName === "tab" ? trigger : tabTracker.activeTab;
       const pageUrl = tab.linkedBrowser.currentURI.spec;
       gMenuBuilder.build({menu, tab, pageUrl, onTab: true});
     }
   },
 };
 
-var gExtensionCount = 0;
+this.menusInternal = class extends ExtensionAPI {
+  constructor(extension) {
+    super(extension);
 
-this.menusInternal = class extends ExtensionAPI {
+    if (!gMenuMap.size) {
+      menuTracker.register();
+    }
+    gMenuMap.set(extension, new Map());
+  }
+
   onShutdown(reason) {
     let {extension} = this;
 
     if (gMenuMap.has(extension)) {
       gMenuMap.delete(extension);
       gRootItems.delete(extension);
-      if (--gExtensionCount == 0) {
+      if (!gMenuMap.size) {
         menuTracker.unregister();
       }
     }
   }
 
   getAPI(context) {
     let {extension} = context;
 
-    gMenuMap.set(extension, new Map());
-    if (++gExtensionCount == 1) {
-      menuTracker.register();
-    }
-
     return {
       menusInternal: {
         create: function(createProperties) {
           // Note that the id is required by the schema. If the addon did not set
           // it, the implementation of menus.create in the child should
           // have added it.
           let menuItem = new MenuItem(extension, createProperties);
           gMenuMap.get(extension).set(menuItem.id, menuItem);
--- a/browser/components/extensions/test/browser/browser_ext_menus.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus.js
@@ -1,13 +1,15 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
+const PAGE = "http://mochi.test:8888/browser/browser/components/extensions/test/browser/context.html";
+
 add_task(async function test_permissions() {
   function background() {
     browser.test.sendMessage("apis", {
       menus: typeof browser.menus,
       contextMenus: typeof browser.contextMenus,
       menusInternal: typeof browser.menusInternal,
     });
   }
@@ -180,18 +182,17 @@ add_task(async function test_onclick_fra
     function onclick(info) {
       browser.test.sendMessage("click", info);
     }
     browser.menus.create({contexts: ["frame", "page"], title: "modify", onclick});
     browser.test.sendMessage("ready");
   }
 
   const extension = ExtensionTestUtils.loadExtension({manifest, background});
-  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser,
-    "http://mochi.test:8888/browser/browser/components/extensions/test/browser/context.html");
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE);
 
   await extension.startup();
   await extension.awaitMessage("ready");
 
   async function click(selectorOrId) {
     const func = (selectorOrId == "body") ? openContextMenu : openContextMenuInFrame;
     const menu = await func(selectorOrId);
     const items = menu.getElementsByAttribute("label", "modify");
@@ -203,8 +204,54 @@ add_task(async function test_onclick_fra
   is(info.frameId, 0, "top level click");
   info = await click("frame");
   isnot(info.frameId, undefined, "frame click, frameId is not undefined");
   isnot(info.frameId, 0, "frame click, frameId probably okay");
 
   await BrowserTestUtils.removeTab(tab);
   await extension.unload();
 });
+
+add_task(async function test_multiple_contexts_init() {
+  const manifest = {
+    permissions: ["menus"],
+  };
+
+  function background() {
+    browser.menus.create({id: "parent", title: "parent"});
+    browser.tabs.create({url: "tab.html", active: false});
+  }
+
+  const files = {
+    "tab.html": "<!DOCTYPE html><meta charset=utf-8><script src=tab.js></script>",
+    "tab.js": function() {
+      browser.menus.create({parentId: "parent", id: "child", title: "child"});
+
+      browser.menus.onClicked.addListener(info => {
+        browser.test.sendMessage("click", info);
+      });
+
+      browser.test.sendMessage("ready");
+    },
+  };
+
+  const tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, PAGE);
+  const extension = ExtensionTestUtils.loadExtension({manifest, background, files});
+
+  await extension.startup();
+  await extension.awaitMessage("ready");
+
+  const menu = await openContextMenu();
+  const items = menu.getElementsByAttribute("label", "parent");
+
+  is(items.length, 1, "Found parent menu item");
+  is(items[0].tagName, "menu", "And it has children");
+
+  const popup = await openSubmenu(items[0]);
+  is(popup.firstChild.label, "child", "Correct child menu item");
+  await closeExtensionContextMenu(popup.firstChild);
+
+  const info = await extension.awaitMessage("click");
+  is(info.menuItemId, "child", "onClicked the correct item");
+
+  await BrowserTestUtils.removeTab(tab);
+  await extension.unload();
+});
--- a/browser/components/extensions/test/browser/head.js
+++ b/browser/components/extensions/test/browser/head.js
@@ -304,17 +304,17 @@ async function openExtensionContextMenu(
   await popupShownPromise;
   return extensionMenu;
 }
 
 async function closeExtensionContextMenu(itemToSelect, modifiers = {}) {
   let contentAreaContextMenu = document.getElementById("contentAreaContextMenu");
   let popupHiddenPromise = BrowserTestUtils.waitForEvent(contentAreaContextMenu, "popuphidden");
   EventUtils.synthesizeMouseAtCenter(itemToSelect, modifiers);
-  await popupHiddenPromise;
+  return popupHiddenPromise;
 }
 
 async function openChromeContextMenu(menuId, target, win = window) {
   const node = win.document.querySelector(target);
   const menu = win.document.getElementById(menuId);
   const shown = BrowserTestUtils.waitForEvent(menu, "popupshown");
   EventUtils.synthesizeMouseAtCenter(node, {type: "contextmenu"}, win);
   await shown;
--- a/toolkit/components/extensions/ExtensionCommon.jsm
+++ b/toolkit/components/extensions/ExtensionCommon.jsm
@@ -1005,17 +1005,17 @@ class SchemaAPIManager extends EventEmit
     }
 
     this._checkLoadModule(module, name);
 
     Services.scriptloader.loadSubScript(module.url, this.global, "UTF-8");
 
     module.loaded = true;
 
-    return this._initModule(module, this.global[name]);
+    return this.global[name];
   }
   /**
    * aSynchronously loads an API module, if not already loaded, and
    * returns its ExtensionAPI constructor.
    *
    * @param {string} name
    *        The name of the module to load.
    *
@@ -1032,17 +1032,17 @@ class SchemaAPIManager extends EventEmit
 
     this._checkLoadModule(module, name);
 
     module.asyncLoaded = ChromeUtils.compileScript(module.url).then(script => {
       script.executeInGlobal(this.global);
 
       module.loaded = true;
 
-      return this._initModule(module, this.global[name]);
+      return this.global[name];
     });
 
     return module.asyncLoaded;
   }
 
   /**
    * Checks whether the given API module may be loaded for the given
    * extension, in the given scope.
@@ -1071,24 +1071,16 @@ class SchemaAPIManager extends EventEmit
 
     if (!Schemas.checkPermissions(module.namespaceName, extension)) {
       return false;
     }
 
     return true;
   }
 
-  _initModule(info, cls) {
-    // FIXME: This both a) does nothing, and b) is not used anymore.
-    cls.namespaceName = cls.namespaceName;
-    cls.scopes = new Set(info.scopes);
-
-    return cls;
-  }
-
   _checkLoadModule(module, name) {
     if (!module) {
       throw new Error(`Module '${name}' does not exist`);
     }
     if (module.asyncLoaded) {
       throw new Error(`Module '${name}' currently being lazily loaded`);
     }
     if (this.global[name]) {