Skip to content

Commit 7a2c286

Browse files
committed
[DWDS] Don't send PauseInterrupted event during a hot reload and publish 25.1.1
Fixes dart-lang/sdk#61560 We rely on a pause within a hot reload to pause execution so that we can reregister breakpoints. However, the existing pause mechanism always sends a PauseInterrupted event, which then triggers the client to think this is a normal pause event and not an internal detail. Instead, we should have the ChromeProxyService signal to the debugger that this is an "internal pause" and therefore it should not send a regular pause event and should use a completer to signal the pause is done. Tests are refactored and updated to correctly check for the events when reregistering breakpoints. Specifically, it checks no other events besides the expected ones are sent.
1 parent 29ba1b1 commit 7a2c286

File tree

8 files changed

+863
-1306
lines changed

8 files changed

+863
-1306
lines changed

dwds/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
## 25.1.1-wip
1+
## 25.1.1
22

33
- Bump SDK constraint to ^3.10.0
4+
- Fix an issue in `reloadSources` where a `PauseInterrupted` event was sent. - [#61560](https://github.com/dart-lang/sdk/issues/61560)
45

56
## 25.1.0
67

dwds/lib/src/debugging/debugger.dart

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@ class Debugger extends Domain {
8282
// DevTools is showing an overlay. Both cannot be shown at the same time:
8383
// bool _pausedOverlayVisible = false;
8484

85-
String get pauseState => _pauseModePauseStates.entries
86-
.firstWhere((entry) => entry.value == _pauseState)
87-
.key;
85+
String get pauseState =>
86+
_pauseModePauseStates.entries
87+
.firstWhere((entry) => entry.value == _pauseState)
88+
.key;
8889

8990
/// The JS frames at the current paused location.
9091
///
@@ -96,15 +97,32 @@ class Debugger extends Domain {
9697
bool _isStepping = false;
9798
DartLocation? _previousSteppingLocation;
9899

100+
// If not null and not completed, the pause handler completes this instead of
101+
// sending a `PauseInterrupted` event to the event stream.
102+
//
103+
// In some cases e.g. a hot reload, DWDS pauses the execution itself in order
104+
// to handle debugging logic like breakpoints. In such cases, sending the
105+
// event to the event stream may trigger the client to believe that the user
106+
// sent the event. To avoid that and still signal that a pause is completed,
107+
// this completer is used. See https://github.com/dart-lang/sdk/issues/61560
108+
// for more details.
109+
Completer<void>? _pauseInterruptedCompleter;
110+
99111
void updateInspector(AppInspectorInterface appInspector) {
100112
inspector = appInspector;
101113
_breakpoints.inspector = appInspector;
102114
}
103115

104-
Future<Success> pause() async {
116+
Future<Success> pause({bool internalPause = false}) async {
105117
_isStepping = false;
118+
if (internalPause) {
119+
_pauseInterruptedCompleter = Completer<void>();
120+
}
106121
final result = await _remoteDebugger.pause();
107122
handleErrorIfPresent(result);
123+
if (internalPause) {
124+
await _pauseInterruptedCompleter!.future;
125+
}
108126
return Success();
109127
}
110128

@@ -522,15 +540,17 @@ class Debugger extends Domain {
522540
final timestamp = DateTime.now().millisecondsSinceEpoch;
523541
final jsBreakpointIds = e.hitBreakpoints ?? [];
524542
if (jsBreakpointIds.isNotEmpty) {
525-
final breakpointIds = jsBreakpointIds
526-
.map((id) => _breakpoints._dartIdByJsId[id])
527-
// In case the breakpoint was set in Chrome DevTools outside of
528-
// package:dwds.
529-
.where((entry) => entry != null)
530-
.toSet();
531-
final pauseBreakpoints = isolate.breakpoints
532-
?.where((bp) => breakpointIds.contains(bp.id))
533-
.toList();
543+
final breakpointIds =
544+
jsBreakpointIds
545+
.map((id) => _breakpoints._dartIdByJsId[id])
546+
// In case the breakpoint was set in Chrome DevTools outside of
547+
// package:dwds.
548+
.where((entry) => entry != null)
549+
.toSet();
550+
final pauseBreakpoints =
551+
isolate.breakpoints
552+
?.where((bp) => breakpointIds.contains(bp.id))
553+
.toList();
534554
event = Event(
535555
kind: EventKind.kPauseBreakpoint,
536556
timestamp: timestamp,
@@ -632,7 +652,13 @@ class Debugger extends Domain {
632652
// DevTools is showing an overlay. Both cannot be shown at the same time.
633653
// _showPausedOverlay();
634654
isolate.pauseEvent = event;
635-
_streamNotify('Debug', event);
655+
if (event.kind == EventKind.kPauseInterrupted &&
656+
_pauseInterruptedCompleter != null &&
657+
!_pauseInterruptedCompleter!.isCompleted) {
658+
_pauseInterruptedCompleter!.complete();
659+
} else {
660+
_streamNotify('Debug', event);
661+
}
636662
}
637663

638664
/// Handles resume events coming from the Chrome connection.

0 commit comments

Comments
 (0)