[dds] [dap] Fix bug that tried to remove previously-removed breakpoints

Fixes https://github.com/flutter/flutter/issues/106369.

Change-Id: I17fb46ad0ea0f30707ff6280f2e8269662e376f7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249485
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

pkg/dds/CHANGELOG.md

@@ -1,3 +1,6 @@
+# 2.2.4
+- Fix an issue where DAP adapters could try to remove the same breakpoint multiple times.
+
 # 2.2.3
 - Internal DAP changes.
 

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

@@ -624,7 +624,12 @@ class IsolateManager {
           _vmBreakpointsByIsolateIdAndUri.putIfAbsent(isolateId, () => {});
       final existingBreakpointsForIsolateAndUri =
           existingBreakpointsForIsolate.putIfAbsent(uri, () => []);
-      await Future.forEach<vm.Breakpoint>(existingBreakpointsForIsolateAndUri,
+      // Before doing async work, take a copy of the breakpoints to remove
+      // and remove them from the list, so any subsequent calls here don't
+      // try to remove the same ones.
+      final breakpointsToRemove = existingBreakpointsForIsolateAndUri.toList();
+      existingBreakpointsForIsolateAndUri.clear();
+      await Future.forEach<vm.Breakpoint>(breakpointsToRemove,
           (bp) => service.removeBreakpoint(isolateId, bp.id!));
 
       // Set new breakpoints.

pkg/dds/pubspec.yaml

@@ -1,5 +1,5 @@
 name: dds
-version: 2.2.3
+version: 2.2.4
 description: >-
   A library used to spawn the Dart Developer Service, used to communicate with
   a Dart VM Service instance.

pkg/dds/test/dap/integration/debug_breakpoints_test.dart

@@ -27,6 +27,50 @@ main() {
       await client.hitBreakpoint(testFile, breakpointLine);
     });
 
+    test('does not stop at a removed breakpoint', () async {
+      final testFile = dap.createTestFile('''
+void main(List<String> args) async {
+  print('Hello!'); $breakpointMarker
+  print('Hello!'); $breakpointMarker
+}
+    ''');
+
+      final client = dap.client;
+      final breakpoint1Line = lineWith(testFile, breakpointMarker);
+      final breakpoint2Line = breakpoint1Line + 1;
+
+      // Hit the first breakpoint.
+      final stop = await client.hitBreakpoint(testFile, breakpoint1Line,
+          additionalBreakpoints: [breakpoint2Line]);
+
+      // Remove all breakpoints.
+      await client.setBreakpoints(testFile, []);
+
+      // Resume and expect termination (should not hit the second breakpoint).
+      await Future.wait([
+        client.event('terminated'),
+        client.continue_(stop.threadId!),
+      ], eagerError: true);
+    });
+
+    test('does not fail updating breakpoints after a removal', () async {
+      // https://github.com/flutter/flutter/issues/106369 was caused by us not
+      // tracking removals correctly, meaning we could try to remove a removed
+      // breakpoint a second time.
+      final client = dap.client;
+      final testFile = dap.createTestFile(simpleBreakpointProgram);
+      final breakpointLine = lineWith(testFile, breakpointMarker);
+
+      await client.hitBreakpoint(testFile, breakpointLine);
+
+      // Remove the breakpoint.
+      await client.setBreakpoints(testFile, []);
+
+      // Send another breakpoint update to ensure it doesn't try to re-remove
+      // the previously removed breakpoint.
+      await client.setBreakpoints(testFile, []);
+    });
+
     test('stops at a line breakpoint in the SDK set via local sources',
         () async {
       final client = dap.client;

pkg/dds/test/dap/integration/test_client.dart

@@ -494,18 +494,24 @@ extension DapTestClientExtension on DapTestClient {
   Future<StoppedEventBody> hitBreakpoint(
     File file,
     int line, {
+    List<int>? additionalBreakpoints,
     File? entryFile,
     String? condition,
     String? cwd,
     List<String>? args,
     Future<Response> Function()? launch,
   }) async {
+    assert(condition == null || additionalBreakpoints == null,
+        'Only one of condition/additionalBreakpoints can be sent');
     entryFile ??= file;
     final stop = expectStop('breakpoint', file: file, line: line);
 
     await Future.wait([
       initialize(),
-      setBreakpoint(file, line, condition: condition),
+      if (additionalBreakpoints != null)
+        setBreakpoints(file, [line, ...additionalBreakpoints])
+      else
+        setBreakpoint(file, line, condition: condition),
       launch?.call() ?? this.launch(entryFile.path, cwd: cwd, args: args),
     ], eagerError: true);
 
@@ -522,6 +528,16 @@ extension DapTestClientExtension on DapTestClient {
     );
   }
 
+  /// Sets breakpoints at [lines] in [file].
+  Future<void> setBreakpoints(File file, List<int> lines) async {
+    await sendRequest(
+      SetBreakpointsArguments(
+        source: Source(path: file.path),
+        breakpoints: lines.map((line) => SourceBreakpoint(line: line)).toList(),
+      ),
+    );
+  }
+
   /// Sets the exception pause mode to [pauseMode] and expects to pause after
   /// running the script.
   ///