Revert "[dap] Simplify URI handling in IsolateManager.addBreakpoint" and "[dds/dap] Add/remove breakpoints as required instead of replacing the whole set"

This reverts two commits (in separate patch sets in the CL):

- commit 482a7ca: [dap] Simplify URI handling in IsolateManager.addBreakpoint
- commit d0a7ef4: [dds/dap] Add/remove breakpoints as required instead of replacing the whole set

The first revert is to avoid conflicts while reverting the second, and will be reapplied later (likely after the release branch, since it is not critical to include). The second revert is because this change resulted in leaked Script objects in the VM.

There is an additional change (patch set 3) to fix up the changelog/versions so they don't go backwards.

Change-Id: Id55949e1622d3367ced87c9a307c43881b030cbb
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440162
Reviewed-by: Slava Egorov <[email protected]>
Reviewed-by: Ben Konyi <[email protected]>
Commit-Queue: Ben Konyi <[email protected]>

Author: DanTup

pkg/dds/CHANGELOG.md

@@ -1,3 +1,6 @@
+# 5.0.5
+- [DAP] The change in DDS 5.0.4 to individually add/remove breakpoints has been reverted and may be restored in a future version.
+
 # 5.0.4
 - [DAP] Breakpoints are now added/removed individually instead of all being cleared and re-added during a `setBreakpoints` request. This improves performance and can avoid breakpoints flickering between unresolved/resolved when adding new breakpoints in the same file.
 

pkg/dds/lib/src/dap/adapters/dart.dart

@@ -1577,70 +1577,26 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
         : name!;
 
     // Use a completer to track when the response is sent, so any events related
-    // to new breakpoints will not be sent to the client before the response
-    // here which provides the IDs to the client.
+    // to these breakpoints are not sent before the client has the IDs.
     final completer = Completer<void>();
 
-    // Map the provided breakpoints onto either new or existing instances of
-    // [ClientBreakpoint] that we use to track the clients breakpoints
-    // internally.
-    final clientBreakpoints = breakpoints.map((bp) {
-      return
-          // First try to match an existing breakpoint so we can avoid deleting
-          // and re-creating all breakpoints if a new one is added to a file.
-          isolateManager.findExistingClientBreakpoint(uri, bp) ??
-              ClientBreakpoint(bp, completer.future);
-    }).toList();
-
-    // Any breakpoints that are not in our new set will need to be removed from
-    // the VM.
-    //
-    // Because multiple client breakpoints may resolve to the same VM breakpoint
-    // we must exclude any that still remain in one of the kept breakpoints.
-    final referencedVmBreakpoints =
-        clientBreakpoints.map((bp) => bp.forThread.values).toSet();
-    final breakpointsToRemove = isolateManager.clientBreakpointsByUri[uri]
-        ?.toSet()
-        // Remove any we're reusing.
-        .difference(clientBreakpoints.toSet())
-        // Remove any that map to VM breakpoints that are still referenced
-        // because we'll want to keep them.
-        .where((clientBreakpoint) => clientBreakpoint.forThread.values
-            .none(referencedVmBreakpoints.contains));
-
-    // Store this new set of breakpoints as the current set for this URI.
-    isolateManager.recordLatestClientBreakpoints(uri, clientBreakpoints);
-
-    // Prepare the response with the existing values before we start updating.
-    final breakpointResponse = SetBreakpointsResponseBody(
+    final clientBreakpoints = breakpoints
+        .map((bp) => ClientBreakpoint(bp, completer.future))
+        .toList();
+    await isolateManager.setBreakpoints(uri, clientBreakpoints);
+
+    sendResponse(SetBreakpointsResponseBody(
       breakpoints: clientBreakpoints
+          // Send breakpoints back as unverified and with our generated IDs so we
+          // can update them with a 'breakpoint' event when we get the
+          // 'BreakpointAdded'/'BreakpointResolved' events from the VM.
           .map((bp) => Breakpoint(
               id: bp.id,
-              verified: bp.verified,
-              line: bp.verified ? bp.resolvedLine : null,
-              column: bp.verified ? bp.resolvedColumn : null,
-              message: bp.verified ? null : bp.verifiedMessage,
-              reason: bp.verified ? null : bp.verifiedReason))
+              verified: false,
+              message: 'Breakpoint has not yet been resolved',
+              reason: 'pending'))
           .toList(),
-    );
-
-    // Update the breakpoints for all existing threads.
-    await Future.wait(isolateManager.threads.map((thread) async {
-      // Remove the deleted breakpoints.
-      if (breakpointsToRemove != null) {
-        await Future.wait(breakpointsToRemove.map((clientBreakpoint) =>
-            isolateManager.removeBreakpoint(clientBreakpoint, thread)));
-      }
-
-      // Add the new breakpoints.
-      await Future.wait(clientBreakpoints.map((clientBreakpoint) async {
-        if (!clientBreakpoint.isKnownToVm) {
-          await isolateManager.addBreakpoint(clientBreakpoint, thread, uri);
-        }
-      }));
-    }));
-
-    sendResponse(breakpointResponse);
+    ));
     completer.complete();
   }