From f9b7592d91742c927f04af4e9fed13f4932be9b9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 13:24:14 -0700 Subject: [PATCH 1/6] Fix error handling issue and add test --- .../src/services/chrome_proxy_service.dart | 52 +++++++++---------- dwds/test/chrome_proxy_service_test.dart | 10 +++- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index 479bc2faf..cc9a134c0 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -1080,6 +1080,17 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( isolateId, step: step, frameIndex: frameIndex, + ).catchError( + (error) => Future.error( + RPCError( + 'resume', + RPCErrorKind.kIsolateMustBePaused.code, + error, + ), + ), + test: (e) => e.toString().contains( + 'Can only perform operation while paused', + ), ), ); @@ -1088,33 +1099,20 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( String? step, int? frameIndex, }) async { - try { - if (inspector.appConnection.isStarted) { - return captureElapsedTime( - () async { - await isInitialized; - await isStarted; - _checkIsolate('resume', isolateId); - return await (await debuggerFuture) - .resume(step: step, frameIndex: frameIndex); - }, - (result) => DwdsEvent.resume(step), - ); - } else { - inspector.appConnection.runMain(); - return Success(); - } - } on WipError catch (e) { - final errorMessage = e.message; - if (errorMessage != null && - errorMessage.contains('Can only perform operation while paused')) { - throw RPCError( - 'resume', - RPCErrorKind.kIsolateMustBePaused.code, - errorMessage, - ); - } - rethrow; + if (inspector.appConnection.isStarted) { + return captureElapsedTime( + () async { + await isInitialized; + await isStarted; + _checkIsolate('resume', isolateId); + return await (await debuggerFuture) + .resume(step: step, frameIndex: frameIndex); + }, + (result) => DwdsEvent.resume(step), + ); + } else { + inspector.appConnection.runMain(); + return Success(); } } diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart index 988a5977e..b366c686c 100644 --- a/dwds/test/chrome_proxy_service_test.dart +++ b/dwds/test/chrome_proxy_service_test.dart @@ -1368,12 +1368,18 @@ void main() { expect(pauseBreakpoints, hasLength(1)); expect(pauseBreakpoints.first.id, bp.id); await service.removeBreakpoint(isolateId!, bp.id!); - }); - tearDown(() async { // Resume execution to not impact other tests. await service.resume(isolateId!); }); + + test('resuming throws error if not paused', () async { + await expectLater( + service.resume(isolateId!), + // Note: 106 is the `kIsolateMustBePaused` error code. + throwsRPCErrorWithMessage('106'), + ); + }); }); group('Step', () { From 45ab6ac5269bebde8a0e2a04b124624e5237fa72 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 13:53:26 -0700 Subject: [PATCH 2/6] Update fix and test --- dwds/lib/src/debugging/debugger.dart | 61 +++++++++++-------- .../src/services/chrome_proxy_service.dart | 11 ---- dwds/test/chrome_proxy_service_test.dart | 6 +- dwds/test/fixtures/context.dart | 4 ++ 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index 27e4c1b54..e506205ba 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -121,32 +121,45 @@ class Debugger extends Domain { /// Note that stepping will automatically continue until Chrome is paused at /// a location for which we have source information. Future resume({String? step, int? frameIndex}) async { - if (frameIndex != null) { - throw ArgumentError('FrameIndex is currently unsupported.'); - } - WipResponse? result; - if (step != null) { - _isStepping = true; - switch (step) { - case 'Over': - result = await _remoteDebugger.stepOver(); - break; - case 'Out': - result = await _remoteDebugger.stepOut(); - break; - case 'Into': - result = await _remoteDebugger.stepInto(); - break; - default: - throwInvalidParam('resume', 'Unexpected value for step: $step'); + try { + if (frameIndex != null) { + throw ArgumentError('FrameIndex is currently unsupported.'); } - } else { - _isStepping = false; - _previousSteppingLocation = null; - result = await _remoteDebugger.resume(); + WipResponse? result; + if (step != null) { + _isStepping = true; + switch (step) { + case 'Over': + result = await _remoteDebugger.stepOver(); + break; + case 'Out': + result = await _remoteDebugger.stepOut(); + break; + case 'Into': + result = await _remoteDebugger.stepInto(); + break; + default: + throwInvalidParam('resume', 'Unexpected value for step: $step'); + } + } else { + _isStepping = false; + _previousSteppingLocation = null; + result = await _remoteDebugger.resume(); + } + handleErrorIfPresent(result); + return Success(); + } on WipError catch (e) { + final errorMessage = e.message; + if (errorMessage != null && + errorMessage.contains('Can only perform operation while paused')) { + throw RPCError( + 'resume', + RPCErrorKind.kIsolateMustBePaused.code, + errorMessage, + ); + } + rethrow; } - handleErrorIfPresent(result); - return Success(); } /// Returns the current Dart stack for the paused debugger. diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index cc9a134c0..70d45fcbc 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -1080,17 +1080,6 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension( isolateId, step: step, frameIndex: frameIndex, - ).catchError( - (error) => Future.error( - RPCError( - 'resume', - RPCErrorKind.kIsolateMustBePaused.code, - error, - ), - ), - test: (e) => e.toString().contains( - 'Can only perform operation while paused', - ), ), ); diff --git a/dwds/test/chrome_proxy_service_test.dart b/dwds/test/chrome_proxy_service_test.dart index b366c686c..c2c1dc3a9 100644 --- a/dwds/test/chrome_proxy_service_test.dart +++ b/dwds/test/chrome_proxy_service_test.dart @@ -1373,11 +1373,11 @@ void main() { await service.resume(isolateId!); }); - test('resuming throws error if not paused', () async { + test('resuming throws kIsolateMustBePaused error if not paused', + () async { await expectLater( service.resume(isolateId!), - // Note: 106 is the `kIsolateMustBePaused` error code. - throwsRPCErrorWithMessage('106'), + throwsRPCErrorWithCode(RPCErrorKind.kIsolateMustBePaused.code), ); }); }); diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index fe010249d..c82af954c 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -55,6 +55,10 @@ Matcher isRPCErrorWithMessage(String message) => Matcher throwsRPCErrorWithMessage(String message) => throwsA(isRPCErrorWithMessage(message)); +Matcher isRPCErrorWithCode(int code) => + isA().having((e) => e.code, 'code', equals(code)); +Matcher throwsRPCErrorWithCode(int code) => throwsA(isRPCErrorWithCode(code)); + enum CompilationMode { buildDaemon, frontendServer } class TestContext { From 40ecf1847794036ee5f965d972709d2901f92e25 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:01:04 -0700 Subject: [PATCH 3/6] Prepare branch for release --- dwds/CHANGELOG.md | 4 ++++ dwds/lib/src/version.dart | 2 +- dwds/pubspec.yaml | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 55f5580b7..87f1c78e9 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,7 @@ +## 19.0.1+1 + +- Fix for Flutter crash when resuming and app is not paused. - [#132160](https://github.com/flutter/flutter/issues/132160) + ## 19.0.1 - Do not show async frame errors on evaluation. - [#2073](https://github.com/dart-lang/webdev/pull/2073) diff --git a/dwds/lib/src/version.dart b/dwds/lib/src/version.dart index df97ea76b..b49f7f857 100644 --- a/dwds/lib/src/version.dart +++ b/dwds/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '19.0.1'; +const packageVersion = '19.0.1+1'; diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index ac7e11d3e..be5b4f4d5 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,6 +1,6 @@ name: dwds # Every time this changes you need to run `dart run build_runner build`. -version: 19.0.1 +version: 19.0.1+1 description: >- A service that proxies between the Chrome debug protocol and the Dart VM service protocol. From 255dabd3386f12769f9e9c76fb5b7d16e7579040 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:34:50 -0700 Subject: [PATCH 4/6] Fix devtools test - https://github.com/dart-lang/webdev/pull/2182 --- dwds/test/devtools_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/devtools_test.dart b/dwds/test/devtools_test.dart index 2dfa86f5a..961cae076 100644 --- a/dwds/test/devtools_test.dart +++ b/dwds/test/devtools_test.dart @@ -95,7 +95,7 @@ void main() { final devToolsWindow = windows.firstWhere((window) => window != newAppWindow); await devToolsWindow.setAsActive(); - expect(await context.webDriver.title, equals('Dart DevTools')); + expect(await context.webDriver.pageSource, contains('DevTools')); }); test( From 6f372ad4a56e745e794f277265ed8e64ad2972be Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:05:15 -0700 Subject: [PATCH 5/6] Skip events test - https://github.com/dart-lang/webdev/pull/2182 --- dwds/test/events_test.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dwds/test/events_test.dart b/dwds/test/events_test.dart index e8ddd4351..3002da255 100644 --- a/dwds/test/events_test.dart +++ b/dwds/test/events_test.dart @@ -157,7 +157,9 @@ void main() { ], () => keyboard.sendChord([Keyboard.alt, 'd']), ); - }); + }, + skip: 'https://github.com/dart-lang/webdev/issues/2181', + ); test('emits DEVTOOLS_LAUNCH event', () async { await expectEventDuring( From d6f7d26b29086b3f5f2dcef7c00dad5f1ac6724c Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Wed, 9 Aug 2023 15:11:42 -0700 Subject: [PATCH 6/6] Format --- dwds/test/events_test.dart | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/dwds/test/events_test.dart b/dwds/test/events_test.dart index 3002da255..13e38c840 100644 --- a/dwds/test/events_test.dart +++ b/dwds/test/events_test.dart @@ -143,20 +143,22 @@ void main() { await context.tearDown(); }); - test('emits DEBUGGER_READY and DEVTOOLS_LOAD events', () async { - await expectEventsDuring( - [ - matchesEvent(DwdsEventKind.debuggerReady, { - 'elapsedMilliseconds': isNotNull, - 'screen': equals('debugger'), - }), - matchesEvent(DwdsEventKind.devToolsLoad, { - 'elapsedMilliseconds': isNotNull, - 'screen': equals('debugger'), - }), - ], - () => keyboard.sendChord([Keyboard.alt, 'd']), - ); + test( + 'emits DEBUGGER_READY and DEVTOOLS_LOAD events', + () async { + await expectEventsDuring( + [ + matchesEvent(DwdsEventKind.debuggerReady, { + 'elapsedMilliseconds': isNotNull, + 'screen': equals('debugger'), + }), + matchesEvent(DwdsEventKind.devToolsLoad, { + 'elapsedMilliseconds': isNotNull, + 'screen': equals('debugger'), + }), + ], + () => keyboard.sendChord([Keyboard.alt, 'd']), + ); }, skip: 'https://github.com/dart-lang/webdev/issues/2181', );