diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index b3aab3509fdd..c12000dd1db1 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -2193,6 +2193,10 @@ pref("devtools.target-switching.server.enabled", true); // remote frames). pref("devtools.every-frame-target.enabled", true); +// Controls the hability to debug popups from the same DevTools +// of the original tab the popups are coming from +pref("devtools.popups.debug", false); + // Toolbox Button preferences pref("devtools.command-button-pick.enabled", true); pref("devtools.command-button-frames.enabled", true); diff --git a/devtools/client/fronts/descriptors/tab.js b/devtools/client/fronts/descriptors/tab.js index 8c02847408d8..23792def11a7 100644 --- a/devtools/client/fronts/descriptors/tab.js +++ b/devtools/client/fronts/descriptors/tab.js @@ -28,6 +28,7 @@ const { const SERVER_TARGET_SWITCHING_ENABLED_PREF = "devtools.target-switching.server.enabled"; +const POPUP_DEBUG_PREF = "devtools.popups.debug"; /** * DescriptorFront for tab targets. @@ -97,8 +98,13 @@ class TabDescriptorFront extends DescriptorMixin( } getWatcher() { + const isPopupDebuggingEnabled = Services.prefs.getBoolPref( + POPUP_DEBUG_PREF, + false + ); return super.getWatcher({ isServerTargetSwitchingEnabled: this.isServerTargetSwitchingEnabled(), + isPopupDebuggingEnabled, }); } diff --git a/devtools/server/actors/descriptors/tab.js b/devtools/server/actors/descriptors/tab.js index 309006c92995..e02f7dddf54a 100644 --- a/devtools/server/actors/descriptors/tab.js +++ b/devtools/server/actors/descriptors/tab.js @@ -188,6 +188,7 @@ const TabDescriptorActor = ActorClassWithSpec(tabDescriptorSpec, { this.conn, createBrowserElementSessionContext(this._browser, { isServerTargetSwitchingEnabled: config.isServerTargetSwitchingEnabled, + isPopupDebuggingEnabled: config.isPopupDebuggingEnabled, }) ); this.manage(this.watcher); diff --git a/devtools/server/actors/targets/target-actor-registry.jsm b/devtools/server/actors/targets/target-actor-registry.jsm index abdf26704c52..bb8841bb56b1 100644 --- a/devtools/server/actors/targets/target-actor-registry.jsm +++ b/devtools/server/actors/targets/target-actor-registry.jsm @@ -91,7 +91,8 @@ var TargetActorRegistry = { (sessionContext.type == "all" && actor.typeName === "parentProcessTarget") || (sessionContext.type == "browser-element" && - actor.browserId == sessionContext.browserId) || + (actor.browserId == sessionContext.browserId || + actor.openerBrowserId == sessionContext.browserId)) || (sessionContext.type == "webextension" && actor.addonId == sessionContext.addonId); if (isMatchingPrefix && isMatchingContext) { diff --git a/devtools/server/actors/targets/window-global.js b/devtools/server/actors/targets/window-global.js index eeeb9c61e10b..65a2e099c142 100644 --- a/devtools/server/actors/targets/window-global.js +++ b/devtools/server/actors/targets/window-global.js @@ -445,6 +445,10 @@ const windowGlobalTargetPrototype = { return this.browsingContext?.browserId; }, + get openerBrowserId() { + return this.browsingContext?.opener?.browserId; + }, + /** * Getter for the WebExtensions ContentScript globals related to the * window global's current DOM window. @@ -581,25 +585,16 @@ const windowGlobalTargetPrototype = { // created by DevTools, which always exists and help better connect resources to the target // in the frontend. Otherwise all other element of webext may be reloaded or go away // and then we would have troubles matching targets for resources. - const browsingContextID = this.devtoolsSpawnedBrowsingContextForWebExtension - ? this.devtoolsSpawnedBrowsingContextForWebExtension.id - : this.originalDocShell.browsingContext.id; - const originalInnerWindowId = this._originalWindow - ? getInnerId(this._originalWindow) - : null; - const innerWindowId = this.devtoolsSpawnedBrowsingContextForWebExtension - ? this.devtoolsSpawnedBrowsingContextForWebExtension.currentWindowGlobal - .innerWindowId - : originalInnerWindowId; - const originalParentInnerWindowId = this._originalWindow - ? this._originalWindow.docShell.browsingContext.parent - ?.currentWindowContext.innerWindowId - : null; - const parentInnerWindowId = this + const originalBrowsingContext = this .devtoolsSpawnedBrowsingContextForWebExtension - ? this.devtoolsSpawnedBrowsingContextForWebExtension.parent - .currentWindowGlobal.innerWindowId - : originalParentInnerWindowId; + ? this.devtoolsSpawnedBrowsingContextForWebExtension + : this.originalDocShell.browsingContext; + const browsingContextID = originalBrowsingContext.id; + const innerWindowId = + originalBrowsingContext.currentWindowContext.innerWindowId; + const parentInnerWindowId = + originalBrowsingContext.parent?.currentWindowContext.innerWindowId; + const isPopup = !!originalBrowsingContext.opener; const response = { actor: this.actorID, @@ -611,6 +606,7 @@ const windowGlobalTargetPrototype = { topInnerWindowId: this.browsingContext.topWindowContext.innerWindowId, isTopLevelTarget: this.isTopLevelTarget, ignoreSubFrames: this.ignoreSubFrames, + isPopup, traits: { // @backward-compat { version 64 } Exposes a new trait to help identify // BrowsingContextActor's inherited actors from the client side. diff --git a/devtools/server/actors/watcher/browsing-context-helpers.jsm b/devtools/server/actors/watcher/browsing-context-helpers.jsm index dab432e79e26..bf1c6d2eb6ff 100644 --- a/devtools/server/actors/watcher/browsing-context-helpers.jsm +++ b/devtools/server/actors/watcher/browsing-context-helpers.jsm @@ -60,6 +60,9 @@ const isEveryFrameTargetEnabled = Services.prefs.getBoolPref( * Also, there is some race conditions where browsingContext.currentWindowGlobal * is null, while the callsite may have a reference to the WindowGlobal. */ +// The goal of this method is to gather all checks done against BrowsingContext and WindowGlobal interfaces +// which leads it to be a lengthy method. So disable the complexity rule which is counter productive here. +// eslint-disable-next-line complexity function isBrowsingContextPartOfContext( browsingContext, sessionContext, @@ -122,9 +125,18 @@ function isBrowsingContextPartOfContext( return true; } if (sessionContext.type == "browser-element") { - if (browsingContext.browserId != sessionContext.browserId) { + // Check if the document is: + // - part of the Browser element, or, + // - a popup originating from the browser element (the popup being loaded in a distinct browser element) + const isMatchingTheBrowserElement = + browsingContext.browserId == sessionContext.browserId; + if ( + !isMatchingTheBrowserElement && + !isPopupToDebug(browsingContext, sessionContext) + ) { return false; } + // For client-side target switching, only mention the "remote frames". // i.e. the frames which are in a distinct process compared to their parent document // If there is no parent, this is most likely the top level document which we want to ignore. @@ -162,6 +174,33 @@ function isBrowsingContextPartOfContext( throw new Error("Unsupported session context type: " + sessionContext.type); } +/** + * Return true for popups to debug when debugging a browser-element. + * + * @param {BrowsingContext} browsingContext + * The browsing context we want to check if it is part of debugged context + * @param {Object} sessionContext + * WatcherActor's session context. This helps know what is the overall debugged scope. + * See watcher actor constructor for more info. + */ +function isPopupToDebug(browsingContext, sessionContext) { + // If enabled, create targets for popups (i.e. window.open() calls). + // If the opener is the tab we are currently debugging, accept the WindowGlobal and create a target for it. + // + // Note that it is important to do this check *after* the isInitialDocument one. + // Popups end up involving three WindowGlobals: + // - a first WindowGlobal loading an initial about:blank document (so isInitialDocument is true) + // - a second WindowGlobal which looks exactly as the first one + // - a final WindowGlobal which loads the URL passed to window.open() (so isInitialDocument is false) + // + // For now, we only instantiate a target for the last WindowGlobal. + return ( + sessionContext.isPopupDebuggingEnabled && + browsingContext.opener && + browsingContext.opener.browserId == sessionContext.browserId + ); +} + /** * Helper function of isBrowsingContextPartOfContext to execute all checks * against WindowGlobal interface which aren't specific to a given SessionContext type diff --git a/devtools/server/actors/watcher/session-context.js b/devtools/server/actors/watcher/session-context.js index 030f215afb5b..a4f99342e5d1 100644 --- a/devtools/server/actors/watcher/session-context.js +++ b/devtools/server/actors/watcher/session-context.js @@ -56,6 +56,9 @@ function createBrowserElementSessionContext(browserElement, config) { // Nowaday, it should always be enabled except for WebExtension special // codepath and some tests. isServerTargetSwitchingEnabled: config.isServerTargetSwitchingEnabled, + // Should we instantiate targets for popups opened in distinct tabs/windows? + // Driven by devtools.popups.debug=true preference. + isPopupDebuggingEnabled: config.isPopupDebuggingEnabled, }; } diff --git a/devtools/server/connectors/js-window-actor/WindowGlobalLogger.jsm b/devtools/server/connectors/js-window-actor/WindowGlobalLogger.jsm index 92d9e901ac6c..f5fabfada62c 100644 --- a/devtools/server/connectors/js-window-actor/WindowGlobalLogger.jsm +++ b/devtools/server/connectors/js-window-actor/WindowGlobalLogger.jsm @@ -54,6 +54,7 @@ const WindowGlobalLogger = { "BrowsingContext.browserId: " + browsingContext.browserId, "BrowsingContext.id: " + browsingContext.id, "innerWindowId: " + windowGlobal.innerWindowId, + "opener.id: " + browsingContext.opener?.id, "pid: " + windowGlobal.osPid, "isClosed: " + windowGlobal.isClosed, "isInProcess: " + windowGlobal.isInProcess, diff --git a/devtools/shared/commands/target/target-command.js b/devtools/shared/commands/target/target-command.js index 93bf13ad825e..b9d7d1c6e25a 100644 --- a/devtools/shared/commands/target/target-command.js +++ b/devtools/shared/commands/target/target-command.js @@ -238,10 +238,16 @@ class TargetCommand extends EventEmitter { // We only consider the top level target to be switched const isDestroyedTargetSwitching = target == this.targetFront; const isServiceWorker = target.targetType === this.TYPES.SERVICE_WORKER; + const isPopup = target.targetForm.isPopup; - // Only notify about service worker targets if this.destroyServiceWorkersOnNavigation - // is true - if (!isServiceWorker || this.destroyServiceWorkersOnNavigation) { + // Never destroy the popup targets when the top level target is destroyed + // as the popup follow a different lifecycle. + // Also avoid destroying service worker targets for similar reason, + // unless this.destroyServiceWorkersOnNavigation is true. + if ( + !isPopup && + (!isServiceWorker || this.destroyServiceWorkersOnNavigation) + ) { this._onTargetDestroyed(target, { isTargetSwitching: isDestroyedTargetSwitching, // Do not destroy service worker front as we may want to keep using it. diff --git a/devtools/shared/commands/target/tests/browser.ini b/devtools/shared/commands/target/tests/browser.ini index 2383be63fefe..92dd10651f6e 100644 --- a/devtools/shared/commands/target/tests/browser.ini +++ b/devtools/shared/commands/target/tests/browser.ini @@ -18,6 +18,7 @@ support-files = [browser_target_command_bfcache.js] [browser_target_command_browser_workers.js] [browser_target_command_detach.js] +[browser_target_command_frames_popups.js] [browser_target_command_frames_reload_server_side_targets.js] skip-if = !fission [browser_target_command_frames.js] diff --git a/devtools/shared/commands/target/tests/browser_target_command_frames_popups.js b/devtools/shared/commands/target/tests/browser_target_command_frames_popups.js new file mode 100644 index 000000000000..8d3de83be271 --- /dev/null +++ b/devtools/shared/commands/target/tests/browser_target_command_frames_popups.js @@ -0,0 +1,153 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Test that we create targets for popups + +const TEST_URL = "https://example.org/document-builder.sjs?html=main page"; +const POPUP_URL = "https://example.com/document-builder.sjs?html=popup"; +const POPUP_SECOND_URL = + "https://example.com/document-builder.sjs?html=popup-navigated"; + +add_task(async function() { + await pushPref("devtools.popups.debug", true); + + // Create a TargetCommand for a given test tab + const tab = await addTab(TEST_URL); + const commands = await CommandsFactory.forTab(tab); + const targetCommand = commands.targetCommand; + const { TYPES } = targetCommand; + + await targetCommand.startListening(); + + // Assert that watchTargets will call the create callback for all existing frames + const targets = []; + const destroyedTargets = []; + const onAvailable = ({ targetFront }) => { + targets.push(targetFront); + }; + const onDestroyed = ({ targetFront }) => { + destroyedTargets.push(targetFront); + }; + await targetCommand.watchTargets({ + types: [TYPES.FRAME], + onAvailable, + onDestroyed, + }); + + is(targets.length, 1, "At first, we only get one target"); + is( + targets[0], + targetCommand.targetFront, + "And this target is the top level one" + ); + + info("Open a popup"); + const firstPopupBrowsingContext = await SpecialPowers.spawn( + tab.linkedBrowser, + [POPUP_URL], + url => { + const win = content.open(url); + return win.browsingContext; + } + ); + + await waitFor(() => targets.length === 2); + ok(true, "We are notified about the first popup's target"); + + is( + targets[1].browsingContextID, + firstPopupBrowsingContext.id, + "the new target is for the popup" + ); + is(targets[1].url, POPUP_URL, "the new target has the right url"); + + info("Navigate the popup to a second location"); + await SpecialPowers.spawn( + firstPopupBrowsingContext, + [POPUP_SECOND_URL], + url => { + content.location.href = url; + } + ); + + await waitFor(() => targets.length === 3); + ok(true, "We are notified about the new location popup's target"); + + await waitFor(() => destroyedTargets.length === 1); + ok(true, "The first popup's target is destroyed"); + is( + destroyedTargets[0], + targets[1], + "The destroyed target is the popup's one" + ); + + is( + targets[2].browsingContextID, + firstPopupBrowsingContext.id, + "the new location target is for the popup" + ); + is( + targets[2].url, + POPUP_SECOND_URL, + "the new location target has the right url" + ); + + info("Close the popup"); + await SpecialPowers.spawn(firstPopupBrowsingContext, [], () => { + content.close(); + }); + + await waitFor(() => destroyedTargets.length === 2); + ok(true, "The popup's target is destroyed"); + is( + destroyedTargets[1], + targets[2], + "The destroyed target is the popup's one" + ); + + info("Open a about:blank popup"); + const aboutBlankPopupBrowsingContext = await SpecialPowers.spawn( + tab.linkedBrowser, + [], + () => { + const win = content.open("about:blank"); + return win.browsingContext; + } + ); + + await waitFor(() => targets.length === 4); + ok(true, "We are notified about the about:blank popup's target"); + + is( + targets[3].browsingContextID, + aboutBlankPopupBrowsingContext.id, + "the new target is for the popup" + ); + is(targets[3].url, "about:blank", "the new target has the right url"); + + info("Select the original tab and reload it"); + gBrowser.selectedTab = tab; + const onBrowserLoaded = BrowserTestUtils.browserLoaded( + gBrowser.selectedBrowser + ); + gBrowser.reloadTab(tab); + await onBrowserLoaded; + + await waitFor(() => targets.length === 5); + is(targets[4], targetCommand.targetFront, "We get a new top level target"); + ok(!targets[3].isDestroyed(), "The about:blank popup target is still alive"); + + info("Call about:blank popup method to ensure it really is functional"); + await targets[3].focus(); + + targetCommand.unwatchTargets({ + types: [TYPES.FRAME], + onAvailable, + onDestroyed, + }); + targetCommand.destroy(); + BrowserTestUtils.removeTab(tab); + await commands.destroy(); +}); diff --git a/devtools/shared/specs/descriptors/tab.js b/devtools/shared/specs/descriptors/tab.js index a8f08d1321e6..435686a05017 100644 --- a/devtools/shared/specs/descriptors/tab.js +++ b/devtools/shared/specs/descriptors/tab.js @@ -28,6 +28,7 @@ const tabDescriptorSpec = generateActorSpec({ getWatcher: { request: { isServerTargetSwitchingEnabled: Option(0, "boolean"), + isPopupDebuggingEnabled: Option(0, "boolean"), }, response: RetVal("watcher"), },