From 5197c39b9ccfaa4882e078c0b93234f826ae424c Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Mon, 10 Oct 2022 11:28:06 +0000 Subject: [PATCH] Bug 1700909 - [devtools] Migrate TabDescriptorFactory to LocalTabCommandsFactory. r=jdescottes Also move iKnownTab to gDevTools.hasToolboxForTab as it feel more natural. Differential Revision: https://phabricator.services.mozilla.com/D157943 --- .../client/accessibility/test/browser/head.js | 2 +- devtools/client/framework/devtools.js | 23 +++--- .../framework/local-tab-commands-factory.js | 72 +++++++++++++++++ devtools/client/framework/moz.build | 2 +- .../framework/tab-descriptor-factory.js | 81 ------------------- devtools/client/framework/test/browser.ini | 2 +- ...ory.js => browser_tab_commands_factory.js} | 28 +++---- devtools/client/responsive/manager.js | 12 +-- devtools/client/shared/link.js | 5 +- devtools/client/shared/test/shared-head.js | 5 +- devtools/client/storage/test/head.js | 6 +- .../shared/commands/target/target-command.js | 2 +- 12 files changed, 111 insertions(+), 129 deletions(-) create mode 100644 devtools/client/framework/local-tab-commands-factory.js delete mode 100644 devtools/client/framework/tab-descriptor-factory.js rename devtools/client/framework/test/{browser_tab_descriptor_factory.js => browser_tab_commands_factory.js} (51%) diff --git a/devtools/client/accessibility/test/browser/head.js b/devtools/client/accessibility/test/browser/head.js index 0829cc294442..3531728dc128 100644 --- a/devtools/client/accessibility/test/browser/head.js +++ b/devtools/client/accessibility/test/browser/head.js @@ -799,7 +799,7 @@ function addA11yPanelTestsTask(tests, uri, msg, options) { * Resolves when the toolbox and tab have been destroyed and closed. */ async function closeTabToolboxAccessibility(tab = gBrowser.selectedTab) { - if (TabDescriptorFactory.isKnownTab(tab)) { + if (gDevTools.hasToolboxForTab(tab)) { await gDevTools.closeToolboxForTab(tab); } diff --git a/devtools/client/framework/devtools.js b/devtools/client/framework/devtools.js index 8b291c2f34e5..1432a2901ca5 100644 --- a/devtools/client/framework/devtools.js +++ b/devtools/client/framework/devtools.js @@ -16,8 +16,8 @@ ChromeUtils.defineESModuleGetters(lazy, { loader.lazyRequireGetter( this, - "TabDescriptorFactory", - "resource://devtools/client/framework/tab-descriptor-factory.js", + "LocalTabCommandsFactory", + "resource://devtools/client/framework/local-tab-commands-factory.js", true ); loader.lazyRequireGetter( @@ -608,18 +608,21 @@ DevTools.prototype = { const openerTab = tab.ownerGlobal.gBrowser.getTabForBrowser( tab.linkedBrowser.browsingContext.opener.embedderElement ); - const openerDescriptor = await TabDescriptorFactory.getDescriptorForTab( + const openerCommands = await LocalTabCommandsFactory.getCommandsForTab( openerTab ); - if (this.getToolboxForDescriptor(openerDescriptor)) { + if ( + openerCommands && + this.getToolboxForDescriptor(openerCommands.descriptorFront) + ) { console.log( "Can't open a toolbox for this document as this is debugged from its opener tab" ); return null; } } - const descriptor = await TabDescriptorFactory.createDescriptorForTab(tab); - return this.showToolbox(descriptor, { + const commands = await LocalTabCommandsFactory.createCommandsForTab(tab); + return this.showToolbox(commands.descriptorFront, { toolId, hostType, startTime, @@ -768,8 +771,8 @@ DevTools.prototype = { * Returns null otherwise. */ async getToolboxForTab(tab) { - const descriptor = await TabDescriptorFactory.getDescriptorForTab(tab); - return this.getToolboxForDescriptor(descriptor); + const commands = await LocalTabCommandsFactory.getCommandsForTab(tab); + return commands && this.getToolboxForDescriptor(commands.descriptorFront); }, /** @@ -780,8 +783,8 @@ DevTools.prototype = { * - or after toolbox.destroy() resolved if a Toolbox was found */ async closeToolboxForTab(tab) { - const descriptor = await TabDescriptorFactory.getDescriptorForTab(tab); - + const commands = await LocalTabCommandsFactory.getCommandsForTab(tab); + const descriptor = commands.descriptorFront; let toolbox = await this._creatingToolboxes.get(descriptor); if (!toolbox) { toolbox = this._toolboxes.get(descriptor); diff --git a/devtools/client/framework/local-tab-commands-factory.js b/devtools/client/framework/local-tab-commands-factory.js new file mode 100644 index 000000000000..e7e32aa34389 --- /dev/null +++ b/devtools/client/framework/local-tab-commands-factory.js @@ -0,0 +1,72 @@ +/* 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"; + +loader.lazyRequireGetter( + this, + "CommandsFactory", + "resource://devtools/shared/commands/commands-factory.js", + true +); + +// Map of existing Commands objects, keyed by XULTab. +const commandsMap = new WeakMap(); + +/** + * Functions for creating unique Commands for Local Tabs. + */ +exports.LocalTabCommandsFactory = { + /** + * Create a unique commands object for the given tab. + * + * If a commands was already created by this factory for the provided tab, + * it will be returned and no new commands created. + * + * Otherwise, this will automatically: + * - spawn a DevToolsServer in the parent process, + * - create a DevToolsClient + * - connect the DevToolsClient to the DevToolsServer + * - call RootActor's `getTab` request to retrieve the WindowGlobalTargetActor's form + * + * @param {XULTab} tab + * The tab to use in creating a new commands. + * + * @return {Commands object} The commands object for the provided tab. + */ + async createCommandsForTab(tab) { + let commands = commandsMap.get(tab); + if (commands) { + // Keep in mind that commands can be either a promise + // or a commands object. + return commands; + } + + const promise = CommandsFactory.forTab(tab); + // Immediately set the commands's promise in cache to prevent race + commandsMap.set(tab, promise); + commands = await promise; + // Then replace the promise with the commands object + commandsMap.set(tab, commands); + + commands.descriptorFront.once("descriptor-destroyed", () => { + commandsMap.delete(tab); + }); + return commands; + }, + + /** + * Retrieve an existing commands created by this factory for the provided + * tab. Returns null if no commands was created yet. + * + * @param {XULTab} tab + * The tab for which the commands should be retrieved + */ + async getCommandsForTab(tab) { + // commandsMap.get(tab) can either return an initialized commands, a promise + // which will resolve a commands, or null if no commands was ever created + // for this tab. + return commandsMap.get(tab); + }, +}; diff --git a/devtools/client/framework/moz.build b/devtools/client/framework/moz.build index 431923d0ca85..d13eda680296 100644 --- a/devtools/client/framework/moz.build +++ b/devtools/client/framework/moz.build @@ -31,13 +31,13 @@ DevToolsModules( "devtools-browser.js", "devtools.js", "enable-devtools-popup.js", + "local-tab-commands-factory.js", "menu-item.js", "menu.js", "selection.js", "source-map-url-service.js", "store-provider.js", "store.js", - "tab-descriptor-factory.js", "toolbox-context-menu.js", "toolbox-host-manager.js", "toolbox-hosts.js", diff --git a/devtools/client/framework/tab-descriptor-factory.js b/devtools/client/framework/tab-descriptor-factory.js deleted file mode 100644 index 4ea2b59985ad..000000000000 --- a/devtools/client/framework/tab-descriptor-factory.js +++ /dev/null @@ -1,81 +0,0 @@ -/* 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"; - -loader.lazyRequireGetter( - this, - "CommandsFactory", - "resource://devtools/shared/commands/commands-factory.js", - true -); - -// Map of existing Commands objects, keyed by XULTab. -const commandsMap = new WeakMap(); - -/** - * Functions for creating (local) Tab Target Descriptors - */ -exports.TabDescriptorFactory = { - /** - * Create a unique tab target descriptor for the given tab. - * - * If a descriptor was already created by this factory for the provided tab, - * it will be returned and no new descriptor created. - * - * Otherwise, this will automatically: - * - spawn a DevToolsServer in the parent process, - * - create a DevToolsClient - * - connect the DevToolsClient to the DevToolsServer - * - call RootActor's `getTab` request to retrieve the WindowGlobalTargetActor's form - * - * @param {XULTab} tab - * The tab to use in creating a new descriptor. - * - * @return {TabDescriptorFront} The tab descriptor for the provided tab. - */ - async createDescriptorForTab(tab) { - let commands = commandsMap.get(tab); - if (commands) { - commands = await commands; - return commands.descriptorFront; - } - - const promise = CommandsFactory.forTab(tab); - // Immediately set the descriptor's promise in cache to prevent race - commandsMap.set(tab, promise); - commands = await promise; - // Then replace the promise with the descriptor object - commandsMap.set(tab, commands); - - commands.descriptorFront.once("descriptor-destroyed", () => { - commandsMap.delete(tab); - }); - return commands.descriptorFront; - }, - - /** - * Retrieve an existing descriptor created by this factory for the provided - * tab. Returns null if no descriptor was created yet. - * - * @param {XULTab} tab - * The tab for which the descriptor should be retrieved - */ - async getDescriptorForTab(tab) { - // commandsMap.get(tab) can either return initialized commands, a promise - // which will resolve a commands, or null if no commands was ever created - // for this tab. - const commands = await commandsMap.get(tab); - return commands?.descriptorFront; - }, - - /** - * This function allows us to ask if there is a known - * descriptor for a tab without creating a descriptor. - * @return true/false - */ - isKnownTab(tab) { - return commandsMap.has(tab); - }, -}; diff --git a/devtools/client/framework/test/browser.ini b/devtools/client/framework/test/browser.ini index ffdb7c8576dd..279be627a8b2 100644 --- a/devtools/client/framework/test/browser.ini +++ b/devtools/client/framework/test/browser.ini @@ -80,7 +80,7 @@ prefs = [browser_source_map-pub-sub.js] [browser_source_map-reload.js] [browser_source_map-late-script.js] -[browser_tab_descriptor_factory.js] +[browser_tab_commands_factory.js] [browser_tab_descriptor_fission.js] [browser_target_from_url.js] [browser_target_cached-front.js] diff --git a/devtools/client/framework/test/browser_tab_descriptor_factory.js b/devtools/client/framework/test/browser_tab_commands_factory.js similarity index 51% rename from devtools/client/framework/test/browser_tab_descriptor_factory.js rename to devtools/client/framework/test/browser_tab_commands_factory.js index f5d8046ad6ad..8c982ad443df 100644 --- a/devtools/client/framework/test/browser_tab_descriptor_factory.js +++ b/devtools/client/framework/test/browser_tab_commands_factory.js @@ -3,11 +3,11 @@ "use strict"; -// Test TabDescriptorFactory +// Test LocalTabCommandsFactory const { - createCommandsDictionary, -} = require("resource://devtools/shared/commands/index.js"); + LocalTabCommandsFactory, +} = require("resource://devtools/client/framework/local-tab-commands-factory.js"); add_task(async function() { await testTabDescriptorWithURL("data:text/html;charset=utf-8,foo"); @@ -21,32 +21,30 @@ async function testTabDescriptorWithURL(url) { info(`Test TabDescriptor against url ${url}\n`); const tab = await addTab(url); - const descriptor = await TabDescriptorFactory.createDescriptorForTab(tab); - is(descriptor.localTab, tab, "TabDescriptor's localTab is set correctly"); + const commands = await LocalTabCommandsFactory.createCommandsForTab(tab); + is( + commands.descriptorFront.localTab, + tab, + "TabDescriptor's localTab is set correctly" + ); info( - "Calling a second time createDescriptorForTab with the same tab, will return the same descriptor" + "Calling a second time createCommandsForTab with the same tab, will return the same commands" ); - const secondDescriptor = await TabDescriptorFactory.createDescriptorForTab( + const secondCommands = await LocalTabCommandsFactory.createCommandsForTab( tab ); - is(descriptor, secondDescriptor, "second descriptor is the same"); + is(commands, secondCommands, "second commands is the same"); // We have to involve TargetCommand in order to have a function TabDescriptor.getTarget. - // With bug 1700909, all code should soon be migrated to use CommandsFactory instead of TabDescriptorFactory. - // Making this test irrelevant and this cryptic workaround useless. - const commands = await createCommandsDictionary(descriptor); await commands.targetCommand.startListening(); info("Wait for descriptor's target"); - const target = await descriptor.getTarget(); + const target = await commands.descriptorFront.getTarget(); info("Call any method to ensure that each target works"); await target.logInPage("foo"); - info("Destroy the descriptor"); - await descriptor.destroy(); - info("Destroy the command"); await commands.destroy(); diff --git a/devtools/client/responsive/manager.js b/devtools/client/responsive/manager.js index 87d36d2c502e..7047325dc133 100644 --- a/devtools/client/responsive/manager.js +++ b/devtools/client/responsive/manager.js @@ -34,12 +34,6 @@ loader.lazyRequireGetter( "resource://devtools/client/shared/components/NotificationBox.js", true ); -loader.lazyRequireGetter( - this, - "TabDescriptorFactory", - "resource://devtools/client/framework/tab-descriptor-factory.js", - true -); loader.lazyRequireGetter( this, "gDevTools", @@ -153,9 +147,8 @@ class ResponsiveUIManager { */ async recordTelemetryOpen(window, tab, options) { // Track whether a toolbox was opened before RDM was opened. - const isKnownTab = TabDescriptorFactory.isKnownTab(tab); let toolbox; - if (isKnownTab) { + if (gDevTools.hasToolboxForTab(tab)) { toolbox = await gDevTools.getToolboxForTab(tab); } const hostType = toolbox ? toolbox.hostType : "none"; @@ -223,9 +216,8 @@ class ResponsiveUIManager { } async recordTelemetryClose(window, tab) { - const isKnownTab = TabDescriptorFactory.isKnownTab(tab); let toolbox; - if (isKnownTab) { + if (gDevTools.hasToolboxForTab(tab)) { toolbox = await gDevTools.getToolboxForTab(tab); } diff --git a/devtools/client/shared/link.js b/devtools/client/shared/link.js index 952f032d87ab..d43f2fdc4e6b 100644 --- a/devtools/client/shared/link.js +++ b/devtools/client/shared/link.js @@ -7,9 +7,6 @@ const { gDevTools, } = require("resource://devtools/client/framework/devtools.js"); -const { - TabDescriptorFactory, -} = require("resource://devtools/client/framework/tab-descriptor-factory.js"); /** * Retrieve the most recent chrome window. @@ -64,7 +61,7 @@ exports.openContentLink = async function(url, options = {}) { } if (!options.triggeringPrincipal && top.gBrowser) { const tab = top.gBrowser.selectedTab; - if (TabDescriptorFactory.isKnownTab(tab)) { + if (gDevTools.hasToolboxForTab(tab)) { options.triggeringPrincipal = tab.linkedBrowser.contentPrincipal; options.csp = tab.linkedBrowser.csp; } diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js index af28eb68e4ff..d7f6b7cbb0d3 100644 --- a/devtools/client/shared/test/shared-head.js +++ b/devtools/client/shared/test/shared-head.js @@ -64,9 +64,6 @@ const { loader, require } = ChromeUtils.import( const { gDevTools, } = require("resource://devtools/client/framework/devtools.js"); -const { - TabDescriptorFactory, -} = require("resource://devtools/client/framework/tab-descriptor-factory.js"); const { CommandsFactory, } = require("resource://devtools/shared/commands/commands-factory.js"); @@ -1215,7 +1212,7 @@ async function openNewTabAndToolbox(url, toolId, hostType) { * closed. */ async function closeTabAndToolbox(tab = gBrowser.selectedTab) { - if (TabDescriptorFactory.isKnownTab(tab)) { + if (gDevTools.hasToolboxForTab(tab)) { await gDevTools.closeToolboxForTab(tab); } diff --git a/devtools/client/storage/test/head.js b/devtools/client/storage/test/head.js index c3b686cb8139..6101fd7d6346 100644 --- a/devtools/client/storage/test/head.js +++ b/devtools/client/storage/test/head.js @@ -55,6 +55,9 @@ Services.scriptloader.loadSubScript( const { TableWidget, } = require("resource://devtools/client/shared/widgets/TableWidget.js"); +const { + LocalTabCommandsFactory, +} = require("resource://devtools/client/framework/local-tab-commands-factory.js"); const STORAGE_PREF = "devtools.storage.enabled"; const DOM_CACHE = "dom.caches.enabled"; const DUMPEMIT_PREF = "devtools.dump.emit"; @@ -159,9 +162,10 @@ async function openTabAndSetupStorage(url, options = {}) { var openStoragePanel = async function({ tab, descriptor, hostType } = {}) { info("Opening the storage inspector"); if (!descriptor) { - descriptor = await TabDescriptorFactory.createDescriptorForTab( + const commands = await LocalTabCommandsFactory.createCommandsForTab( tab || gBrowser.selectedTab ); + descriptor = commands.descriptorFront; } let storage, toolbox; diff --git a/devtools/shared/commands/target/target-command.js b/devtools/shared/commands/target/target-command.js index 738edc63837f..8f171f82d932 100644 --- a/devtools/shared/commands/target/target-command.js +++ b/devtools/shared/commands/target/target-command.js @@ -996,7 +996,7 @@ class TargetCommand extends EventEmitter { // TabDescriptor may emit the event with a null targetFront, interpret that as if the previous target // has already been destroyed if (targetFront) { - // Wait for the target to be destroyed so that TabDescriptorFactory clears its memoized target for this tab + // Wait for the target to be destroyed so that LocalTabCommandsFactory clears its memoized target for this tab await targetFront.once("target-destroyed"); }