From 712857ab0cc322fbf24f0dae2d9cf51a04f71e27 Mon Sep 17 00:00:00 2001 From: Julian Descottes Date: Mon, 8 Nov 2021 08:03:08 +0000 Subject: [PATCH] Bug 1737865 - [devtools] Do not treat xpcshell targets as parent process targets r=ochameau,nchevobbe Differential Revision: https://phabricator.services.mozilla.com/D130212 --- .../client/debugger/src/client/firefox.js | 11 +++++++--- .../test/browser_process_descriptor.js | 6 ++++- devtools/client/fronts/descriptors/process.js | 22 ++++++++++++++++--- devtools/client/fronts/root.js | 2 +- .../client/fronts/targets/target-mixin.js | 2 +- devtools/client/webconsole/webconsole-ui.js | 2 +- devtools/server/actors/descriptors/process.js | 20 ++++++++++++----- .../tests/browser/browser_getProcess.js | 2 +- .../resource/legacy-listeners/source.js | 2 +- .../legacy-listeners/thread-states.js | 2 +- .../commands/resource/resource-command.js | 6 ++--- .../legacy-serviceworkers-watcher.js | 2 +- .../shared/commands/target/target-command.js | 8 +++---- 13 files changed, 60 insertions(+), 27 deletions(-) diff --git a/devtools/client/debugger/src/client/firefox.js b/devtools/client/debugger/src/client/firefox.js index c024df89742f..0aae1b8b584f 100644 --- a/devtools/client/debugger/src/client/firefox.js +++ b/devtools/client/debugger/src/client/firefox.js @@ -31,9 +31,14 @@ export async function onConnect(_commands, _resourceCommand, _actions, store) { const { descriptorFront } = commands; const { targetFront } = targetCommand; + + // For tab, browser and webextension toolboxes, we want to enable watching for + // worker targets as soon as the debugger is opened. + // And also for service workers, if the related experimental feature is enabled if ( - targetFront.isBrowsingContext || - descriptorFront.isParentProcessDescriptor + descriptorFront.isTabDescriptor || + descriptorFront.isWebExtensionDescriptor || + descriptorFront.isBrowserProcessDescriptor ) { targetCommand.listenForWorkers = true; if (descriptorFront.isLocalTab && features.windowlessServiceWorkers) { @@ -108,7 +113,7 @@ export function onDisconnect() { } async function onTargetAvailable({ targetFront, isTargetSwitching }) { - const isBrowserToolbox = commands.descriptorFront.isParentProcessDescriptor; + const isBrowserToolbox = commands.descriptorFront.isBrowserProcessDescriptor; const isNonTopLevelFrameTarget = !targetFront.isTopLevel && targetFront.targetType === targetCommand.TYPES.FRAME; diff --git a/devtools/client/framework/test/browser_process_descriptor.js b/devtools/client/framework/test/browser_process_descriptor.js index 400d90e6fe4b..c02480029f56 100644 --- a/devtools/client/framework/test/browser_process_descriptor.js +++ b/devtools/client/framework/test/browser_process_descriptor.js @@ -38,7 +38,11 @@ add_task(async () => { const descriptor = await client.mainRoot.getProcess(osPid); ok(descriptor, "Got the process descriptor"); is(descriptor.id, osPid, "descriptor id is the PID"); - is(descriptor.isParent, false, "isParent is false for content processes"); + is( + descriptor.isParentProcessDescriptor, + false, + "isParentProcessDescriptor is false for content processes" + ); // Force getting the target, otherwise we don't connect to the process // via the connector and don't know when the process is destroyed. diff --git a/devtools/client/fronts/descriptors/process.js b/devtools/client/fronts/descriptors/process.js index cbdfda9b2040..1198e7048b5a 100644 --- a/devtools/client/fronts/descriptors/process.js +++ b/devtools/client/fronts/descriptors/process.js @@ -25,14 +25,15 @@ class ProcessDescriptorFront extends DescriptorMixin( ) { constructor(client, targetFront, parentFront) { super(client, targetFront, parentFront); - this.isParent = false; + this._isParent = false; this._processTargetFront = null; this._targetFrontPromise = null; } form(json) { this.id = json.id; - this.isParent = json.isParent; + this._isParent = json.isParent; + this._isWindowlessParent = json.isWindowlessParent; this.traits = json.traits || {}; } @@ -64,8 +65,23 @@ class ProcessDescriptorFront extends DescriptorMixin( return front; } + /** + * This flag should be true for parent process descriptors of a regular + * browser instance, where you can expect the target to be associated with a + * window global. + * + * This will typically be true for the descriptor used by the Browser Toolbox + * or the Browser Console opened against a regular Firefox instance. + * + * On the contrary this will be false for parent process descriptors created + * for xpcshell debugging or for background task debugging. + */ + get isBrowserProcessDescriptor() { + return this._isParent && !this._isWindowlessParent; + } + get isParentProcessDescriptor() { - return this.isParent; + return this._isParent; } get isProcessDescriptor() { diff --git a/devtools/client/fronts/root.js b/devtools/client/fronts/root.js index aaa4306e6af9..28e964403fc7 100644 --- a/devtools/client/fronts/root.js +++ b/devtools/client/fronts/root.js @@ -166,7 +166,7 @@ class RootFront extends FrontClassWithSpec(rootSpec) { const processWorkers = await Promise.all( processes.map(async processDescriptorFront => { // Ignore parent process - if (processDescriptorFront.isParent) { + if (processDescriptorFront.isParentProcessDescriptor) { return []; } const front = await processDescriptorFront.getTarget(); diff --git a/devtools/client/fronts/targets/target-mixin.js b/devtools/client/fronts/targets/target-mixin.js index 074bb76b8a5b..77fa11c6742a 100644 --- a/devtools/client/fronts/targets/target-mixin.js +++ b/devtools/client/fronts/targets/target-mixin.js @@ -502,7 +502,7 @@ function TargetMixin(parentClass) { } const isBrowserToolbox = - targetCommand.descriptorFront.isParentProcessDescriptor; + targetCommand.descriptorFront.isBrowserProcessDescriptor; const isNonTopLevelFrameTarget = !this.isTopLevel && this.targetType === targetCommand.TYPES.FRAME; diff --git a/devtools/client/webconsole/webconsole-ui.js b/devtools/client/webconsole/webconsole-ui.js index 131d18036cc8..82be9304ed93 100644 --- a/devtools/client/webconsole/webconsole-ui.js +++ b/devtools/client/webconsole/webconsole-ui.js @@ -62,7 +62,7 @@ class WebConsoleUI { this.isBrowserConsole = this.hud.isBrowserConsole; this.isBrowserToolboxConsole = - this.hud.commands.descriptorFront.isParentProcessDescriptor && + this.hud.commands.descriptorFront.isBrowserProcessDescriptor && !this.isBrowserConsole; this.fissionSupport = Services.prefs.getBoolPref( constants.PREFS.FEATURES.BROWSER_TOOLBOX_FISSION diff --git a/devtools/server/actors/descriptors/process.js b/devtools/server/actors/descriptors/process.js index 23a6c739ac6e..040aba02468f 100644 --- a/devtools/server/actors/descriptors/process.js +++ b/devtools/server/actors/descriptors/process.js @@ -57,13 +57,20 @@ const ProcessDescriptorActor = ActorClassWithSpec(processDescriptorSpec, { return null; }, - _parentProcessConnect() { + get isWindowlessParent() { + return this.isParent && this.isXpcshell; + }, + + get isXpcshell() { const env = Cc["@mozilla.org/process/environment;1"].getService( Ci.nsIEnvironment ); - const isXpcshell = env.exists("XPCSHELL_TEST_PROFILE_DIR"); + return env.exists("XPCSHELL_TEST_PROFILE_DIR"); + }, + + _parentProcessConnect() { let targetActor; - if (isXpcshell) { + if (this.isXpcshell) { // Check if we are running on xpcshell. // When running on xpcshell, there is no valid browsing context to attach to // and so ParentProcessTargetActor doesn't make sense as it inherits from @@ -163,19 +170,20 @@ const ProcessDescriptorActor = ActorClassWithSpec(processDescriptorSpec, { actor: this.actorID, id: this.id, isParent: this.isParent, + isWindowlessParent: this.isWindowlessParent, traits: { // Supports the Watcher actor. Can be removed as part of Bug 1680280. watcher: true, // ParentProcessTargetActor can be reloaded. - supportsReloadDescriptor: this.isParent, + supportsReloadDescriptor: this.isParent && !this.isWindowlessParent, }, }; }, async reloadDescriptor({ bypassCache }) { - if (!this.isParent) { + if (!this.isParent || this.isWindowlessParent) { throw new Error( - "reloadDescriptor is not available for content process descriptors" + "reloadDescriptor is only available for parent process descriptors linked to a window" ); } diff --git a/devtools/server/tests/browser/browser_getProcess.js b/devtools/server/tests/browser/browser_getProcess.js index 2e0247200e89..74c0197eba57 100644 --- a/devtools/server/tests/browser/browser_getProcess.js +++ b/devtools/server/tests/browser/browser_getProcess.js @@ -48,7 +48,7 @@ add_task(async () => { ok(descriptor, "Got the new process descriptor"); // Connect to the first content process available - const content = processes.filter(p => !p.isParent)[0]; + const content = processes.filter(p => !p.isParentProcessDescriptor)[0]; const processDescriptor = await client.mainRoot.getProcess(content.id); const front = await processDescriptor.getTarget(); diff --git a/devtools/shared/commands/resource/legacy-listeners/source.js b/devtools/shared/commands/resource/legacy-listeners/source.js index 3e9cce944aa4..742cc9227380 100644 --- a/devtools/shared/commands/resource/legacy-listeners/source.js +++ b/devtools/shared/commands/resource/legacy-listeners/source.js @@ -25,7 +25,7 @@ const ResourceCommand = require("devtools/shared/commands/resource/resource-comm */ module.exports = async function({ targetCommand, targetFront, onAvailable }) { const isBrowserToolbox = - targetCommand.descriptorFront.isParentProcessDescriptor; + targetCommand.descriptorFront.isBrowserProcessDescriptor; const isNonTopLevelFrameTarget = !targetFront.isTopLevel && targetFront.targetType === targetCommand.TYPES.FRAME; diff --git a/devtools/shared/commands/resource/legacy-listeners/thread-states.js b/devtools/shared/commands/resource/legacy-listeners/thread-states.js index 7f3ff2fe9f7f..86a46b416a9c 100644 --- a/devtools/shared/commands/resource/legacy-listeners/thread-states.js +++ b/devtools/shared/commands/resource/legacy-listeners/thread-states.js @@ -8,7 +8,7 @@ const ResourceCommand = require("devtools/shared/commands/resource/resource-comm module.exports = async function({ targetCommand, targetFront, onAvailable }) { const isBrowserToolbox = - targetCommand.descriptorFront.isParentProcessDescriptor; + targetCommand.descriptorFront.isBrowserProcessDescriptor; const isNonTopLevelFrameTarget = !targetFront.isTopLevel && targetFront.targetType === targetCommand.TYPES.FRAME; diff --git a/devtools/shared/commands/resource/resource-command.js b/devtools/shared/commands/resource/resource-command.js index d62f2d989dae..9f7870861d64 100644 --- a/devtools/shared/commands/resource/resource-command.js +++ b/devtools/shared/commands/resource/resource-command.js @@ -861,11 +861,11 @@ class ResourceCommand { * @return {Boolean} True, if the server supports this type. */ hasResourceCommandSupport(resourceType) { - // If the targetCommand top level target is a parent process, we're in the browser console or browser toolbox. - // In such case, if the browser toolbox fission pref is disabled, we don't want to use watchers + // If we're in the browser console or browser toolbox and the browser + // toolbox fission pref is disabled, we don't want to use watchers // (even if traits on the server are enabled). if ( - this.targetCommand.descriptorFront.isParentProcessDescriptor && + this.targetCommand.descriptorFront.isBrowserProcessDescriptor && !Services.prefs.getBoolPref(BROWSERTOOLBOX_FISSION_ENABLED, false) ) { return false; diff --git a/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js b/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js index 74e5e7ba9970..7289deb4058b 100644 --- a/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js +++ b/devtools/shared/commands/target/legacy-target-watchers/legacy-serviceworkers-watcher.js @@ -293,7 +293,7 @@ class LegacyServiceWorkersWatcher extends LegacyWorkersWatcher { // Check if the registration is relevant for the current target, ie // corresponds to the same domain. _isRegistrationValidForTarget(registration) { - if (this.targetCommand.descriptorFront.isParentProcessDescriptor) { + if (this.targetCommand.descriptorFront.isBrowserProcessDescriptor) { // All registrations are valid for main process debugging. return true; } diff --git a/devtools/shared/commands/target/target-command.js b/devtools/shared/commands/target/target-command.js index 04734e293bed..c5fe146b5d60 100644 --- a/devtools/shared/commands/target/target-command.js +++ b/devtools/shared/commands/target/target-command.js @@ -339,11 +339,11 @@ class TargetCommand extends EventEmitter { * optional targetTypeOrTrait */ hasTargetWatcherSupport(targetTypeOrTrait) { - // If the top level target is a parent process, we're in the browser console or browser toolbox. - // In such case, if the browser toolbox fission pref is disabled, we don't want to use watchers + // If we're in the browser console or browser toolbox and the browser + // toolbox fission pref is disabled, we don't want to use watchers // (even if traits on the server are enabled). if ( - this.descriptorFront.isParentProcessDescriptor && + this.descriptorFront.isBrowserProcessDescriptor && !Services.prefs.getBoolPref(BROWSERTOOLBOX_FISSION_ENABLED, false) ) { return false; @@ -460,7 +460,7 @@ class TargetCommand extends EventEmitter { if (this.descriptorFront.isLocalTab) { types = [TargetCommand.TYPES.FRAME]; - } else if (this.descriptorFront.isParentProcessDescriptor) { + } else if (this.descriptorFront.isBrowserProcessDescriptor) { const fissionBrowserToolboxEnabled = Services.prefs.getBoolPref( BROWSERTOOLBOX_FISSION_ENABLED );