From 4035ab6ac9c5e6b18e3456958c8e61c1eb9c3229 Mon Sep 17 00:00:00 2001
From: Ivan Inozemtsev <iinozemtsev@google.com>
Date: Fri, 28 Nov 2025 16:39:24 +0100
Subject: [PATCH] Cache js breakpoint ids and reuse them when adding new
 breakpoints

Internally, there are two different code paths to trigger hot restart:

1.  Via connected debugger, triggering `_hotRestart` funciton in `dwds_vm_client.dart`. This codepath removes existing breakpoints and blocks main script execution (by passing `pauseIsolatesOnStart` into `dartHotRestartDwds`)  until the connected debugger re-adds breakpoints.
2. Via in-app shortcut in injected UI, which just calls `dartHotRestartDwds` directly. This codepath does not clear breakpoints and does not block main startup.

The flow #2 is currently doesn't work as intended:

1. Chrome breakpoints aren't cleared, but the attached debugger still gets an isolate start event
2. It tries to set all breakpoints in a new isolate, and sends add breakpoint commands
3. Chrome API returns an error tha the breakpoint exists, and the error is propagated back to DAP server, which sends a set breakpoint error notification to an IDE.
4. The IDE thinks that the breakpoint is "unconfirmed", and hides it from the UI completely.

As a result, the user does not see the breakpoint, but it's there, in Chrome debugger state, and the execution stops on it, and there's no way for user to remove it (except for clearing all breakpoints by the restart/reload from an IDE, which clears chrome debugger state).

The CL attempts to fix it by caching chrome breakpoint ids by js locations, so that when the debugger adds a breakpoint resolving to the same js location, it gets back the same js breakpoint id.
---
 dwds/CHANGELOG.md                    |  1 +
 dwds/lib/src/debugging/debugger.dart | 65 ++++++++++++++++++++++------
 2 files changed, 52 insertions(+), 14 deletions(-)

diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
index 814cfe500..ea82cfb14 100644
--- a/dwds/CHANGELOG.md
+++ b/dwds/CHANGELOG.md
@@ -3,6 +3,7 @@
 - Bump `build_web_compilers` to ^4.4.1.
 - Remove unused `clientFuture` arg from `DwdsVmClient` methods.
 - Fix pausing starting of `main` after the hot restart.
+- Cache javascript breakpoint ids to prevent 'Breakpoint already exists' errors.
 
 ## 26.2.2
 
diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart
index 38e063e32..40995e678 100644
--- a/dwds/lib/src/debugging/debugger.dart
+++ b/dwds/lib/src/debugging/debugger.dart
@@ -771,6 +771,20 @@ class _Breakpoints {
   final _dartIdByJsId = <String, String>{};
   final _jsIdByDartId = <String, String>{};
 
+  /// JS location -> JS breakpoint ID
+  final _jsIdByJsLocKey = <String, String>{};
+
+  /// JS breakpoint ID -> JS location
+  final _jsLocKeyByJsId = <String, String>{};
+
+  String _jsLocKey(JsLocation location) =>
+      '${location.runtimeScriptId}:${location.line}:${location.column}';
+
+  String? _lookupExistingJsBreakpoint(JsLocation location) {
+    final jsLocKey = _jsLocKey(location);
+    return _jsIdByJsLocKey[jsLocKey];
+  }
+
   final _bpByDartId = <String, Future<Breakpoint>>{};
 
   final _queue = AtomicQueue();
@@ -835,7 +849,11 @@ class _Breakpoints {
               'cannot set JS breakpoint at $location',
         );
       }
-      _note(jsId: jsBreakpointId, bp: dartBreakpoint);
+      _note(
+        jsId: jsBreakpointId,
+        bp: dartBreakpoint,
+        jsLocation: location.jsLocation,
+      );
       return dartBreakpoint;
     } on WipError catch (wipError) {
       throw RPCError('addBreakpoint', 102, '$wipError');
@@ -880,18 +898,19 @@ class _Breakpoints {
     return _queue.run(() async {
       final scriptId = location.jsLocation.runtimeScriptId;
       if (scriptId != null) {
-        return sendCommandAndValidateResult<String>(
-          remoteDebugger,
-          method: 'Debugger.setBreakpoint',
-          resultField: 'breakpointId',
-          params: {
-            'location': {
-              'lineNumber': location.jsLocation.line,
-              'columnNumber': location.jsLocation.column,
-              'scriptId': scriptId,
-            },
-          },
-        );
+        return _lookupExistingJsBreakpoint(location.jsLocation) ??
+            await sendCommandAndValidateResult<String>(
+              remoteDebugger,
+              method: 'Debugger.setBreakpoint',
+              resultField: 'breakpointId',
+              params: {
+                'location': {
+                  'lineNumber': location.jsLocation.line,
+                  'columnNumber': location.jsLocation.column,
+                  'scriptId': scriptId,
+                },
+              },
+            );
       } else {
         _logger.fine('No runtime script ID for location $location');
         return null;
@@ -901,11 +920,24 @@ class _Breakpoints {
 
   /// Records the internal Dart <=> JS breakpoint id mapping and adds the
   /// breakpoint to the current isolates list of breakpoints.
-  void _note({required Breakpoint bp, required String jsId}) {
+  void _note({
+    required Breakpoint bp,
+    required String jsId,
+    required JsLocation jsLocation,
+  }) {
     final bpId = bp.id;
     if (bpId != null) {
       _dartIdByJsId[jsId] = bpId;
       _jsIdByDartId[bpId] = jsId;
+
+      // Cache JS breakpoint id.
+      final jsLocKey = _jsLocKey(jsLocation);
+      if (_jsIdByJsLocKey.containsKey(jsLocKey)) {
+        _logger.fine('Reused existing JS breakpoint $jsId for $jsLocKey');
+      }
+      _jsIdByJsLocKey[jsLocKey] = jsId;
+      _jsLocKeyByJsId[jsId] = jsLocKey;
+
       final isolate = inspector.isolate;
       isolate.breakpoints?.add(bp);
     }
@@ -916,6 +948,11 @@ class _Breakpoints {
     required String dartId,
   }) async {
     final isolate = inspector.isolate;
+    // Clear from JS location caches
+    final jsLocKey = _jsLocKeyByJsId.remove(jsId);
+    if (jsLocKey != null) {
+      _jsIdByJsLocKey.remove(jsLocKey);
+    }
     _dartIdByJsId.remove(jsId);
     _jsIdByDartId.remove(dartId);
     isolate.breakpoints?.removeWhere((b) => b.id == dartId);