From d4ba518a1201c452eeb3bace212ed8da89170aea Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Thu, 7 Apr 2022 22:26:19 +0000 Subject: [PATCH] Bug 1762394 menu startupCache persistence r=robwu Add a StartupCache for menus so they are recreated when extensions use event pages. Differential Revision: https://phabricator.services.mozilla.com/D139789 --- .../components/extensions/ext-browser.json | 2 + .../components/extensions/parent/ext-menus.js | 195 ++++++-- .../test/xpcshell/test_ext_menu_startup.js | 439 ++++++++++++++++++ .../extensions/test/xpcshell/xpcshell.ini | 1 + .../components/extensions/ExtensionParent.jsm | 2 + 5 files changed, 590 insertions(+), 49 deletions(-) create mode 100644 browser/components/extensions/test/xpcshell/test_ext_menu_startup.js diff --git a/browser/components/extensions/ext-browser.json b/browser/components/extensions/ext-browser.json index 8bcbc5c1febf..8654213b2f38 100644 --- a/browser/components/extensions/ext-browser.json +++ b/browser/components/extensions/ext-browser.json @@ -118,6 +118,8 @@ "url": "chrome://browser/content/parent/ext-menus.js", "schema": "chrome://browser/content/schemas/menus.json", "scopes": ["addon_parent"], + "events": ["startup"], + "permissions": ["menus", "contextMenus"], "paths": [ ["contextMenus"], ["menus"], diff --git a/browser/components/extensions/parent/ext-menus.js b/browser/components/extensions/parent/ext-menus.js index 94552042b10b..9b3bdc2f392c 100644 --- a/browser/components/extensions/parent/ext-menus.js +++ b/browser/components/extensions/parent/ext-menus.js @@ -24,7 +24,7 @@ var { ExtensionParent } = ChromeUtils.import( "resource://gre/modules/ExtensionParent.jsm" ); -var { IconDetails } = ExtensionParent; +var { IconDetails, StartupCache } = ExtensionParent; const ACTION_MENU_TOP_LEVEL_LIMIT = 6; @@ -33,6 +33,12 @@ const ACTION_MENU_TOP_LEVEL_LIMIT = 6; // this cannot be a weak map. var gMenuMap = new Map(); +// Map[Extension -> Map[ID -> MenuCreateProperties]] +// The map object for each extension is a reference to the same +// object in StartupCache.menus. This provides a non-async +// getter for that object. +var gStartupCache = new Map(); + // Map[Extension -> MenuItem] var gRootItems = new Map(); @@ -711,38 +717,42 @@ function addMenuEventInfo(info, contextData, extension, includeSensitiveData) { } } -function MenuItem(extension, createProperties, isRoot = false) { - this.extension = extension; - this.children = []; - this.parent = null; - this.tabManager = extension.tabManager; +class MenuItem { + constructor(extension, createProperties, isRoot = false) { + this.extension = extension; + this.children = []; + this.parent = null; + this.tabManager = extension.tabManager; - this.setDefaults(); - this.setProps(createProperties); + this.setDefaults(); + this.setProps(createProperties); - if (!this.hasOwnProperty("_id")) { - this.id = gNextMenuItemID++; + if (!this.hasOwnProperty("_id")) { + this.id = gNextMenuItemID++; + } + // If the item is not the root and has no parent + // it must be a child of the root. + if (!isRoot && !this.parent) { + this.root.addChild(this); + } } - // If the item is not the root and has no parent - // it must be a child of the root. - if (!isRoot && !this.parent) { - this.root.addChild(this); - } -} -MenuItem.prototype = { - setProps(createProperties) { - for (let propName in createProperties) { - if (createProperties[propName] === null) { + static mergeProps(obj, properties) { + for (let propName in properties) { + if (properties[propName] === null) { // Omitted optional argument. continue; } - this[propName] = createProperties[propName]; + obj[propName] = properties[propName]; } - if ("icons" in createProperties && createProperties.icons === null) { - this.icons = null; + if ("icons" in properties && properties.icons === null && obj.icons) { + obj.icons = null; } + } + + setProps(createProperties) { + MenuItem.mergeProps(this, createProperties); if (createProperties.documentUrlPatterns != null) { this.documentUrlMatchPattern = parseMatchPatterns( @@ -766,7 +776,7 @@ MenuItem.prototype = { if (createProperties.parentId && !createProperties.contexts) { this.contexts = this.parent.contexts; } - }, + } setDefaults() { this.setProps({ @@ -776,7 +786,7 @@ MenuItem.prototype = { enabled: true, visible: true, }); - }, + } set id(id) { if (this.hasOwnProperty("_id")) { @@ -787,11 +797,11 @@ MenuItem.prototype = { throw new ExtensionError(`ID already exists: ${id}`); } this._id = id; - }, + } get id() { return this._id; - }, + } get elementId() { let id = this.id; @@ -803,7 +813,7 @@ MenuItem.prototype = { id = `_${id}`; } return `${makeWidgetId(this.extension.id)}-menuitem-${id}`; - }, + } ensureValidParentId(parentId) { if (parentId === undefined) { @@ -822,7 +832,25 @@ MenuItem.prototype = { ); } } - }, + } + + /** + * When updating menu properties we need to ensure parents exist + * in the cache map before children. That allows the menus to be + * created in the correct sequence on startup. This reparents the + * tree starting from this instance of MenuItem. + */ + reparentInCache() { + let { id, extension } = this; + let cachedMap = gStartupCache.get(extension); + let createProperties = cachedMap.get(id); + cachedMap.delete(id); + cachedMap.set(id, createProperties); + + for (let child of this.children) { + child.reparentInCache(); + } + } set parentId(parentId) { this.ensureValidParentId(parentId); @@ -837,11 +865,11 @@ MenuItem.prototype = { let menuMap = gMenuMap.get(this.extension); menuMap.get(parentId).addChild(this); } - }, + } get parentId() { return this.parent ? this.parent.id : undefined; - }, + } addChild(child) { if (child.parent) { @@ -849,7 +877,7 @@ MenuItem.prototype = { } this.children.push(child); child.parent = this; - }, + } detachChild(child) { let idx = this.children.indexOf(child); @@ -858,7 +886,7 @@ MenuItem.prototype = { } this.children.splice(idx, 1); child.parent = null; - }, + } get root() { let extension = this.extension; @@ -872,7 +900,7 @@ MenuItem.prototype = { } return gRootItems.get(extension); - }, + } remove() { if (this.parent) { @@ -885,10 +913,14 @@ MenuItem.prototype = { let menuMap = gMenuMap.get(this.extension); menuMap.delete(this.id); + // Menu items are saved if !extension.persistentBackground. + if (gStartupCache.get(this.extension)?.delete(this.id)) { + StartupCache.save(); + } if (this.root == this) { gRootItems.delete(this.extension); } - }, + } getClickInfo(contextData, wasChecked) { let info = { @@ -906,7 +938,7 @@ MenuItem.prototype = { } return info; - }, + } enabledForContext(contextData) { if (!this.visible) { @@ -968,8 +1000,8 @@ MenuItem.prototype = { } return true; - }, -}; + } +} // windowTracker only looks as browser windows, but we're also interested in // the Library window. Helper for menuTracker below. @@ -1199,6 +1231,38 @@ this.menusInternal = class extends ExtensionAPIPersistent { gMenuMap.set(extension, new Map()); } + restoreFromCache() { + let { extension } = this; + // ensure extension has not shutdown + if (!this.extension) { + return; + } + for (let createProperties of gStartupCache.get(extension).values()) { + // The order of menu creation is significant, see reparentInCache. + let menuItem = new MenuItem(extension, createProperties); + gMenuMap.get(extension).set(menuItem.id, menuItem); + } + // Used for testing + extension.emit("webext-menus-created", gMenuMap.get(extension)); + } + + async onStartup() { + let { extension } = this; + if (extension.persistentBackground) { + return; + } + // Using the map retains insertion order. + let cachedMenus = await StartupCache.menus.get(extension.id, () => { + return new Map(); + }); + gStartupCache.set(extension, cachedMenus); + if (!cachedMenus.size) { + return; + } + + this.restoreFromCache(); + } + onShutdown() { let { extension } = this; @@ -1206,6 +1270,7 @@ this.menusInternal = class extends ExtensionAPIPersistent { gMenuMap.delete(extension); gRootItems.delete(extension); gShownMenuItems.delete(extension); + gStartupCache.delete(extension); gOnShownSubscribers.delete(extension); if (!gMenuMap.size) { menuTracker.unregister(); @@ -1333,7 +1398,7 @@ this.menusInternal = class extends ExtensionAPIPersistent { contextMenus: menus, menus, menusInternal: { - create: function(createProperties) { + create(createProperties) { // event pages require id if (!extension.persistentBackground) { if (!createProperties.id) { @@ -1349,31 +1414,63 @@ this.menusInternal = class extends ExtensionAPIPersistent { } // 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. + // it, the implementation of menus.create in the child will add it for + // extensions with persistent backgrounds, but not otherwise. let menuItem = new MenuItem(extension, createProperties); gMenuMap.get(extension).set(menuItem.id, menuItem); - }, - - update: function(id, updateProperties) { - let menuItem = gMenuMap.get(extension).get(id); - if (menuItem) { - menuItem.setProps(updateProperties); + if (!extension.persistentBackground) { + // Only cache properties that are necessary. + let cached = {}; + MenuItem.mergeProps(cached, createProperties); + gStartupCache.get(extension).set(menuItem.id, cached); + StartupCache.save(); } }, - remove: function(id) { + update(id, updateProperties) { + let menuItem = gMenuMap.get(extension).get(id); + if (!menuItem) { + return; + } + menuItem.setProps(updateProperties); + + // Update the startup cache for non-persistent extensions. + if (extension.persistentBackground) { + return; + } + + let cached = gStartupCache.get(extension).get(id); + let reparent = + updateProperties.parentId != null && + cached.parentId != updateProperties.parentId; + MenuItem.mergeProps(cached, updateProperties); + if (reparent) { + // The order of menu creation is significant, see reparentInCache. + menuItem.reparentInCache(); + } + StartupCache.save(); + }, + + remove(id) { let menuItem = gMenuMap.get(extension).get(id); if (menuItem) { menuItem.remove(); } }, - removeAll: function() { + removeAll() { let root = gRootItems.get(extension); if (root) { root.remove(); } + // Should be empty, just extra assurance. + if (!extension.persistentBackground) { + let cached = gStartupCache.get(extension); + if (cached.size) { + cached.clear(); + StartupCache.save(); + } + } }, onClicked: new EventManager({ diff --git a/browser/components/extensions/test/xpcshell/test_ext_menu_startup.js b/browser/components/extensions/test/xpcshell/test_ext_menu_startup.js new file mode 100644 index 000000000000..408b70db180d --- /dev/null +++ b/browser/components/extensions/test/xpcshell/test_ext_menu_startup.js @@ -0,0 +1,439 @@ +"use strict"; + +ChromeUtils.defineModuleGetter( + this, + "ExtensionParent", + "resource://gre/modules/ExtensionParent.jsm" +); + +ChromeUtils.defineModuleGetter( + this, + "Management", + "resource://gre/modules/Extension.jsm" +); + +const { AddonTestUtils } = ChromeUtils.import( + "resource://testing-common/AddonTestUtils.jsm" +); + +AddonTestUtils.init(this); +AddonTestUtils.overrideCertDB(); +AddonTestUtils.createAppInfo( + "xpcshell@tests.mozilla.org", + "XPCShell", + "42", + "42" +); + +Services.prefs.setBoolPref("extensions.eventPages.enabled", true); + +function getExtension(id, background, useAddonManager) { + return ExtensionTestUtils.loadExtension({ + useAddonManager, + manifest: { + applications: { gecko: { id } }, + permissions: ["menus"], + background: { persistent: false }, + }, + background, + }); +} + +async function expectCached(extension, expect) { + let { StartupCache } = ExtensionParent; + let cached = await StartupCache.menus.get(extension.id); + let createProperties = Array.from(cached.values()); + equal(cached.size, expect.length, "menus saved in cache"); + // The menus startupCache is a map and the order is significant + // for recreating menus on startup. Ensure that they are in + // the expected order. We only verify specific keys here rather + // than all menu properties. + for (let i in createProperties) { + Assert.deepEqual( + createProperties[i], + expect[i], + "expected cached properties exist" + ); + } +} + +function promiseExtensionEvent(wrapper, event) { + return new Promise(resolve => { + wrapper.extension.once(event, (kind, data) => { + resolve(data); + }); + }); +} + +add_setup(async () => { + await AddonTestUtils.promiseStartupManager(); +}); + +add_task(async function test_menu_onInstalled() { + async function background() { + browser.runtime.onInstalled.addListener(async () => { + const parentId = browser.menus.create({ + contexts: ["all"], + title: "parent", + id: "test-parent", + }); + browser.menus.create({ + parentId, + title: "click A", + id: "test-click-a", + }); + browser.menus.create( + { + parentId, + title: "click B", + id: "test-click-b", + }, + () => { + browser.test.sendMessage("onInstalled"); + } + ); + }); + browser.menus.create( + { + contexts: ["tab"], + title: "top-level", + id: "test-top-level", + }, + () => { + browser.test.sendMessage("create", browser.runtime.lastError?.message); + } + ); + + browser.test.onMessage.addListener(async msg => { + browser.test.log(`onMessage ${msg}`); + if (msg == "updatemenu") { + await browser.menus.update("test-click-a", { title: "click updated" }); + } else if (msg == "removemenu") { + await browser.menus.remove("test-click-b"); + } else if (msg == "removeall") { + await browser.menus.removeAll(); + } + browser.test.sendMessage("updated"); + }); + } + + const extension = getExtension( + "test-persist@mochitest", + background, + "permanent" + ); + + await extension.startup(); + let lastError = await extension.awaitMessage("create"); + Assert.equal(lastError, undefined, "no error creating menu"); + await extension.awaitMessage("onInstalled"); + await extension.terminateBackground(); + + await expectCached(extension, [ + { + contexts: ["tab"], + id: "test-top-level", + title: "top-level", + }, + { contexts: ["all"], id: "test-parent", title: "parent" }, + { + id: "test-click-a", + parentId: "test-parent", + title: "click A", + }, + { + id: "test-click-b", + parentId: "test-parent", + title: "click B", + }, + ]); + + await extension.wakeupBackground(); + lastError = await extension.awaitMessage("create"); + Assert.equal( + lastError, + "The menu id test-top-level already exists in menus.create.", + "correct error creating menu" + ); + + await AddonTestUtils.promiseRestartManager(); + await extension.awaitStartup(); + + // verify the startupcache + await expectCached(extension, [ + { + contexts: ["tab"], + id: "test-top-level", + title: "top-level", + }, + { contexts: ["all"], id: "test-parent", title: "parent" }, + { + id: "test-click-a", + parentId: "test-parent", + title: "click A", + }, + { + id: "test-click-b", + parentId: "test-parent", + title: "click B", + }, + ]); + + equal( + extension.extension.backgroundState, + "stopped", + "background is not running" + ); + await extension.wakeupBackground(); + lastError = await extension.awaitMessage("create"); + Assert.equal( + lastError, + "The menu id test-top-level already exists in menus.create.", + "correct error creating menu" + ); + + extension.sendMessage("updatemenu"); + await extension.awaitMessage("updated"); + await extension.terminateBackground(); + + // Title change is cached + await expectCached(extension, [ + { + contexts: ["tab"], + id: "test-top-level", + title: "top-level", + }, + { contexts: ["all"], id: "test-parent", title: "parent" }, + { + id: "test-click-a", + parentId: "test-parent", + title: "click updated", + }, + { + id: "test-click-b", + parentId: "test-parent", + title: "click B", + }, + ]); + + await extension.wakeupBackground(); + lastError = await extension.awaitMessage("create"); + Assert.equal( + lastError, + "The menu id test-top-level already exists in menus.create.", + "correct error creating menu" + ); + + extension.sendMessage("removemenu"); + await extension.awaitMessage("updated"); + await extension.terminateBackground(); + + // menu removed + await expectCached(extension, [ + { + contexts: ["tab"], + id: "test-top-level", + title: "top-level", + }, + { contexts: ["all"], id: "test-parent", title: "parent" }, + { + id: "test-click-a", + parentId: "test-parent", + title: "click updated", + }, + ]); + + await extension.wakeupBackground(); + lastError = await extension.awaitMessage("create"); + Assert.equal( + lastError, + "The menu id test-top-level already exists in menus.create.", + "correct error creating menu" + ); + + extension.sendMessage("removeall"); + await extension.awaitMessage("updated"); + await extension.terminateBackground(); + + // menus removed + await expectCached(extension, []); + + await extension.unload(); +}); + +add_task(async function test_menu_nested() { + async function background() { + browser.test.onMessage.addListener(async (action, properties) => { + browser.test.log(`onMessage ${action}`); + switch (action) { + case "create": + await new Promise(resolve => { + browser.menus.create(properties, resolve); + }); + break; + case "update": + { + let { id, ...update } = properties; + await browser.menus.update(id, update); + } + break; + case "remove": + { + let { id } = properties; + await browser.menus.remove(id); + } + break; + case "removeAll": + await browser.menus.removeAll(); + break; + } + browser.test.sendMessage("updated"); + }); + } + + const extension = getExtension( + "test-nesting@mochitest", + background, + "permanent" + ); + await extension.startup(); + + extension.sendMessage("create", { + id: "first", + contexts: ["all"], + title: "first", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "first", title: "first" }, + ]); + + extension.sendMessage("create", { + id: "second", + contexts: ["all"], + title: "second", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "first", title: "first" }, + { contexts: ["all"], id: "second", title: "second" }, + ]); + + extension.sendMessage("create", { + id: "third", + contexts: ["all"], + title: "third", + parentId: "first", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "first", title: "first" }, + { contexts: ["all"], id: "second", title: "second" }, + { + contexts: ["all"], + id: "third", + parentId: "first", + title: "third", + }, + ]); + + extension.sendMessage("create", { + id: "fourth", + contexts: ["all"], + title: "fourth", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "first", title: "first" }, + { contexts: ["all"], id: "second", title: "second" }, + { + contexts: ["all"], + id: "third", + parentId: "first", + title: "third", + }, + { contexts: ["all"], id: "fourth", title: "fourth" }, + ]); + + extension.sendMessage("update", { + id: "first", + parentId: "second", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "second", title: "second" }, + { contexts: ["all"], id: "fourth", title: "fourth" }, + { + contexts: ["all"], + id: "first", + title: "first", + parentId: "second", + }, + { + contexts: ["all"], + id: "third", + parentId: "first", + title: "third", + }, + ]); + + await AddonTestUtils.promiseShutdownManager(); + // We need to attach an event listener before the + // startup event is emitted. Fortunately, we + // emit via Management before emitting on extension. + let promiseMenus; + Management.once("startup", (kind, ext) => { + info(`management ${kind} ${ext.id}`); + promiseMenus = promiseExtensionEvent( + { extension: ext }, + "webext-menus-created" + ); + }); + await AddonTestUtils.promiseStartupManager(); + await extension.awaitStartup(); + await extension.wakeupBackground(); + + await expectCached(extension, [ + { contexts: ["all"], id: "second", title: "second" }, + { contexts: ["all"], id: "fourth", title: "fourth" }, + { + contexts: ["all"], + id: "first", + title: "first", + parentId: "second", + }, + { + contexts: ["all"], + id: "third", + parentId: "first", + title: "third", + }, + ]); + // validate nesting + let menus = await promiseMenus; + equal(menus.get("first").parentId, "second", "menuitem parent is correct"); + equal( + menus.get("second").children.length, + 1, + "menuitem parent has correct number of children" + ); + equal( + menus.get("second").root.children.length, + 2, // second and forth + "menuitem root has correct number of children" + ); + + extension.sendMessage("remove", { + id: "second", + }); + await extension.awaitMessage("updated"); + await expectCached(extension, [ + { contexts: ["all"], id: "fourth", title: "fourth" }, + ]); + + extension.sendMessage("removeAll"); + await extension.awaitMessage("updated"); + await expectCached(extension, []); + + await extension.unload(); +}); diff --git a/browser/components/extensions/test/xpcshell/xpcshell.ini b/browser/components/extensions/test/xpcshell/xpcshell.ini index cec32852d7f8..a10c8c6d9293 100644 --- a/browser/components/extensions/test/xpcshell/xpcshell.ini +++ b/browser/components/extensions/test/xpcshell/xpcshell.ini @@ -20,6 +20,7 @@ skip-if = tsan # Times out, bug 1612707 [test_ext_manifest_omnibox.js] [test_ext_manifest_permissions.js] [test_ext_menu_caller.js] +[test_ext_menu_startup.js] [test_ext_normandyAddonStudy.js] [test_ext_pageAction_shutdown.js] [test_ext_pkcs11_management.js] diff --git a/toolkit/components/extensions/ExtensionParent.jsm b/toolkit/components/extensions/ExtensionParent.jsm index fdfada851de7..a760ccced4eb 100644 --- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -1875,6 +1875,7 @@ StartupCache = { "other", "permissions", "schemas", + "menus", ]), _ensureDirectoryPromise: null, @@ -1956,6 +1957,7 @@ StartupCache = { this.locales.delete(id), this.manifests.delete(id), this.permissions.delete(id), + this.menus.delete(id), ]).catch(e => { // Ignore the error. It happens when we try to flush the add-on // data after the AddonManager has flushed the entire startup cache.