Skip to content

Fix reading pause_isolates_on_start value from the DWDS #2398

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Fix reading pause_isolates_on_start value from the DWDS #2398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09b9284
Fix reading pause_isolates_on_start value from the DWDS
elliette Mar 26, 2024
b549582
Update tests
elliette Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions dwds/lib/dart_web_debug_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ class Dwds {
StreamController<DebugConnection> get extensionDebugConnections =>
_devHandler.extensionDebugConnections;

bool get shouldPauseIsolatesOnStart => _shouldPauseIsolatesOnStart;
bool _shouldPauseIsolatesOnStart = false;

Future<void> stop() async {
await _devTools?.close();
await _devHandler.close();
Expand All @@ -61,9 +58,6 @@ class Dwds {
final appDebugServices = await _devHandler.loadAppServices(appConnection);
final chromeProxyService = appDebugServices.chromeProxyService;
await chromeProxyService.isInitialized;
chromeProxyService.pauseIsolatesOnStartStream.listen((value) {
_shouldPauseIsolatesOnStart = value;
});
return DebugConnection(appDebugServices);
}

Expand Down Expand Up @@ -145,4 +139,7 @@ class Dwds {
debugSettings.enableDebugging,
);
}

bool shouldPauseIsolatesOnStart(String appId) =>
_devHandler.shouldPauseIsolatesOnStart(appId);
}
3 changes: 3 additions & 0 deletions dwds/lib/src/handlers/dev_handler.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class DevHandler {
_servicesByAppId.clear();
}();

bool shouldPauseIsolatesOnStart(String appId) =>
_servicesByAppId[appId]?.chromeProxyService.pauseIsolatesOnStart ?? false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this part - when do we expect _servicesByAppId[appId] to be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_servicesByAppId[appId] is null when the debug session has been closed (which is exactly what we want, because that means shouldPauseIsolatesOnStart will return false and the code runner which calls this won't wait for a resume event


void _emitBuildResults(BuildResult result) {
if (result.status != BuildStatus.succeeded) return;
for (var injectedConnection in _injectedConnections) {
Expand Down
9 changes: 0 additions & 9 deletions dwds/lib/src/services/chrome_proxy_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,6 @@ class ChromeProxyService implements VmServiceInterface {
/// This value can be updated at runtime via [setFlag].
bool get pauseIsolatesOnStart => _pauseIsolatesOnStart;

final _pauseIsolatesOnStartController = StreamController<bool>.broadcast();

/// A global stream of the value of the [_pauseIsolatesOnStartFlag].
///
/// The flag's value can be updated during runtime.
Stream<bool> get pauseIsolatesOnStartStream =>
_pauseIsolatesOnStartController.stream;

final _resumeAfterHotRestartEventsController =
StreamController<String>.broadcast();

Expand Down Expand Up @@ -1227,7 +1219,6 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
if (name == _pauseIsolatesOnStartFlag) {
assert(value == 'true' || value == 'false');
_pauseIsolatesOnStart = value == 'true';
_pauseIsolatesOnStartController.sink.add(_pauseIsolatesOnStart);
}

return Success();
Expand Down
24 changes: 10 additions & 14 deletions dwds/test/chrome_proxy_service_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2064,34 +2064,30 @@ void main() {
});

group('setFlag', () {
test('pause_isolates_on_start set to true', () async {
test('pause_isolates_on_start set to true', () {
final service = context.service;
expect(
service.setFlag('pause_isolates_on_start', 'true'),
completion(_isSuccess),
);
// Re-try until sucess because the value doesn't get updated
// synchronously (it is sent over a stream):
final pauseIsolatesOnStart = await retryFn(
() => context.dwds!.shouldPauseIsolatesOnStart,
expectedResult: true,
final appId = context.appConnection.request.appId;
expect(
context.dwds!.shouldPauseIsolatesOnStart(appId),
equals(true),
);
expect(pauseIsolatesOnStart, equals(true));
});

test('pause_isolates_on_start set to false', () async {
test('pause_isolates_on_start set to false', () {
final service = context.service;
expect(
service.setFlag('pause_isolates_on_start', 'false'),
completion(_isSuccess),
);
// Re-try until sucess because the value doesn't get updated
// synchronously (it is sent over a stream):
final pauseIsolatesOnStart = await retryFn(
() => context.dwds!.shouldPauseIsolatesOnStart,
expectedResult: false,
final appId = context.appConnection.request.appId;
expect(
context.dwds!.shouldPauseIsolatesOnStart(appId),
equals(false),
);
expect(pauseIsolatesOnStart, equals(false));
});

test('pause_isolates_on_start set to invalid value', () {
Expand Down