From e985b5cc50b3ad0e881ceae47913fefbaecc4164 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 31 Jul 2024 12:48:21 +0000 Subject: [PATCH] Bug 1866818 - [devtools] Enable the JS tracer on remote debugging. r=devtools-reviewers,devtools-backward-compat-reviewers,nchevobbe Display a warning message in the tracer sidebar when there could be a backward compat issue. Differential Revision: https://phabricator.services.mozilla.com/D217745 --- .../aboutdebugging/test/browser/browser.toml | 2 + ...aboutdebugging_devtoolstoolbox_jstracer.js | 79 +++++++++++++++++++ .../client/debugger/src/actions/tracing.js | 11 +++ .../client/debugger/src/client/firefox.js | 18 +++++ .../src/components/PrimaryPanes/Tracer.css | 18 +++++ .../src/components/PrimaryPanes/Tracer.js | 14 ++++ .../debugger/src/reducers/tracer-frames.js | 11 +++ .../client/debugger/src/selectors/tracer.js | 6 ++ devtools/client/definitions.js | 4 +- devtools/client/devtools-client.js | 8 ++ devtools/client/framework/toolbox.js | 3 + devtools/client/shared/test/shared-head.js | 26 +++++- .../server/actors/target-configuration.js | 2 + .../actors/targets/base-target-actor.js | 11 +++ .../actors/webconsole/commands/manager.js | 10 +-- devtools/shared/specs/target-configuration.js | 1 + devtools/shared/transport/local-transport.js | 6 ++ 17 files changed, 219 insertions(+), 11 deletions(-) create mode 100644 devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_jstracer.js diff --git a/devtools/client/aboutdebugging/test/browser/browser.toml b/devtools/client/aboutdebugging/test/browser/browser.toml index 771f58b7e3bb..8b1dddd4f3d5 100644 --- a/devtools/client/aboutdebugging/test/browser/browser.toml +++ b/devtools/client/aboutdebugging/test/browser/browser.toml @@ -108,6 +108,8 @@ fail-if = ["a11y_checks"] # Bug 1849028 clicked element may not be focusable and ["browser_aboutdebugging_devtoolstoolbox_focus.js"] +["browser_aboutdebugging_devtoolstoolbox_jstracer.js"] + ["browser_aboutdebugging_devtoolstoolbox_menubar.js"] ["browser_aboutdebugging_devtoolstoolbox_navigate_back_forward.js"] diff --git a/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_jstracer.js b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_jstracer.js new file mode 100644 index 000000000000..20633efa8ba5 --- /dev/null +++ b/devtools/client/aboutdebugging/test/browser/browser_aboutdebugging_devtoolstoolbox_jstracer.js @@ -0,0 +1,79 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +Services.scriptloader.loadSubScript( + "chrome://mochitests/content/browser/devtools/client/debugger/test/mochitest/shared-head.js", + this +); + +const TAB_URL = + "data:text/html,"; + +/* import-globals-from helper-collapsibilities.js */ +Services.scriptloader.loadSubScript( + CHROME_URL_ROOT + "helper-collapsibilities.js", + this +); + +/** + * Test JavaScript Tracer in about:devtools-toolbox tabs (ie non localTab tab target). + */ +add_task(async function () { + // This is preffed off for now, so ensure turning it on + await pushPref("devtools.debugger.features.javascript-tracing", true); + + const testTab = await addTab(TAB_URL); + + info("Force all debug target panes to be expanded"); + prepareCollapsibilitiesTest(); + + const { document, tab, window } = await openAboutDebugging(); + await selectThisFirefoxPage(document, window.AboutDebugging.store); + const { devtoolsTab, devtoolsWindow } = await openAboutDevtoolsToolbox( + document, + tab, + window, + TAB_URL + ); + info("Select performance panel"); + const toolbox = getToolbox(devtoolsWindow); + await toolbox.selectTool("jsdebugger"); + + info("Add a breakpoint at line 10 in the test script"); + const debuggerContext = createDebuggerContext(toolbox); + + await toggleJsTracerMenuItem( + debuggerContext, + "#jstracer-menu-item-debugger-sidebar" + ); + + await toggleJsTracer(toolbox); + + info("Invoke some code that will be traced"); + await ContentTask.spawn(testTab.linkedBrowser, {}, function () { + content.wrappedJSObject.foo(); + }); + + info("Wait for the expected traces to appear in the call tree"); + const tree = await waitForElementWithSelector( + debuggerContext, + "#tracer-tab-panel .tree" + ); + const traces = await waitFor(() => { + const elements = tree.querySelectorAll( + ".trace-line .frame-link-function-display-name" + ); + if (elements.length == 2) { + return elements; + } + return false; + }); + is(traces[0].textContent, "λ foo"); + is(traces[1].textContent, "λ bar"); + + await closeAboutDevtoolsToolbox(document, devtoolsTab, window); + await removeTab(testTab); + await removeTab(tab); +}); diff --git a/devtools/client/debugger/src/actions/tracing.js b/devtools/client/debugger/src/actions/tracing.js index 8cad9f111cfc..1e80b97fa8fe 100644 --- a/devtools/client/debugger/src/actions/tracing.js +++ b/devtools/client/debugger/src/actions/tracing.js @@ -67,3 +67,14 @@ export function selectTrace(traceIndex) { ); }; } + +export function setLocalAndRemoteRuntimeVersion( + localPlatformVersion, + remotePlatformVersion +) { + return { + type: "SET_RUNTIME_VERSIONS", + localPlatformVersion, + remotePlatformVersion, + }; +} diff --git a/devtools/client/debugger/src/client/firefox.js b/devtools/client/debugger/src/client/firefox.js index 77ae6bbe2233..1e60cde304cd 100644 --- a/devtools/client/debugger/src/client/firefox.js +++ b/devtools/client/debugger/src/client/firefox.js @@ -11,6 +11,9 @@ import sourceQueue from "../utils/source-queue"; const { TRACER_LOG_METHODS, } = require("resource://devtools/shared/specs/tracer.js"); +const { AppConstants } = ChromeUtils.importESModule( + "resource://gre/modules/AppConstants.sys.mjs" +); let actions; let commands; @@ -94,6 +97,15 @@ export async function onConnect(_commands, _resourceCommand, _actions, store) { // to be able to clear the tracer data on tracing start, that, even if the // tracer is waiting for next interation/load. commands.tracerCommand.on("toggle", onTracingToggled); + + if (!commands.client.isLocalClient) { + const localPlatformVersion = AppConstants.MOZ_APP_VERSION; + const remotePlatformVersion = await getRemotePlatformVersion(); + actions.setLocalAndRemoteRuntimeVersion( + localPlatformVersion, + remotePlatformVersion + ); + } } export function onDisconnect() { @@ -238,4 +250,10 @@ function onDocumentEventAvailable(events) { } } +async function getRemotePlatformVersion() { + const deviceFront = await commands.client.mainRoot.getFront("device"); + const description = await deviceFront.getDescription(); + return description.platformversion; +} + export { clientCommands }; diff --git a/devtools/client/debugger/src/components/PrimaryPanes/Tracer.css b/devtools/client/debugger/src/components/PrimaryPanes/Tracer.css index 7037ae0a0626..7bd6e0ba6f19 100644 --- a/devtools/client/debugger/src/components/PrimaryPanes/Tracer.css +++ b/devtools/client/debugger/src/components/PrimaryPanes/Tracer.css @@ -57,6 +57,24 @@ fill: var(--theme-icon-checked-color); } +.tracer-toolbar .tracer-runtime-version-mismatch { + --icon-size: 16px; + --icon-inline-padding: 4px; + + background-color: var(--theme-warning-background); + color: var(--theme-warning-color); + border-block-end: 1px solid var(--theme-splitter-color); + padding: 1em; + padding-inline-start: calc(var(--icon-inline-padding) * 2 + var(--icon-size)); + background-image: url(resource://devtools-shared-images/alert-small.svg); + background-position-x: var(--icon-inline-padding); + background-position-y: 50%; + background-repeat: no-repeat; + background-size: var(--icon-size); + -moz-context-properties: fill; + fill: var(--theme-warning-color); +} + .tracer-tab .tracer-message { display: flex; justify-content: center; diff --git a/devtools/client/debugger/src/components/PrimaryPanes/Tracer.js b/devtools/client/debugger/src/components/PrimaryPanes/Tracer.js index c98dad6963d7..bf373faa5ca8 100644 --- a/devtools/client/debugger/src/components/PrimaryPanes/Tracer.js +++ b/devtools/client/debugger/src/components/PrimaryPanes/Tracer.js @@ -20,6 +20,7 @@ import { getAllMutationTraces, getAllTraceCount, getIsCurrentlyTracing, + getRuntimeVersions, } from "../../selectors/index"; const VirtualizedTree = require("resource://devtools/client/shared/components/VirtualizedTree.js"); const FrameView = createFactory( @@ -721,6 +722,8 @@ export class Tracer extends Component { render() { const isZoomed = this.state.renderedTraceCount != this.props.traceCount; + const { runtimeVersions } = this.props; + return div( { className: "tracer-container", @@ -738,6 +741,16 @@ export class Tracer extends Component { "This panel is experimental. It may change, regress, be dropped or replaced." ) : null, + runtimeVersions && + runtimeVersions.localPlatformVersion != + runtimeVersions.remotePlatformVersion + ? div( + { + className: "tracer-runtime-version-mismatch", + }, + `Client and remote runtime have different versions (${runtimeVersions.localPlatformVersion} vs ${runtimeVersions.remotePlatformVersion}) . The Tracer may be broken because of protocol changes between these two versions. Please upgrade or downgrade one of the two to use the same major version.` + ) + : null, this.renderSearchInput() ), isZoomed @@ -830,6 +843,7 @@ const mapStateToProps = state => { mutationTraces: getAllMutationTraces(state), traceCount: getAllTraceCount(state), selectedTraceIndex: getSelectedTraceIndex(state), + runtimeVersions: getRuntimeVersions(state), }; }; diff --git a/devtools/client/debugger/src/reducers/tracer-frames.js b/devtools/client/debugger/src/reducers/tracer-frames.js index 6a76c29871ed..46eee9df5a02 100644 --- a/devtools/client/debugger/src/reducers/tracer-frames.js +++ b/devtools/client/debugger/src/reducers/tracer-frames.js @@ -30,6 +30,10 @@ function initialState() { // Index of the currently selected trace within `mutableTraces`. selectedTraceIndex: null, + + // Runtime versions to help show warning when there is a mismatch between frontend and backend versions + localPlatformVersion: null, + remotePlatformVersion: null, }; } @@ -127,6 +131,13 @@ function update(state = initialState(), action) { selectedTrace: null, }; } + case "SET_RUNTIME_VERSIONS": { + return { + ...state, + localPlatformVersion: action.localPlatformVersion, + remotePlatformVersion: action.remotePlatformVersion, + }; + } } return state; } diff --git a/devtools/client/debugger/src/selectors/tracer.js b/devtools/client/debugger/src/selectors/tracer.js index 2523a16b68cf..c01b51b77979 100644 --- a/devtools/client/debugger/src/selectors/tracer.js +++ b/devtools/client/debugger/src/selectors/tracer.js @@ -26,3 +26,9 @@ export function getAllMutationTraces(state) { export function getAllTraceCount(state) { return state.tracerFrames?.mutableTraces.length || 0; } +export function getRuntimeVersions(state) { + return { + localPlatformVersion: state.tracerFrames?.localPlatformVersion, + remotePlatformVersion: state.tracerFrames?.remotePlatformVersion, + }; +} diff --git a/devtools/client/definitions.js b/devtools/client/definitions.js index eab95533dbc6..b9db99eb504b 100644 --- a/devtools/client/definitions.js +++ b/devtools/client/definitions.js @@ -619,9 +619,7 @@ exports.ToolboxButtons = [ "toolbox.buttons.jstracer", osString == "Darwin" ? "Cmd+Shift+5" : "Ctrl+Shift+5" ), - isToolSupported: toolbox => - (toolbox.commands.descriptorFront.isLocalTab || - toolbox.isBrowserToolbox) && + isToolSupported: () => Services.prefs.getBoolPref( "devtools.debugger.features.javascript-tracing", false diff --git a/devtools/client/devtools-client.js b/devtools/client/devtools-client.js index fb4a5a99891b..4ed2cbfc1a0e 100644 --- a/devtools/client/devtools-client.js +++ b/devtools/client/devtools-client.js @@ -792,6 +792,14 @@ DevToolsClient.prototype = { return this._transport; }, + /** + * Boolean flag to help identify client connected to the current runtime, + * via a LocalDevToolsTransport pipe. + */ + get isLocalClient() { + return !!this._transport.isLocalTransport; + }, + dumpPools() { for (const pool of this._pools) { console.log(`%c${pool.actorID}`, "font-weight: bold;", [ diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index 0ffaec4b934f..852b493129f9 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -241,6 +241,9 @@ const BOOLEAN_CONFIGURATION_PREFS = { name: "pauseOverlay", thread: true, }, + "devtools.debugger.features.javascript-tracing": { + name: "isTracerFeatureEnabled", + }, }; /** diff --git a/devtools/client/shared/test/shared-head.js b/devtools/client/shared/test/shared-head.js index 1c350557c13b..211607afd4eb 100644 --- a/devtools/client/shared/test/shared-head.js +++ b/devtools/client/shared/test/shared-head.js @@ -2392,12 +2392,36 @@ async function unregisterServiceWorker(workerUrl) { * Toggle the JavavaScript tracer via its toolbox toolbar button. */ async function toggleJsTracer(toolbox) { - const { isTracingEnabled } = toolbox.commands.tracerCommand; + const { tracerCommand } = toolbox.commands; + const { isTracingEnabled } = tracerCommand; const { logMethod, traceOnNextInteraction, traceOnNextLoad } = toolbox.commands.tracerCommand.getTracingOptions(); + + // When the tracer is waiting for user interaction or page load, it won't be made active + // right away. The test should manually wait for its activation. + const shouldWaitForToggle = !traceOnNextInteraction && !traceOnNextLoad; + let onTracingToggled; + if (shouldWaitForToggle) { + onTracingToggled = new Promise(resolve => { + tracerCommand.on("toggle", async function listener() { + // Ignore the event, if we are still in the same state as before the click + if (tracerCommand.isTracingActive == isTracingEnabled) { + return; + } + tracerCommand.off("toggle", listener); + resolve(); + }); + }); + } + const toolbarButton = toolbox.doc.getElementById("command-button-jstracer"); toolbarButton.click(); + if (shouldWaitForToggle) { + info("Waiting for the tracer to be active"); + await onTracingToggled; + } + const { TRACER_LOG_METHODS, } = require("resource://devtools/shared/specs/tracer.js"); diff --git a/devtools/server/actors/target-configuration.js b/devtools/server/actors/target-configuration.js index e739c1cc3def..ca1f4ad75027 100644 --- a/devtools/server/actors/target-configuration.js +++ b/devtools/server/actors/target-configuration.js @@ -31,6 +31,8 @@ const SUPPORTED_OPTIONS = { customFormatters: true, // Set a custom user agent customUserAgent: true, + // Is the tracer experimental feature manually enabled by the user? + isTracerFeatureEnabled: true, // Enable JavaScript javascriptEnabled: true, // Force a custom device pixel ratio (used in RDM). Set to null to restore origin ratio. diff --git a/devtools/server/actors/targets/base-target-actor.js b/devtools/server/actors/targets/base-target-actor.js index 9e364130fc12..88b3a763a3fb 100644 --- a/devtools/server/actors/targets/base-target-actor.js +++ b/devtools/server/actors/targets/base-target-actor.js @@ -224,6 +224,14 @@ class BaseTargetActor extends Actor { return this.#instantiatedTargetScopedActors.has(prefix); } + /** + * Server side boolean to know if the tracer has been enabled by the user. + * + * By enabled, we mean the feature has been exposed to the user, + * not that the tracer is actively tracing executions. + */ + isTracerFeatureEnabled = false; + /** * Apply target-specific options. * @@ -238,6 +246,9 @@ class BaseTargetActor extends Actor { * actor is instantiated. */ updateTargetConfiguration(options = {}, calledFromDocumentCreation = false) { + if (typeof options.isTracerFeatureEnabled === "boolean") { + this.isTracerFeatureEnabled = options.isTracerFeatureEnabled; + } // If there is some tracer options, we should start tracing, otherwise we should stop (if we were) if (options.tracerOptions) { // Ignore the SessionData update if the user requested to start the tracer on next page load and: diff --git a/devtools/server/actors/webconsole/commands/manager.js b/devtools/server/actors/webconsole/commands/manager.js index e96e0a617fb4..d025ecd15661 100644 --- a/devtools/server/actors/webconsole/commands/manager.js +++ b/devtools/server/actors/webconsole/commands/manager.js @@ -866,16 +866,12 @@ WebConsoleCommandsManager.register({ name: "trace", isSideEffectFree: false, command(owner, args) { + // Disable :trace command on worker until this feature is enabled by default if (isWorker) { throw new Error(":trace command isn't supported in workers"); } - // Disable :trace command on worker until this feature is enabled by default - if ( - !Services.prefs.getBoolPref( - "devtools.debugger.features.javascript-tracing", - false - ) - ) { + + if (!owner.consoleActor.parentActor.isTracerFeatureEnabled) { throw new Error( ":trace requires 'devtools.debugger.features.javascript-tracing' preference to be true" ); diff --git a/devtools/shared/specs/target-configuration.js b/devtools/shared/specs/target-configuration.js index 8db15ff77dfa..7567976f7512 100644 --- a/devtools/shared/specs/target-configuration.js +++ b/devtools/shared/specs/target-configuration.js @@ -25,6 +25,7 @@ types.addDictType("target-configuration.configuration", { serviceWorkersTestingEnabled: "nullable:boolean", setTabOffline: "nullable:boolean", touchEventsOverride: "nullable:string", + isTracerFeatureEnabled: "nullable:boolean", }); const targetConfigurationSpec = generateActorSpec({ diff --git a/devtools/shared/transport/local-transport.js b/devtools/shared/transport/local-transport.js index bff83b766648..99df74c90ebe 100644 --- a/devtools/shared/transport/local-transport.js +++ b/devtools/shared/transport/local-transport.js @@ -35,6 +35,12 @@ function LocalDebuggerTransport(other) { } LocalDebuggerTransport.prototype = { + /** + * Boolean to help identify DevToolsClient instances connected to a LocalDevToolsTransport pipe + * and so connected to the same runtime as the frontend. + */ + isLocalTransport: true, + /** * Transmit a message by directly calling the onPacket handler of the other * endpoint.