From bf8e3bda617e1cc98c2daf33ef8868bbfd019521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Z=C3=BCnd?= Date: Wed, 1 Feb 2023 10:59:54 +0100 Subject: [PATCH] [linkifier] Add option to reveal the breakpoint edit dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL adds a new configuration option when we linkify script locations. If the location we want to linkify also has breakpoint set, then we can optionally reveal the breakpoint edit dialog (after also jumping to the corresponding source location). This functionality will be used to polish the UX around logpoints and conditional breakpoints. R=kimanh@chromium.org Bug: 1027458 Change-Id: I593d250e5b270a4c278afe1665ee023a36e836a6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4207834 Commit-Queue: Simon Zünd Reviewed-by: Kim-Anh Tran --- .../ui/legacy/components/utils/Linkifier.ts | 51 +++++++++++++++- .../legacy/components/utils/Linkifier_test.ts | 58 ++++++++++++++++++- 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/front_end/ui/legacy/components/utils/Linkifier.ts b/front_end/ui/legacy/components/utils/Linkifier.ts index d3e1a3525a..10ef77a3a5 100644 --- a/front_end/ui/legacy/components/utils/Linkifier.ts +++ b/front_end/ui/legacy/components/utils/Linkifier.ts @@ -161,6 +161,33 @@ export class Linkifier implements SDK.TargetManager.Observer { } } + /** + * When we link to a breakpoint condition, we need to stash the BreakpointLocation as the revealable + * in the LinkInfo. + */ + private static bindBreakpoint(anchor: Element, uiLocation: Workspace.UISourceCode.UILocation): void { + const info = Linkifier.linkInfo(anchor); + if (!info) { + return; + } + + const breakpoint = Bindings.BreakpointManager.BreakpointManager.instance().findBreakpoint(uiLocation); + if (breakpoint) { + info.revealable = breakpoint; + } + } + + /** + * When we link to a breakpoint condition, we store the BreakpointLocation in the revealable. + * Clear it when the LiveLocation updates. + */ + private static unbindBreakpoint(anchor: Element): void { + const info = Linkifier.linkInfo(anchor); + if (info && info.revealable) { + info.revealable = null; + } + } + targetAdded(target: SDK.Target.Target): void { this.anchorsByTarget.set(target, []); this.locationPoolByTarget.set(target, new Bindings.LiveLocation.LiveLocationPool()); @@ -242,7 +269,10 @@ export class Linkifier implements SDK.TargetManager.Observer { return fallbackAnchor; } - const linkDisplayOptions = {showColumnNumber: linkifyURLOptions.showColumnNumber}; + const linkDisplayOptions: LinkDisplayOptions = { + showColumnNumber: linkifyURLOptions.showColumnNumber, + revealBreakpoint: options?.revealBreakpoint, + }; const currentOnLiveLocationUpdate = this.onLiveLocationUpdate; void Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance() @@ -400,6 +430,9 @@ export class Linkifier implements SDK.TargetManager.Observer { anchor: HTMLElement, options: LinkDisplayOptions, liveLocation: Bindings.LiveLocation.LiveLocation): Promise { Linkifier.unbindUILocation(anchor); + if (options.revealBreakpoint) { + Linkifier.unbindBreakpoint(anchor); + } const uiLocation = await liveLocation.uiLocation(); if (!uiLocation) { if (liveLocation instanceof Bindings.CSSWorkspaceBinding.LiveLocation) { @@ -421,6 +454,9 @@ export class Linkifier implements SDK.TargetManager.Observer { } Linkifier.bindUILocation(anchor, uiLocation); + if (options.revealBreakpoint) { + Linkifier.bindBreakpoint(anchor, uiLocation); + } const text = uiLocation.linkText(true /* skipTrim */, options.showColumnNumber); Linkifier.setTrimmedText(anchor, text, this.maxLength); @@ -967,6 +1003,11 @@ export interface LinkifyOptions { showColumnNumber?: boolean; inlineFrameIndex: number; tabStop?: boolean; + + /** + * {@link LinkDisplayOptions.revealBreakpoint} + */ + revealBreakpoint?: boolean; } // TODO(crbug.com/1172300) Ignored during the jsdoc to ts migration @@ -982,6 +1023,14 @@ export interface _CreateLinkOptions { interface LinkDisplayOptions { showColumnNumber: boolean; + + /** + * If true, we'll check if there is a breakpoint at the UILocation we get + * from the LiveLocation. If we find a breakpoint, we'll reveal the corresponding + * {@link Bindings.BreakpointManager.BreakpointLocation}. Which opens the + * breakpoint edit dialog. + */ + revealBreakpoint?: boolean; } export type LinkHandler = (arg0: TextUtils.ContentProvider.ContentProvider, arg1: number) => void; diff --git a/test/unittests/front_end/ui/legacy/components/utils/Linkifier_test.ts b/test/unittests/front_end/ui/legacy/components/utils/Linkifier_test.ts index 1d80a26e8f..e15b10b2dc 100644 --- a/test/unittests/front_end/ui/legacy/components/utils/Linkifier_test.ts +++ b/test/unittests/front_end/ui/legacy/components/utils/Linkifier_test.ts @@ -12,6 +12,7 @@ import type * as Protocol from '../../../../../../../front_end/generated/protoco import {createTarget} from '../../../../helpers/EnvironmentHelpers.js'; import {describeWithMockConnection, dispatchEvent} from '../../../../helpers/MockConnection.js'; import {assertNotNullOrUndefined} from '../../../../../../../front_end/core/platform/platform.js'; +import {MockProtocolBackend} from '../../../../helpers/MockScopeChain.js'; const {assert} = chai; @@ -19,6 +20,13 @@ const scriptId1 = '1' as Protocol.Runtime.ScriptId; const scriptId2 = '2' as Protocol.Runtime.ScriptId; const executionContextId = 1234 as Protocol.Runtime.ExecutionContextId; +const simpleScriptContent = ` +function foo(x) { + const y = x + 3; + return y; +} +`; + describeWithMockConnection('Linkifier', async () => { let SDK: typeof SDKModule; let Components: typeof ComponentsModule; @@ -46,7 +54,10 @@ describeWithMockConnection('Linkifier', async () => { targetManager, }); Bindings.IgnoreListManager.IgnoreListManager.instance({forceNew, debuggerWorkspaceBinding}); - return {target, linkifier}; + Bindings.BreakpointManager.BreakpointManager.instance( + {forceNew, targetManager, workspace, debuggerWorkspaceBinding}); + const backend = new MockProtocolBackend(); + return {target, linkifier, backend}; } describe('Linkifier.linkifyURL', () => { @@ -278,4 +289,49 @@ describeWithMockConnection('Linkifier', async () => { assertNotNullOrUndefined(anchor); assert.strictEqual(anchor.textContent, `w.com/a.js:${lineNumber + 1}`); }); + + describe('maybeLinkifyScriptLocation', () => { + it('uses the BreakLocation as a revealable if the option is provided and a breakpoint is at the given location', + async () => { + const {target, linkifier, backend} = setUpEnvironment(); + const breakpointManager = Bindings.BreakpointManager.BreakpointManager.instance(); + const debuggerWorkspaceBinding = Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance(); + const lineNumber = 1; + const columnNumber = 0; + const url = 'https://www.google.com/script.js' as Platform.DevToolsPath.UrlString; + + const script = await backend.addScript(target, {content: simpleScriptContent, url}, null); + const uiSourceCode = debuggerWorkspaceBinding.uiSourceCodeForScript(script); + assertNotNullOrUndefined(uiSourceCode); + + const responder = backend.responderToBreakpointByUrlRequest(url, lineNumber); + void responder({ + breakpointId: 'BREAK_ID' as Protocol.Debugger.BreakpointId, + locations: [ + { + scriptId: script.scriptId, + lineNumber, + columnNumber, + }, + ], + }); + const breakpoint = await breakpointManager.setBreakpoint( + uiSourceCode, lineNumber, columnNumber, 'x' as BindingsModule.BreakpointManager.UserCondition, + /* enabled */ true, /* isLogpoint */ true, Bindings.BreakpointManager.BreakpointOrigin.USER_ACTION); + + // Create a link that matches exactly the breakpoint location. + const anchor = linkifier.maybeLinkifyScriptLocation( + target, script.scriptId, url, lineNumber, {inlineFrameIndex: 0, revealBreakpoint: true}); + assertNotNullOrUndefined(anchor); + + await debuggerWorkspaceBinding.pendingLiveLocationChangesPromise(); + + // Assert that the linkinfo has the `BreakLocation` as its revealable. + // When clicking the link, `revealables` have predecence over e.g. the + // UILocation or url. + const linkInfo = Components.Linkifier.Linkifier.linkInfo(anchor); + assertNotNullOrUndefined(linkInfo); + assert.propertyVal(linkInfo.revealable, 'breakpoint', breakpoint); + }); + }); });