diff --git a/packages/devtools_app/lib/src/screens/debugger/controls.dart b/packages/devtools_app/lib/src/screens/debugger/controls.dart index 7db8774ef63..78943e01ec0 100644 --- a/packages/devtools_app/lib/src/screens/debugger/controls.dart +++ b/packages/devtools_app/lib/src/screens/debugger/controls.dart @@ -50,13 +50,7 @@ class _DebuggingControlsState extends State @override Widget build(BuildContext context) { final resuming = controller.resuming.value; - final hasStackFrames = controller.stackFramesWithLocation.value.isNotEmpty; - final isSystemIsolate = controller.isSystemIsolate; - final canStep = - serviceConnection.serviceManager.isMainIsolatePaused && - !resuming && - hasStackFrames && - !isSystemIsolate; + final canStep = controller.canStep; final isVmApp = serviceConnection.serviceManager.connectedApp?.isRunningOnDartVM ?? false; 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 aacc0ec31a1..52c89b01499 100644 --- a/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart +++ b/packages/devtools_app/lib/src/screens/debugger/debugger_controller.dart @@ -203,6 +203,18 @@ class DebuggerController extends DevToolsScreenController bool get isSystemIsolate => _isolate.value?.isSystemIsolate ?? false; + /// Whether the debugger is currently in a state where step operations + /// (Step Over / In / Out) can be performed. + /// + /// Used by the debugger control buttons in `controls.dart` and by the + /// keyboard-shortcut Actions in `debugger_screen.dart` so that both + /// surfaces gate on the same conditions. + bool get canStep => + serviceConnection.serviceManager.isMainIsolatePaused && + !resuming.value && + stackFramesWithLocation.value.isNotEmpty && + !isSystemIsolate; + String get _isolateRefId { final id = _isolate.value?.id; if (id == null) return ''; diff --git a/packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart b/packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart index 80745175e6f..9d4723af8c1 100644 --- a/packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart +++ b/packages/devtools_app/lib/src/screens/debugger/debugger_screen.dart @@ -54,12 +54,22 @@ class DebuggerScreen extends Screen { searchInFileKeySet: SearchInFileIntent(controller), escapeKeySet: EscapeIntent(controller), openFileKeySet: OpenFileIntent(controller), + pauseResumeKeySet: PauseResumeIntent(controller), + nextStackFrameKeySet: NextStackFrameIntent(controller), + stepOverKeySet: StepOverIntent(controller), + stepInKeySet: StepInIntent(controller), + stepOutKeySet: StepOutIntent(controller), }; final actions = >{ GoToLineNumberIntent: GoToLineNumberAction(), SearchInFileIntent: SearchInFileAction(), EscapeIntent: EscapeAction(), OpenFileIntent: OpenFileAction(), + PauseResumeIntent: PauseResumeAction(), + NextStackFrameIntent: NextStackFrameAction(), + StepOverIntent: StepOverAction(), + StepInIntent: StepInAction(), + StepOutIntent: StepOutAction(), }; return ShortcutsConfiguration(shortcuts: shortcuts, actions: actions); } @@ -412,6 +422,120 @@ class OpenFileAction extends Action { } } +// ----- Stepping / pause keyboard intents (issue #3867) ----------------------- +// +// Each Action delegates to `DebuggerController` for both the gating +// (`isEnabled`) and the side-effecting call (`invoke`), so the keyboard +// shortcuts stay in lock-step with the debugger buttons in `controls.dart`. + +class PauseResumeIntent extends Intent { + const PauseResumeIntent(this._controller); + + final DebuggerController _controller; +} + +class PauseResumeAction extends Action { + @override + bool isEnabled(PauseResumeIntent intent) { + if (intent._controller.isSystemIsolate) { + return false; + } + final isPaused = serviceConnection.serviceManager.isMainIsolatePaused; + if (isPaused) { + // When paused, the button becomes "Resume" and is disabled while a + // resume is already in flight. + return !intent._controller.resuming.value; + } + return true; + } + + @override + void invoke(PauseResumeIntent intent) { + final controller = intent._controller; + if (serviceConnection.serviceManager.isMainIsolatePaused) { + unawaited(controller.resume()); + } else { + unawaited(controller.pause()); + } + } +} + +class NextStackFrameIntent extends Intent { + const NextStackFrameIntent(this._controller); + + final DebuggerController _controller; +} + +class NextStackFrameAction extends Action { + @override + bool isEnabled(NextStackFrameIntent intent) => + intent._controller.stackFramesWithLocation.value.length > 1; + + @override + void invoke(NextStackFrameIntent intent) { + final controller = intent._controller; + final frames = controller.stackFramesWithLocation.value; + final currentlySelected = controller.selectedStackFrame.value; + final currentIndex = currentlySelected == null + ? -1 + : frames.indexWhere( + (frame) => frame.frame.index == currentlySelected.frame.index, + ); + // Cycle through frames so repeated presses walk the call stack and wrap + // back to the top. + final nextIndex = (currentIndex + 1) % frames.length; + unawaited(controller.selectStackFrame(frames[nextIndex])); + } +} + +class StepOverIntent extends Intent { + const StepOverIntent(this._controller); + + final DebuggerController _controller; +} + +class StepOverAction extends Action { + @override + bool isEnabled(StepOverIntent intent) => intent._controller.canStep; + + @override + void invoke(StepOverIntent intent) { + unawaited(intent._controller.stepOver()); + } +} + +class StepInIntent extends Intent { + const StepInIntent(this._controller); + + final DebuggerController _controller; +} + +class StepInAction extends Action { + @override + bool isEnabled(StepInIntent intent) => intent._controller.canStep; + + @override + void invoke(StepInIntent intent) { + unawaited(intent._controller.stepIn()); + } +} + +class StepOutIntent extends Intent { + const StepOutIntent(this._controller); + + final DebuggerController _controller; +} + +class StepOutAction extends Action { + @override + bool isEnabled(StepOutIntent intent) => intent._controller.canStep; + + @override + void invoke(StepOutIntent intent) { + unawaited(intent._controller.stepOut()); + } +} + class DebuggerStatus extends StatefulWidget { const DebuggerStatus({super.key, required this.controller}); diff --git a/packages/devtools_app/lib/src/screens/debugger/key_sets.dart b/packages/devtools_app/lib/src/screens/debugger/key_sets.dart index 9efc9bd576e..c3c5d3d5305 100644 --- a/packages/devtools_app/lib/src/screens/debugger/key_sets.dart +++ b/packages/devtools_app/lib/src/screens/debugger/key_sets.dart @@ -38,3 +38,23 @@ final openFileKeySet = LogicalKeySet( final openFileKeySetDescription = openFileKeySet.describeKeys( isMacOS: HostPlatform.instance.isMacOS, ); + +// Debugger stepping / pause shortcuts. +// +// Bindings mirror Chrome DevTools (F8 / F9 / F10 / F11 / Shift+F11), which +// VS Code also uses for the stepping triplet. Aligning with these surfaces +// keeps the debugger feel familiar across IDEs. +// +// See https://github.com/flutter/devtools/issues/3867. +final pauseResumeKeySet = LogicalKeySet(LogicalKeyboardKey.f8); + +final nextStackFrameKeySet = LogicalKeySet(LogicalKeyboardKey.f9); + +final stepOverKeySet = LogicalKeySet(LogicalKeyboardKey.f10); + +final stepInKeySet = LogicalKeySet(LogicalKeyboardKey.f11); + +final stepOutKeySet = LogicalKeySet( + LogicalKeyboardKey.shift, + LogicalKeyboardKey.f11, +); diff --git a/packages/devtools_app/test/screens/debugger/debugger_keyboard_shortcuts_test.dart b/packages/devtools_app/test/screens/debugger/debugger_keyboard_shortcuts_test.dart new file mode 100644 index 00000000000..262208c5252 --- /dev/null +++ b/packages/devtools_app/test/screens/debugger/debugger_keyboard_shortcuts_test.dart @@ -0,0 +1,209 @@ +// Copyright 2026 The Flutter Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd. + +import 'package:devtools_app/devtools_app.dart'; +import 'package:devtools_app/src/screens/debugger/debugger_model.dart'; +import 'package:devtools_app/src/screens/debugger/debugger_screen.dart'; +import 'package:devtools_app_shared/ui.dart'; +import 'package:devtools_app_shared/utils.dart'; +import 'package:devtools_test/devtools_test.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:mockito/mockito.dart'; +import 'package:vm_service/vm_service.dart'; + +// Regression tests for the debugger keyboard shortcuts added for +// https://github.com/flutter/devtools/issues/3867. Each Action's `isEnabled` +// and `invoke` is exercised directly so the gating logic does not depend on +// pumping the full debugger widget tree. + +void main() { + late FakeServiceConnectionManager fakeServiceConnection; + late MockDebuggerController debuggerController; + + StackFrameAndSourcePosition makeFrame(int index) { + return StackFrameAndSourcePosition( + Frame(index: index, kind: FrameKind.kRegular), + ); + } + + setUp(() { + fakeServiceConnection = FakeServiceConnectionManager(); + mockConnectedApp(fakeServiceConnection.serviceManager.connectedApp!); + setGlobal(ServiceConnectionManager, fakeServiceConnection); + setGlobal(IdeTheme, IdeTheme()); + setGlobal(NotificationService, NotificationService()); + + debuggerController = createMockDebuggerControllerWithDefaults(); + when(debuggerController.isSystemIsolate).thenReturn(false); + when(debuggerController.resuming).thenReturn(ValueNotifier(false)); + when(debuggerController.stackFramesWithLocation).thenReturn( + ValueNotifier>( + [], + ), + ); + when( + debuggerController.selectedStackFrame, + ).thenReturn(ValueNotifier(null)); + }); + + group('PauseResumeAction', () { + test('calls pause when not paused', () { + fakeServiceConnection.serviceManager.isMainIsolatePaused = false; + when(debuggerController.pause()).thenAnswer((_) async => Success()); + + final action = PauseResumeAction(); + final intent = PauseResumeIntent(debuggerController); + expect(action.isEnabled(intent), isTrue); + action.invoke(intent); + + verify(debuggerController.pause()).called(1); + verifyNever(debuggerController.resume()); + }); + + test('calls resume when paused', () { + fakeServiceConnection.serviceManager.isMainIsolatePaused = true; + when(debuggerController.resume()).thenAnswer((_) async => Success()); + + final action = PauseResumeAction(); + final intent = PauseResumeIntent(debuggerController); + expect(action.isEnabled(intent), isTrue); + action.invoke(intent); + + verify(debuggerController.resume()).called(1); + verifyNever(debuggerController.pause()); + }); + + test('isEnabled is false on a system isolate', () { + when(debuggerController.isSystemIsolate).thenReturn(true); + final action = PauseResumeAction(); + expect(action.isEnabled(PauseResumeIntent(debuggerController)), isFalse); + }); + + test('isEnabled is false when already resuming a paused isolate', () { + fakeServiceConnection.serviceManager.isMainIsolatePaused = true; + when(debuggerController.resuming).thenReturn(ValueNotifier(true)); + final action = PauseResumeAction(); + expect(action.isEnabled(PauseResumeIntent(debuggerController)), isFalse); + }); + }); + + group('NextStackFrameAction', () { + test('isEnabled is false with fewer than 2 frames', () { + when(debuggerController.stackFramesWithLocation).thenReturn( + ValueNotifier>( + [makeFrame(0)], + ), + ); + final action = NextStackFrameAction(); + expect( + action.isEnabled(NextStackFrameIntent(debuggerController)), + isFalse, + ); + }); + + test('selects the next frame after the currently selected one', () { + final frame0 = makeFrame(0); + final frame1 = makeFrame(1); + final frame2 = makeFrame(2); + when(debuggerController.stackFramesWithLocation).thenReturn( + ValueNotifier>( + [frame0, frame1, frame2], + ), + ); + when( + debuggerController.selectedStackFrame, + ).thenReturn(ValueNotifier(frame0)); + when(debuggerController.selectStackFrame(any)).thenAnswer((_) async {}); + + NextStackFrameAction().invoke(NextStackFrameIntent(debuggerController)); + + verify(debuggerController.selectStackFrame(frame1)).called(1); + }); + + test('wraps from the bottom frame back to the top', () { + final frame0 = makeFrame(0); + final frame1 = makeFrame(1); + when(debuggerController.stackFramesWithLocation).thenReturn( + ValueNotifier>( + [frame0, frame1], + ), + ); + when( + debuggerController.selectedStackFrame, + ).thenReturn(ValueNotifier(frame1)); + when(debuggerController.selectStackFrame(any)).thenAnswer((_) async {}); + + NextStackFrameAction().invoke(NextStackFrameIntent(debuggerController)); + + verify(debuggerController.selectStackFrame(frame0)).called(1); + }); + + test('selects the first frame when nothing is selected', () { + final frame0 = makeFrame(0); + final frame1 = makeFrame(1); + when(debuggerController.stackFramesWithLocation).thenReturn( + ValueNotifier>( + [frame0, frame1], + ), + ); + when( + debuggerController.selectedStackFrame, + ).thenReturn(ValueNotifier(null)); + when(debuggerController.selectStackFrame(any)).thenAnswer((_) async {}); + + NextStackFrameAction().invoke(NextStackFrameIntent(debuggerController)); + + verify(debuggerController.selectStackFrame(frame0)).called(1); + }); + }); + + // The Step actions delegate their gating to `DebuggerController.canStep`, + // which is its own well-defined property; we stub it directly here rather + // than re-asserting every condition that feeds into it. + group('Step actions', () { + test('StepOverAction calls stepOver when canStep is true', () { + when(debuggerController.canStep).thenReturn(true); + when(debuggerController.stepOver()).thenAnswer((_) async => Success()); + final action = StepOverAction(); + expect(action.isEnabled(StepOverIntent(debuggerController)), isTrue); + action.invoke(StepOverIntent(debuggerController)); + verify(debuggerController.stepOver()).called(1); + }); + + test('StepInAction calls stepIn when canStep is true', () { + when(debuggerController.canStep).thenReturn(true); + when(debuggerController.stepIn()).thenAnswer((_) async => Success()); + final action = StepInAction(); + expect(action.isEnabled(StepInIntent(debuggerController)), isTrue); + action.invoke(StepInIntent(debuggerController)); + verify(debuggerController.stepIn()).called(1); + }); + + test('StepOutAction calls stepOut when canStep is true', () { + when(debuggerController.canStep).thenReturn(true); + when(debuggerController.stepOut()).thenAnswer((_) async => Success()); + final action = StepOutAction(); + expect(action.isEnabled(StepOutIntent(debuggerController)), isTrue); + action.invoke(StepOutIntent(debuggerController)); + verify(debuggerController.stepOut()).called(1); + }); + + test('step actions are disabled when canStep is false', () { + when(debuggerController.canStep).thenReturn(false); + expect( + StepOverAction().isEnabled(StepOverIntent(debuggerController)), + isFalse, + ); + expect( + StepInAction().isEnabled(StepInIntent(debuggerController)), + isFalse, + ); + expect( + StepOutAction().isEnabled(StepOutIntent(debuggerController)), + isFalse, + ); + }); + }); +}