diff --git a/packages/devtools_app/integration_test/test/live_connection/debugger_panel_test.dart b/packages/devtools_app/integration_test/test/live_connection/debugger_panel_test.dart index 360a8b1b51a..b5245639274 100644 --- a/packages/devtools_app/integration_test/test/live_connection/debugger_panel_test.dart +++ b/packages/devtools_app/integration_test/test/live_connection/debugger_panel_test.dart @@ -3,8 +3,10 @@ // found in the LICENSE file. import 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_app/src/screens/debugger/breakpoints.dart'; import 'package:devtools_app/src/screens/debugger/call_stack.dart'; import 'package:devtools_app/src/screens/debugger/codeview.dart'; +import 'package:devtools_app/src/service/service_extension_widgets.dart'; import 'package:devtools_test/helpers.dart'; import 'package:devtools_test/integration_test.dart'; import 'package:flutter/material.dart'; @@ -59,27 +61,44 @@ void main() { isTrue, ); - logStatus('opening the "more" menu'); + logStatus('Navigating to line 57...'); - final moreMenuFinder = find.byType(PopupMenuButton); - expect(moreMenuFinder, findsOneWidget); - await tester.tap(moreMenuFinder); - await tester.pumpAndSettle(safePumpDuration); + await goToLine(tester, lineNumber: 57); - logStatus('selecting the go-to-line menu option'); + logStatus('looking for line 57'); - final goToLineOptionFinder = find.textContaining('Go to line number'); - expect(goToLineOptionFinder, findsOneWidget); - await tester.tap(goToLineOptionFinder); - await tester.pumpAndSettle(safePumpDuration); + // Look for the line 57 gutter item: + final gutter57Finder = findGutterItemWithText('57'); + expect(gutter57Finder, findsOneWidget); - logStatus('entering line number in the go-to-line dialog'); + // Look for the line 57 line item: + final line57Finder = findLineItemWithText("print('Hello!');"); + expect(line57Finder, findsOneWidget); - final goToLineInputFinder = find.widgetWithText(TextField, 'Line Number'); - expect(goToLineInputFinder, findsOneWidget); - await tester.enterText(goToLineInputFinder, '30'); - await tester.testTextInput.receiveAction(TextInputAction.done); - await tester.pumpAndSettle(safePumpDuration); + // Verify that the gutter item and line item are aligned: + expect( + areHorizontallyAligned( + gutter57Finder, + line57Finder, + tester: tester, + ), + isTrue, + ); + + logStatus('setting a breakpoint'); + + // Tap on the gutter for the line to set a breakpoint: + await tester.tap(gutter57Finder); + await tester.pumpAndSettle(longPumpDuration); + + logStatus('performing a hot restart'); + + await tester.tap(find.byType(HotRestartButton)); + await tester.pumpAndSettle(longPumpDuration); + + logStatus('Navigating to line 30...'); + + await goToLine(tester, lineNumber: 30); logStatus('looking for line 30'); @@ -107,6 +126,11 @@ void main() { await tester.tap(gutter30Finder); await tester.pumpAndSettle(longPumpDuration); + logStatus('verifying breakpoints'); + + final bpSetBeforeRestart = findBreakpointWithText('main.dart:57'); + expect(bpSetBeforeRestart, findsOneWidget); + logStatus('pausing at breakpoint'); final topFrameFinder = findStackFrameWithText('incrementCounter'); @@ -170,6 +194,30 @@ bool areHorizontallyAligned( return widgetACenter.dy == widgetBCenter.dy; } +Future goToLine(WidgetTester tester, {required int lineNumber}) async { + logStatus('opening the "more" menu'); + + final moreMenuFinder = find.byType(PopupMenuButton); + expect(moreMenuFinder, findsOneWidget); + await tester.tap(moreMenuFinder); + await tester.pumpAndSettle(safePumpDuration); + + logStatus('selecting the go-to-line menu option'); + + final goToLineOptionFinder = find.textContaining('Go to line number'); + expect(goToLineOptionFinder, findsOneWidget); + await tester.tap(goToLineOptionFinder); + await tester.pumpAndSettle(safePumpDuration); + + logStatus('entering line number $lineNumber in the go-to-line dialog'); + + final goToLineInputFinder = find.widgetWithText(TextField, 'Line Number'); + expect(goToLineInputFinder, findsOneWidget); + await tester.enterText(goToLineInputFinder, '$lineNumber'); + await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.pumpAndSettle(safePumpDuration); +} + T getWidgetFromFinder(Finder finder) => finder.first.evaluate().first.widget as T; @@ -192,3 +240,8 @@ Finder findStackFrameWithText(String text) => find.descendant( of: find.byType(CallStack), matching: find.richTextContaining(text), ); + +Finder findBreakpointWithText(String text) => find.descendant( + of: find.byType(Breakpoints), + matching: find.richTextContaining(text), + ); diff --git a/packages/devtools_app/lib/src/framework/framework_core.dart b/packages/devtools_app/lib/src/framework/framework_core.dart index f8792e1d0f5..fa52410338a 100644 --- a/packages/devtools_app/lib/src/framework/framework_core.dart +++ b/packages/devtools_app/lib/src/framework/framework_core.dart @@ -103,7 +103,7 @@ class FrameworkCore { service, onClosed: finishedCompleter.future, ); - breakpointManager.initialize(); + await breakpointManager.initialize(); return true; } catch (e, st) { if (logException) { diff --git a/packages/devtools_app/lib/src/screens/debugger/breakpoint_manager.dart b/packages/devtools_app/lib/src/screens/debugger/breakpoint_manager.dart index 77bbd42ba87..01835e1d3ee 100644 --- a/packages/devtools_app/lib/src/screens/debugger/breakpoint_manager.dart +++ b/packages/devtools_app/lib/src/screens/debugger/breakpoint_manager.dart @@ -35,19 +35,21 @@ class BreakpointManager with DisposerMixin { String get _isolateRefId => _isolateRef?.id ?? ''; - void initialize() { + final _previousIsolateBreakpoints = []; + + Future initialize() async { final isolate = serviceConnection.serviceManager.isolateManager.selectedIsolate.value; if (initialSwitchToIsolate && isolate != null) { - switchToIsolate( + await switchToIsolate( serviceConnection.serviceManager.isolateManager.selectedIsolate.value, ); } addAutoDisposeListener( serviceConnection.serviceManager.isolateManager.selectedIsolate, - () { - switchToIsolate( + () async { + await switchToIsolate( serviceConnection.serviceManager.isolateManager.selectedIsolate.value, ); }, @@ -60,40 +62,38 @@ class BreakpointManager with DisposerMixin { ); } - void switchToIsolate(IsolateRef? isolateRef) async { + Future switchToIsolate(IsolateRef? isolateRef) async { _isolateRef = isolateRef; if (isolateRef == null) { - _breakpoints.value.clear(); - _breakpointsWithLocation.value.clear(); + _saveAndClearCurrentBreakpoints(); return; } - final isolate = await _service.getIsolate(_isolateRefId); - if (isolate.id != _isolateRefId) { - // Current request is obsolete. - return; + final breakpointsForIsolate = + await _getBreakpointsForIsolate(_isolateRefId); + if (breakpointsForIsolate.isNotEmpty) { + // If the isolate already has breakpoints, then update them: + await _updateBreakpoints( + breakpoints: breakpointsForIsolate, + isolateId: _isolateRefId, + ); + } else { + // Otherwise, re-establish the breakpoints from the previous isolate: + await _setUpBreakpoints( + breakpoints: _previousIsolateBreakpoints, + isolateRef: isolateRef, + ); } - - _breakpoints.value = isolate.breakpoints ?? []; - - // Build _breakpointsWithLocation from _breakpoints. - // ignore: unawaited_futures - Future.wait( - _breakpoints.value.map(breakpointManager.createBreakpointWithLocation), - ).then((list) { - if (isolate.id != _isolateRefId) { - // Current request is obsolete. - return; - } - _breakpointsWithLocation.value = list.toList()..sort(); - }); } - void clearCache() { + void clearCache({required bool isServiceShutdown}) { _breakPositionsMap.clear(); _breakpoints.value = []; _breakpointsWithLocation.value = []; + if (isServiceShutdown) { + _previousIsolateBreakpoints.clear(); + } } Future clearBreakpoints() async { @@ -138,6 +138,16 @@ class BreakpointManager with DisposerMixin { } } + void _saveAndClearCurrentBreakpoints() { + if (breakpointsWithLocation.value.isNotEmpty) { + _previousIsolateBreakpoints + ..clear() + ..addAll(_breakpointsWithLocation.value); + } + _breakpoints.value = []; + _breakpointsWithLocation.value = []; + } + void _updateAfterIsolateReload( Event _, ) async { @@ -175,6 +185,87 @@ class BreakpointManager with DisposerMixin { ]); } + Future> _getBreakpointsForIsolate(String isolateId) async { + final isolate = await _service.getIsolate(isolateId); + if (isolate.id != _isolateRefId) { + // Current request is obsolete. + return []; + } + + // Ignore attempts from DWDS to re-establish breakpoints because DevTools is + // now in charge of re-establishing breakpoints: + final connectedToDwds = + serviceConnection.serviceManager.connectedApp?.isDartWebAppNow ?? false; + if (connectedToDwds) return []; + + return isolate.breakpoints ?? []; + } + + Future _updateBreakpoints({ + required List breakpoints, + required String isolateId, + }) async { + _breakpoints.value = breakpoints; + // Build _breakpointsWithLocation from _breakpoints. + await Future.wait( + _breakpoints.value.map(breakpointManager.createBreakpointWithLocation), + ).then((list) { + if (isolateId != _isolateRefId) { + // Current request is obsolete. + return; + } + _breakpointsWithLocation.value = list.toList()..sort(); + }); + } + + Future _setUpBreakpoints({ + required List breakpoints, + required IsolateRef isolateRef, + }) async { + final scriptUriToRef = await _scriptRefsForBreakpoints( + breakpoints: breakpoints, + isolateRef: isolateRef, + ); + + for (final breakpoint in breakpoints) { + final newScriptRef = scriptUriToRef[breakpoint.scriptUri]; + final breakpointLine = breakpoint.line; + + final scriptId = newScriptRef?.id; + if (scriptId != null && breakpointLine != null) { + await addBreakpoint(scriptId, breakpointLine); + } + } + } + + Future> _scriptRefsForBreakpoints({ + required List breakpoints, + required IsolateRef isolateRef, + }) async { + final bpScriptUris = breakpoints.fold( + {}, + (scriptSet, breakpoint) { + final scriptUri = breakpoint.scriptUri; + if (scriptUri != null) { + scriptSet.add(scriptUri); + } + return scriptSet; + }, + ); + + final newScripts = await scriptManager.retrieveAndSortScripts(isolateRef); + final scriptUriToRef = + newScripts.fold({}, (scriptMap, script) { + final scriptUri = script.uri; + if (scriptUri != null && bpScriptUris.contains(scriptUri)) { + scriptMap[scriptUri] = script; + } + return scriptMap; + }); + + return scriptUriToRef; + } + /// Return the list of valid positions for breakpoints for a given script. Future> getBreakablePositions( IsolateRef? isolateRef, @@ -239,7 +330,7 @@ class BreakpointManager with DisposerMixin { } } - void _handleDebugEvent(Event event) { + Future _handleDebugEvent(Event event) async { if (event.isolate!.id != _isolateRefId) return; switch (event.kind) { @@ -247,18 +338,20 @@ class BreakpointManager with DisposerMixin { // that knows how to notify when performing a list edit operation. case EventKind.kBreakpointAdded: final breakpoint = event.breakpoint!; + final isDuplicate = + _breakpoints.value.any((bp) => bp.id == breakpoint.id); + if (isDuplicate) break; _breakpoints.value = [..._breakpoints.value, breakpoint]; - unawaited( - breakpointManager.createBreakpointWithLocation(breakpoint).then((bp) { - final list = [ - ..._breakpointsWithLocation.value, - bp, - ]..sort(); - - _breakpointsWithLocation.value = list; - }), - ); + await breakpointManager + .createBreakpointWithLocation(breakpoint) + .then((bp) { + final list = [ + ..._breakpointsWithLocation.value, + bp, + ]..sort(); + _breakpointsWithLocation.value = list; + }); break; case EventKind.kBreakpointResolved: @@ -269,21 +362,30 @@ class BreakpointManager with DisposerMixin { breakpoint, ]; - unawaited( - breakpointManager.createBreakpointWithLocation(breakpoint).then((bp) { - final list = _breakpointsWithLocation.value.toList(); - // Remote the bp with the older, unresolved information from the list. - list.removeWhere((breakpoint) => bp.breakpoint.id == bp.id); - // Add the bp with the newer, resolved information. - list.add(bp); - list.sort(); - _breakpointsWithLocation.value = list; - }), - ); + await breakpointManager + .createBreakpointWithLocation(breakpoint) + .then((bp) { + final list = _breakpointsWithLocation.value; + // Remove the bp with the older, unresolved information from the list. + list.removeWhere((breakpoint) => breakpoint.id == bp.id); + // Add the bp with the newer, resolved information. + list.add(bp); + list.sort(); + _breakpointsWithLocation.value = list; + }); break; case EventKind.kBreakpointRemoved: + // Ignore any breakpoints removed during a hot restart, because the VM + // service removes them before resuming the isolate and then performing + // the restart. Note we only track hot restarts triggered by DevTools, + // if a hot-restart was triggered by another client we won't know. + // See https://github.com/flutter/flutter/issues/134470 + final hotRestartInProgress = serviceConnection + .serviceManager.isolateManager.hotRestartInProgress; + if (hotRestartInProgress) break; + final breakpoint = event.breakpoint; _breakpoints.value = [ diff --git a/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart b/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart index 48017694079..92141173827 100644 --- a/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart +++ b/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart @@ -82,7 +82,7 @@ class DebuggerController extends DisposableController /// Method to call after the vm service shuts down. void _onServiceShutdown() { - _clearCaches(); + _clearCaches(isServiceShutdown: true); _hasTruncatedFrames.value = false; unawaited(_getStackOperation?.cancel()); @@ -484,9 +484,9 @@ class DebuggerController extends DisposableController await _populateFrameInfo(stackInfo.frames, truncated: stackInfo.truncated); } - void _clearCaches() { + void _clearCaches({bool isServiceShutdown = false}) { _lastEvent = null; - breakpointManager.clearCache(); + breakpointManager.clearCache(isServiceShutdown: isServiceShutdown); } Future _populateScripts(Isolate isolate) async { diff --git a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md index cc6100775bb..cca4832f5b5 100644 --- a/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md +++ b/packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md @@ -46,6 +46,7 @@ mode. - [#7178](https://github.com/flutter/devtools/pull/7178) * Improved contrast of profiling details when displaying profiler hits in dark mode. - [#7178](https://github.com/flutter/devtools/pull/7178) * Fixed syntax highlighting for comments when the source file uses `\r\n` line endings [#7190](https://github.com/flutter/devtools/pull/7190) +* Re-establish breakpoints after a hot-restart. - [#7205](https://github.com/flutter/devtools/pull/7205) ## Network profiler updates diff --git a/packages/devtools_app/test/test_infra/fixtures/flutter_app/lib/main.dart b/packages/devtools_app/test/test_infra/fixtures/flutter_app/lib/main.dart index 07d9ec90b6c..1f7bfdb647f 100644 --- a/packages/devtools_app/test/test_infra/fixtures/flutter_app/lib/main.dart +++ b/packages/devtools_app/test/test_infra/fixtures/flutter_app/lib/main.dart @@ -43,7 +43,17 @@ class MyApp extends StatelessWidget { body: const Center( child: Text('Hello, World!'), ), + floatingActionButton: FloatingActionButton( + onPressed: _printHello, + tooltip: 'Say hi', + child: const Icon(Icons.abc), + ), ), ); } + + void _printHello() { + // ignore: avoid_print, for testing. + print('Hello!'); + } } diff --git a/packages/devtools_app/test/test_infra/goldens/integration_animated_physical_model_selected.png b/packages/devtools_app/test/test_infra/goldens/integration_animated_physical_model_selected.png index 96e0b310049..70a6872b2e3 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_animated_physical_model_selected.png and b/packages/devtools_app/test/test_infra/goldens/integration_animated_physical_model_selected.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_initial_load.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_initial_load.png index 1d80bdd8540..ee3d0e13f57 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_initial_load.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_initial_load.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_richtext_selected.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_richtext_selected.png index c9c3b3ad9fd..a9f61745316 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_richtext_selected.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_richtext_selected.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_scaffold_selected.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_scaffold_selected.png index b3c40243c79..2b66f6ada9a 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_scaffold_selected.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_scaffold_selected.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center.png index 995c7b658ec..0c2309ddab0 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center_details_tree.png b/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center_details_tree.png index 3cab0360140..21622672729 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center_details_tree.png and b/packages/devtools_app/test/test_infra/goldens/integration_inspector_select_center_details_tree.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_empty1.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_empty1.png index e73f0d8156b..8f259c26a9e 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/memory_diff_empty1.png and b/packages/devtools_app/test/test_infra/goldens/memory_diff_empty1.png differ diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_empty2.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_empty2.png index e73f0d8156b..8f259c26a9e 100644 Binary files a/packages/devtools_app/test/test_infra/goldens/memory_diff_empty2.png and b/packages/devtools_app/test/test_infra/goldens/memory_diff_empty2.png differ diff --git a/packages/devtools_app_shared/lib/src/service/isolate_manager.dart b/packages/devtools_app_shared/lib/src/service/isolate_manager.dart index 2548443fbd4..5aa305e9891 100644 --- a/packages/devtools_app_shared/lib/src/service/isolate_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/isolate_manager.dart @@ -50,6 +50,10 @@ final class IsolateManager with DisposerMixin { final _isolateRunnableCompleters = >{}; + // TODO(https://github.com/flutter/flutter/issues/134470): Track hot-restarts + // triggered by other clients. + bool hotRestartInProgress = false; + Future init(List isolates) async { await initIsolates(isolates); } @@ -132,6 +136,9 @@ final class IsolateManager with DisposerMixin { () => Completer(), ); isolateRunnable.complete(); + if (hotRestartInProgress) { + hotRestartInProgress = false; + } } else if (event.kind == EventKind.kIsolateStart && !event.isolate!.isSystemIsolate!) { await _registerIsolate(event.isolate!); diff --git a/packages/devtools_app_shared/lib/src/service/service_manager.dart b/packages/devtools_app_shared/lib/src/service/service_manager.dart index e4d7d856699..19a92329030 100644 --- a/packages/devtools_app_shared/lib/src/service/service_manager.dart +++ b/packages/devtools_app_shared/lib/src/service/service_manager.dart @@ -433,7 +433,12 @@ class ServiceManager { /// This can throw an [RPCError]. Future performHotRestart() async { - await callServiceOnMainIsolate(hotRestartServiceName); + isolateManager.hotRestartInProgress = true; + try { + await callServiceOnMainIsolate(hotRestartServiceName); + } catch (_) { + isolateManager.hotRestartInProgress = false; + } } }