chore: cleanup custom breakpoint ui (microsoft#1865)

Uses a manually controlled checkbox list, fixes some state erros. Also
handle the recently-deprecated DOMDebugger.setInstrumentationBreakpoint API.

package.nls.json

@@ -1,5 +1,5 @@
 {
-  "add.eventListener.breakpoint": "Add Event Listener Breakpoint",
+  "add.eventListener.breakpoint": "Toggle Event Listener Breakpoints",
   "attach.node.process": "Attach to Node Process",
   "base.cascadeTerminateToConfigurations.label": "A list of debug sessions which, when this debug session is terminated, will also be stopped.",
   "base.enableDWARF.label": "Toggles whether the debugger will try to read DWARF debug symbols from WebAssembly, which can be resource intensive. Requires the `ms-vscode.wasm-dwarf-debugging` extension to function.",

src/adapter/customBreakpoints.ts

@@ -62,14 +62,25 @@ export function customBreakpoints(): Map<string, ICustomBreakpoint> {
         };
       },
       apply: async (cdp: Cdp.Api, enabled: boolean): Promise<boolean> => {
-        if (enabled)
-          return !!(await cdp.DOMDebugger.setInstrumentationBreakpoint({
-            eventName: instrumentation,
-          }));
-        else
-          return !!(await cdp.DOMDebugger.removeInstrumentationBreakpoint({
-            eventName: instrumentation,
-          }));
+        // DOMDebugger.setInstrumentationBreakpoint was very recently deprecated,
+        // try the old method as a fallback.
+        const ok1 = enabled
+          ? await cdp.EventBreakpoints.setInstrumentationBreakpoint({ eventName: instrumentation })
+          : await cdp.EventBreakpoints.removeInstrumentationBreakpoint({
+              eventName: instrumentation,
+            });
+
+        if (ok1) {
+          return true;
+        }
+
+        const ok2 = enabled
+          ? await cdp.DOMDebugger.setInstrumentationBreakpoint({ eventName: instrumentation })
+          : await cdp.DOMDebugger.removeInstrumentationBreakpoint({
+              eventName: instrumentation,
+            });
+
+        return !!ok2;
       },
     };
   }

src/adapter/debugAdapter.ts

@@ -48,7 +48,7 @@ export class DebugAdapter implements IDisposable {
   readonly sourceContainer: SourceContainer;
   readonly breakpointManager: BreakpointManager;
   private _disposables = new DisposableList();
-  private _customBreakpoints = new Set<string>();
+  private _customBreakpoints: string[] = [];
   private _thread: Thread | undefined;
   private _threadDeferred = getDeferred<Thread>();
   private _configurationDoneDeferred: IDeferred<void>;
@@ -101,9 +101,8 @@ export class DebugAdapter implements IDisposable {
     this.dap.on('evaluate', params => this.onEvaluate(params));
     this.dap.on('completions', params => this._withThread(thread => thread.completions(params)));
     this.dap.on('exceptionInfo', () => this._withThread(thread => thread.exceptionInfo()));
-    this.dap.on('enableCustomBreakpoints', params => this.enableCustomBreakpoints(params));
+    this.dap.on('setCustomBreakpoints', params => this.setCustomBreakpoints(params));
     this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params));
-    this.dap.on('disableCustomBreakpoints', params => this._disableCustomBreakpoints(params));
     this.dap.on('prettyPrintSource', params => this._prettyPrintSource(params));
     this.dap.on('revealPage', () => this._withThread(thread => thread.revealPage()));
     this.dap.on('getPerformance', () =>
@@ -456,8 +455,7 @@ export class DebugAdapter implements IDisposable {
       profile.start(this.dap, this._thread, { type: BasicCpuProfiler.type });
     }
 
-    for (const breakpoint of this._customBreakpoints)
-      this._thread.updateCustomBreakpoint(breakpoint, true);
+    this._thread.updateCustomBreakpoints(this._customBreakpoints);
 
     this.asyncStackPolicy
       .connect(cdp)
@@ -475,27 +473,14 @@ export class DebugAdapter implements IDisposable {
     return this._thread;
   }
 
-  async enableCustomBreakpoints(
-    params: Dap.EnableCustomBreakpointsParams,
-  ): Promise<Dap.EnableCustomBreakpointsResult> {
-    const promises: Promise<void>[] = [];
-    for (const id of params.ids) {
-      this._customBreakpoints.add(id);
-      if (this._thread) promises.push(this._thread.updateCustomBreakpoint(id, true));
+  async setCustomBreakpoints({
+    ids,
+  }: Dap.SetCustomBreakpointsParams): Promise<Dap.SetCustomBreakpointsResult> {
+    this._customBreakpoints = ids;
+    if (this._thread) {
+      await this._thread.updateCustomBreakpoints(ids);
     }
-    await Promise.all(promises);
-    return {};
-  }
 
-  async _disableCustomBreakpoints(
-    params: Dap.DisableCustomBreakpointsParams,
-  ): Promise<Dap.DisableCustomBreakpointsResult> {
-    const promises: Promise<void>[] = [];
-    for (const id of params.ids) {
-      this._customBreakpoints.delete(id);
-      if (this._thread) promises.push(this._thread.updateCustomBreakpoint(id, false));
-    }
-    await Promise.all(promises);
     return {};
   }
 

src/adapter/threads.ts

@@ -173,6 +173,7 @@ export class Thread implements IVariableStoreLocationProvider {
   private _sourceMapDisabler?: SourceMapDisabler;
   private _expectedPauseReason?: ExpectedPauseReason;
   private _excludedCallers: readonly Dap.ExcludedCaller[] = [];
+  private _enabledCustomBreakpoints?: ReadonlySet<CustomBreakpointId>;
   private readonly stateQueue = new StateQueue();
   private readonly _onPausedEmitter = new EventEmitter<IPausedDetails>();
   private readonly _dap: DeferredContainer<Dap.Api>;
@@ -1249,13 +1250,28 @@ export class Thread implements IVariableStoreLocationProvider {
     return `@ VM${raw.scriptId || 'XX'}:${raw.lineNumber}`;
   }
 
-  async updateCustomBreakpoint(id: CustomBreakpointId, enabled: boolean): Promise<void> {
+  async updateCustomBreakpoints(ids: CustomBreakpointId[]): Promise<void> {
     if (!this.target.supportsCustomBreakpoints()) return;
-    const breakpoint = customBreakpoints().get(id);
-    if (!breakpoint) return;
+    this._enabledCustomBreakpoints ??= new Set();
+
     // Do not fail for custom breakpoints, to account for
     // future changes in cdp vs stale breakpoints saved in the workspace.
-    await breakpoint.apply(this._cdp, enabled);
+    const newIds = new Set(ids);
+    const todo: (Promise<unknown> | undefined)[] = [];
+
+    for (const newId of newIds) {
+      if (!this._enabledCustomBreakpoints.has(newId)) {
+        todo.push(customBreakpoints().get(newId)?.apply(this._cdp, true));
+      }
+    }
+    for (const oldId of this._enabledCustomBreakpoints) {
+      if (!newIds.has(oldId)) {
+        todo.push(customBreakpoints().get(oldId)?.apply(this._cdp, false));
+      }
+    }
+
+    this._enabledCustomBreakpoints = newIds;
+    await Promise.all(todo);
   }
 
   private async _createPausedDetails(event: Cdp.Debugger.PausedEvent): Promise<IPausedDetails> {

src/build/dapCustom.ts

@@ -81,27 +81,14 @@ const makeRequest = (
 
 const dapCustom: JSONSchema4 = {
   definitions: {
-    ...makeRequest('enableCustomBreakpoints', 'Enable custom breakpoints.', {
+    ...makeRequest('setCustomBreakpoints', 'Sets the enabled custom breakpoints.', {
       properties: {
         ids: {
           type: 'array',
           items: {
             type: 'string',
           },
-          description: 'Id of breakpoints to enable.',
-        },
-      },
-      required: ['ids'],
-    }),
-
-    ...makeRequest('disableCustomBreakpoints', 'Disable custom breakpoints.', {
-      properties: {
-        ids: {
-          type: 'array',
-          items: {
-            type: 'string',
-          },
-          description: 'Id of breakpoints to enable.',
+          description: 'Id of breakpoints that should be enabled.',
         },
       },
       required: ['ids'],

src/build/generate-contributions.ts

@@ -1213,15 +1213,10 @@ const commands: ReadonlyArray<{
     category: 'Debug',
   },
   {
-    command: Commands.AddCustomBreakpoints,
+    command: Commands.ToggleCustomBreakpoints,
     title: refString('add.eventListener.breakpoint'),
     icon: '$(add)',
   },
-  {
-    command: Commands.RemoveCustomBreakpoints,
-    title: refString('remove.eventListener.breakpoint'),
-    icon: '$(remove)',
-  },
   {
     command: Commands.RemoveAllCustomBreakpoints,
     title: refString('remove.eventListener.breakpoint.all'),
@@ -1437,7 +1432,7 @@ const menus: Menus = {
   ],
   'view/title': [
     {
-      command: Commands.AddCustomBreakpoints,
+      command: Commands.ToggleCustomBreakpoints,
       when: `view == ${CustomViews.EventListenerBreakpoints}`,
       group: 'navigation',
     },

src/cdp/api.d.ts

@@ -23859,6 +23859,28 @@ export namespace Cdp {
       | 'PageSupportNeeded'
       | 'Circumstantial';
 
+    export interface BackForwardCacheBlockingDetails {
+      /**
+       * Url of the file where blockage happened. Optional because of tests.
+       */
+      url?: string;
+
+      /**
+       * Function name where blockage happened. Optional because of anonymous functions and tests.
+       */
+      function?: string;
+
+      /**
+       * Line number in the script (0-based).
+       */
+      lineNumber: integer;
+
+      /**
+       * Column number in the script (0-based).
+       */
+      columnNumber: integer;
+    }
+
     export interface BackForwardCacheNotRestoredExplanation {
       /**
        * Type of the reason
@@ -23876,6 +23898,8 @@ export namespace Cdp {
        * - EmbedderExtensionSentMessageToCachedFrame: the extension ID.
        */
       context?: string;
+
+      details?: BackForwardCacheBlockingDetails[];
     }
 
     export interface BackForwardCacheNotRestoredExplanationTree {
@@ -28676,6 +28700,8 @@ export namespace Cdp {
       ends: integer[];
     }
 
+    export type AttributionReportingTriggerDataMatching = 'exact' | 'modulus';
+
     export interface AttributionReportingSourceRegistration {
       time: Network.TimeSinceEpoch;
 
@@ -28708,6 +28734,8 @@ export namespace Cdp {
       aggregationKeys: AttributionReportingAggregationKeysEntry[];
 
       debugKey?: UnsignedInt64AsBase10;
+
+      triggerDataMatching: AttributionReportingTriggerDataMatching;
     }
 
     export type AttributionReportingSourceRegistrationResult =

src/common/contributionUtils.ts

@@ -25,7 +25,7 @@ export const enum CustomViews {
 }
 
 export const enum Commands {
-  AddCustomBreakpoints = 'extension.js-debug.addCustomBreakpoints',
+  ToggleCustomBreakpoints = 'extension.js-debug.addCustomBreakpoints',
   AttachProcess = 'extension.pwa-node-debug.attachNodeProcess',
   AutoAttachClearVariables = 'extension.js-debug.clearAutoAttachVariables',
   AutoAttachSetVariables = 'extension.js-debug.setAutoAttachVariables',
@@ -38,7 +38,6 @@ export const enum Commands {
   PickProcess = 'extension.js-debug.pickNodeProcess',
   PrettyPrint = 'extension.js-debug.prettyPrint',
   RemoveAllCustomBreakpoints = 'extension.js-debug.removeAllCustomBreakpoints',
-  RemoveCustomBreakpoints = 'extension.js-debug.removeCustomBreakpoint',
   RevealPage = 'extension.js-debug.revealPage',
   RequestCDPProxy = 'extension.js-debug.requestCDPProxy',
   /** Use node-debug's command so existing keybindings work */
@@ -86,7 +85,7 @@ const debugTypes: { [K in DebugType]: null } = {
 };
 
 const commandsObj: { [K in Commands]: null } = {
-  [Commands.AddCustomBreakpoints]: null,
+  [Commands.ToggleCustomBreakpoints]: null,
   [Commands.AttachProcess]: null,
   [Commands.AutoAttachClearVariables]: null,
   [Commands.AutoAttachSetVariables]: null,
@@ -99,7 +98,6 @@ const commandsObj: { [K in Commands]: null } = {
   [Commands.PickProcess]: null,
   [Commands.PrettyPrint]: null,
   [Commands.RemoveAllCustomBreakpoints]: null,
-  [Commands.RemoveCustomBreakpoints]: null,
   [Commands.RevealPage]: null,
   [Commands.StartProfile]: null,
   [Commands.StopProfile]: null,

src/common/objUtils.ts

@@ -340,7 +340,7 @@ export function trailingEdgeThrottle(
  */
 export async function bisectArrayAsync<T>(
   items: ReadonlyArray<T>,
-  predicate: (item: T) => Promise<boolean>,
+  predicate: (item: T) => Promise<boolean> | boolean,
 ): Promise<[T[], T[]]> {
   const a: T[] = [];
   const b: T[] = [];