Bug 1333403 - Part 3: Fix using browser.menus from multiple contexts r=kmag
☠☠ backed out by 1bd81296c74c ☠ ☠
authorTomislav Jovanovic <tomica@gmail.com>
Tue, 25 Apr 2017 23:51:26 +0200
changeset 411463 d842c744941efbbcc04136fcce2eb42fd1fb76f2
parent 411462 cfc47df7453793820bfc716d94f14de32e87d4ce
child 411464 1bd81296c74c392a01a593a719806f8900049607
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]) {