From 74ea826464f2eced35e5d14f79a64c689085bb63 Mon Sep 17 00:00:00 2001 From: Alex Hunt Date: Tue, 6 Feb 2024 04:09:53 -0800 Subject: [PATCH] Add nativeSourceCodeFetching capability flag (#42818) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42818 ## Context We're introducing the concept of **capability flags** to provide granular control of behaviours in the Inspector Proxy, to replace the recently added `type: 'Legacy' | 'Modern'` target switch. A capability flag disables a specific feature/hack in the Inspector Proxy layer by indicating that the target supports one or more modern CDP features. ## This diff Implements a second granular flag, `nativeSourceCodeFetching`, and adds tests for this. Changelog: [Internal] Reviewed By: robhogan Differential Revision: D53352242 fbshipit-source-id: 94b62d84c731c903c5f99f8206d5c91bc501d030 --- .../InspectorProxyCdpRewritingHacks-test.js | 73 +++++++++++++++++++ .../src/inspector-proxy/Device.js | 30 ++++++-- .../src/inspector-proxy/cdp-types/messages.js | 7 ++ .../src/inspector-proxy/cdp-types/protocol.js | 26 ++++++- .../src/inspector-proxy/types.js | 7 ++ 5 files changed, 132 insertions(+), 11 deletions(-) diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js index 240046a53eb..08327991c3c 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyCdpRewritingHacks-test.js @@ -9,6 +9,8 @@ * @oncall react_native */ +import type {TargetCapabilityFlags} from '../inspector-proxy/types'; + import {allowSelfSignedCertsInNodeFetch} from './FetchUtils'; import { createAndConnectTarget, @@ -472,5 +474,76 @@ describe.each(['HTTP', 'HTTPS'])( }, ); }); + + describe.each([ + ['for modern targets', {}], + [ + "when target has 'nativeSourceMapFetching' capability flag", + {nativeSourceMapFetching: true}, + ], + ])('disabled %s', (_, capabilities: TargetCapabilityFlags) => { + const pageDescription = { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + type: 'Modern', + capabilities, + vm: 'bar-vm', + }; + + describe('Debugger.scriptParsed', () => { + test('should forward event directly to client (does not rewrite sourceMapURL host)', async () => { + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + pageDescription, + ); + try { + const message = { + method: 'Debugger.scriptParsed', + params: { + sourceMapURL: `${protocol.toLowerCase()}://10.0.2.2:${ + serverRef.port + }/source-map`, + }, + }; + await sendFromTargetToDebugger(device, debugger_, 'page1', message); + + expect(debugger_.handle).toBeCalledWith(message); + } finally { + device.close(); + debugger_.close(); + } + }); + }); + + describe('Debugger.getScriptSource', () => { + test('should forward request directly to device (does not read source from disk in proxy)', async () => { + const {device, debugger_} = await createAndConnectTarget( + serverRef, + autoCleanup.signal, + pageDescription, + ); + try { + const message = { + id: 1, + method: 'Debugger.getScriptSource', + params: { + scriptId: 'script1', + }, + }; + await sendFromDebuggerToTarget(debugger_, device, 'page1', message); + + expect(device.wrappedEventParsed).toBeCalledWith({ + pageId: 'page1', + wrappedEvent: message, + }); + } finally { + device.close(); + debugger_.close(); + } + }); + }); + }); }, ); diff --git a/packages/dev-middleware/src/inspector-proxy/Device.js b/packages/dev-middleware/src/inspector-proxy/Device.js index b95be9f5efd..50bd3f70c4f 100644 --- a/packages/dev-middleware/src/inspector-proxy/Device.js +++ b/packages/dev-middleware/src/inspector-proxy/Device.js @@ -14,6 +14,7 @@ import type { CDPClientMessage, CDPRequest, CDPResponse, + CDPServerMessage, } from './cdp-types/messages'; import type { MessageFromDevice, @@ -231,8 +232,8 @@ export default class Device { frontendUserAgent: metadata.userAgent, }); let processedReq = debuggerRequest; - if (!page || page.type === 'Legacy') { - processedReq = this.#interceptMessageFromDebuggerLegacy( + if (!page || !this.#pageHasCapability(page, 'nativeSourceCodeFetching')) { + processedReq = this.#interceptClientMessageForSourceFetching( debuggerRequest, debuggerInfo, socket, @@ -419,6 +420,7 @@ export default class Device { this.#processMessageFromDeviceLegacy( parsedPayload, this.#debuggerConnection, + pageId, ).then(() => { const messageToSend = JSON.stringify(parsedPayload); debuggerSocket.send(messageToSend); @@ -499,16 +501,27 @@ export default class Device { // Allows to make changes in incoming message from device. async #processMessageFromDeviceLegacy( - payload: {method: string, params: {sourceMapURL: string, url: string}}, + payload: CDPServerMessage, debuggerInfo: DebuggerInfo, + pageId: ?string, ) { + // TODO(moti): Handle null case explicitly, or ideally associate a copy + // of the page metadata object with the connection so this can never be + // null. + const page: ?Page = pageId != null ? this.#pages.get(pageId) : null; + // Replace Android addresses for scriptParsed event. - if (payload.method === 'Debugger.scriptParsed') { - const params = payload.params || {}; + if ( + (!page || !this.#pageHasCapability(page, 'nativeSourceCodeFetching')) && + payload.method === 'Debugger.scriptParsed' && + payload.params != null + ) { + const params = payload.params; if ('sourceMapURL' in params) { for (let i = 0; i < EMULATOR_LOCALHOST_ADDRESSES.length; ++i) { const address = EMULATOR_LOCALHOST_ADDRESSES[i]; - if (params.sourceMapURL.indexOf(address) >= 0) { + if (params.sourceMapURL.includes(address)) { + // $FlowFixMe[cannot-write] payload.params.sourceMapURL = params.sourceMapURL.replace( address, 'localhost', @@ -526,6 +539,7 @@ export default class Device { // message to the debug client. try { const sourceMap = await this.#fetchText(sourceMapURL); + // $FlowFixMe[cannot-write] payload.params.sourceMapURL = 'data:application/json;charset=utf-8;base64,' + new Buffer(sourceMap).toString('base64'); @@ -540,6 +554,7 @@ export default class Device { for (let i = 0; i < EMULATOR_LOCALHOST_ADDRESSES.length; ++i) { const address = EMULATOR_LOCALHOST_ADDRESSES[i]; if (params.url.indexOf(address) >= 0) { + // $FlowFixMe[cannot-write] payload.params.url = params.url.replace(address, 'localhost'); debuggerInfo.originalSourceURLAddress = address; } @@ -550,6 +565,7 @@ export default class Device { // Chrome to not download source maps. In this case we want to prepend script ID // with 'file://' prefix. if (payload.params.url.match(/^[0-9a-z]+$/)) { + // $FlowFixMe[cannot-write] payload.params.url = FILE_PREFIX + payload.params.url; debuggerInfo.prependedFilePrefix = true; } @@ -601,7 +617,7 @@ export default class Device { * original/replacement CDP message object, or `null` (will forward nothing * to the target). */ - #interceptMessageFromDebuggerLegacy( + #interceptClientMessageForSourceFetching( req: CDPClientMessage, debuggerInfo: DebuggerInfo, socket: WS, diff --git a/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js b/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js index 3743e9082c9..79eb1458a99 100644 --- a/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js +++ b/packages/dev-middleware/src/inspector-proxy/cdp-types/messages.js @@ -41,5 +41,12 @@ export type CDPRequestError = $ReadOnly<{ export type CDPClientMessage = | CDPRequest<'Debugger.getScriptSource'> + | CDPRequest<'Debugger.scriptParsed'> | CDPRequest<'Debugger.setBreakpointByUrl'> | CDPRequest<>; + +export type CDPServerMessage = + | CDPEvent<'Debugger.scriptParsed'> + | CDPEvent<> + | CDPResponse<'Debugger.getScriptSource'> + | CDPResponse<>; diff --git a/packages/dev-middleware/src/inspector-proxy/cdp-types/protocol.js b/packages/dev-middleware/src/inspector-proxy/cdp-types/protocol.js index aed9fe31bf3..4b7500c4070 100644 --- a/packages/dev-middleware/src/inspector-proxy/cdp-types/protocol.js +++ b/packages/dev-middleware/src/inspector-proxy/cdp-types/protocol.js @@ -11,6 +11,8 @@ // Adapted from https://github.com/ChromeDevTools/devtools-protocol/blob/master/types/protocol.d.ts +type integer = number; + export interface Debugger { GetScriptSourceParams: $ReadOnly<{ /** @@ -35,7 +37,7 @@ export interface Debugger { /** * Line number to set breakpoint at. */ - lineNumber: number, + lineNumber: integer, /** * URL of the resources to set breakpoint on. @@ -56,7 +58,7 @@ export interface Debugger { /** * Offset in the line to set breakpoint at. */ - columnNumber?: number, + columnNumber?: integer, /** * Expression to use as a breakpoint condition. When specified, debugger will only stop on the @@ -64,9 +66,27 @@ export interface Debugger { */ condition?: string, }>; + + ScriptParsedEvent: $ReadOnly<{ + /** + * Identifier of the script parsed. + */ + scriptId: string, + + /** + * URL or name of the script parsed (if any). + */ + url: string, + + /** + * URL of source map associated with script (if any). + */ + sourceMapURL: string, + }>; } export type Events = { + 'Debugger.scriptParsed': Debugger['ScriptParsedEvent'], [method: string]: mixed, }; @@ -75,12 +95,10 @@ export type Commands = { paramsType: Debugger['GetScriptSourceParams'], resultType: Debugger['GetScriptSourceResult'], }, - 'Debugger.setBreakpointByUrl': { paramsType: Debugger['SetBreakpointByUrlParams'], resultType: void, }, - [method: string]: { paramsType: mixed, resultType: mixed, diff --git a/packages/dev-middleware/src/inspector-proxy/types.js b/packages/dev-middleware/src/inspector-proxy/types.js index ed65350f943..7ff48f1a83d 100644 --- a/packages/dev-middleware/src/inspector-proxy/types.js +++ b/packages/dev-middleware/src/inspector-proxy/types.js @@ -23,6 +23,13 @@ export type TargetCapabilityFlags = $ReadOnly<{ * In the launch flow, this allows targets to be matched directly by `appId`. */ nativePageReloads?: boolean, + + /** + * The target supports fetching source code and source maps. + * + * In the proxy, this disables source fetching emulation and host rewrites. + */ + nativeSourceCodeFetching?: boolean, }>; // Page information received from the device. New page is created for