Skip to content

Commit

Permalink
Set requireUserPermissionsToResume based on pause-isolates-on-start a…
Browse files Browse the repository at this point in the history
…nd pause-isolates-on-exit flags

Bug: flutter/devtools#7231
Change-Id: I4665cce72ad9b505cbf513ea151698d5de4eb3f5
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/360441
Commit-Queue: Elliott Brooks <elliottbrooks@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
  • Loading branch information
elliette authored and Commit Queue committed Apr 8, 2024
1 parent 51d6170 commit 9382e58
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 11 deletions.
2 changes: 2 additions & 0 deletions pkg/dds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
and prepared DDS for using `unified_analytics` through the Dart Tooling Daemon.
- Internal change: removed `analytics` parameter from the DevTools server `defaultHandler` method.
- Updated `README.md` and added contributing guide (`CONTRIBUTING.md`).
- Updated `package:dds_service_extensions` constraint to ^2.0.0.
- Determine default `requireUserPermissionToResume` values from the `pause_isolates_on_start` and `pause_isolates_on_exit` flags.

# 4.0.0
- Updated DDS protocol to version 2.0.
Expand Down
58 changes: 50 additions & 8 deletions pkg/dds/lib/src/isolate_manager.dart
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ class IsolateManager {
);
}
}
await _determineRequireUserPermissionToResumeFromFlags();
_initialized = true;
}

Expand Down Expand Up @@ -383,14 +384,10 @@ class IsolateManager {
DartDevelopmentServiceClient client,
json_rpc.Parameters parameters,
) async {
int pauseTypeMask = 0;
if (parameters['onPauseStart'].asBoolOr(false)) {
pauseTypeMask |= PauseTypeMasks.pauseOnStartMask;
}
if (parameters['onPauseExit'].asBoolOr(false)) {
pauseTypeMask |= PauseTypeMasks.pauseOnExitMask;
}
_requireUserPermissionToResumeMask = pauseTypeMask;
_setRequireUserPermissionToResume(
onPauseStart: parameters['onPauseStart'].asBoolOr(false),
onPauseExit: parameters['onPauseExit'].asBoolOr(false),
);

// Check if isolates have been waiting for a user resume and resume any
// isolates that no longer need to wait for a user resume.
Expand All @@ -415,6 +412,51 @@ class IsolateManager {
};
}

Future<void> _determineRequireUserPermissionToResumeFromFlags() async {
try {
final result = await dds.vmServiceClient.sendRequest('getFlagList')
as Map<String, dynamic>;
final flagList = FlagList.parse(result);
final flags = flagList!.flags!;

bool? pauseOnStartValue;
bool? pauseOnExitValue;
for (final flag in flags) {
if (flag.name == 'pause_isolates_on_start') {
pauseOnStartValue = flag.valueAsString == 'true';
}
if (flag.name == 'pause_isolates_on_exit') {
pauseOnExitValue = flag.valueAsString == 'true';
}
if (pauseOnStartValue != null && pauseOnExitValue != null) {
break;
}
}

_setRequireUserPermissionToResume(
onPauseStart: pauseOnStartValue ?? false,
onPauseExit: pauseOnExitValue ?? false,
);
} catch (_) {
// Swallow any errors. Otherwise, this will cause initialization to
// silently fail.
}
}

void _setRequireUserPermissionToResume({
required bool onPauseStart,
required bool onPauseExit,
}) {
int pauseTypeMask = 0;
if (onPauseStart) {
pauseTypeMask |= PauseTypeMasks.pauseOnStartMask;
}
if (onPauseExit) {
pauseTypeMask |= PauseTypeMasks.pauseOnExitMask;
}
_requireUserPermissionToResumeMask = pauseTypeMask;
}

Future<Map<String, dynamic>> getCachedCpuSamples(
json_rpc.Parameters parameters) async {
final isolateId = parameters['isolateId'].asString;
Expand Down
2 changes: 1 addition & 1 deletion pkg/dds/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ dependencies:
async: ^2.4.1
browser_launcher: ^1.0.0
collection: ^1.15.0
dds_service_extensions: ^1.6.0
dds_service_extensions: ^2.0.0
dap: ^1.2.0
extension_discovery: ^2.0.0
devtools_shared: ^8.0.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void fooBar() {
final test = <IsolateTest>[
hasPausedAtStart,
(VmService service, IsolateRef isolate) async {
service.requireUserPermissionToResume(onPauseStart: false);
final isolateId = isolate.id!;
final client1 = await createClient(
service: service,
Expand Down
4 changes: 2 additions & 2 deletions pkg/dds/test/client_resume_approvals_disconnect_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ final test = <IsolateTest>[
// Multiple clients, disconnect client awaiting approval.
hasPausedAtStart,
(VmService service, IsolateRef isolate) async {
service.requireUserPermissionToResume(onPauseStart: false);
final isolateId = isolate.id!;
final client1 = await createClient(
service: service,
Expand All @@ -43,8 +44,7 @@ final test = <IsolateTest>[
await hasPausedAtStart(service, isolate);

// Once client2 disconnects, there are no clients which require resume
// approval. Ensure we resume immediately so we don't deadlock waiting for
// approvals from disconnected clients.
// approval.
await client2.dispose();
},
hasStoppedAtExit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ void fooBar() {
final test = <IsolateTest>[
// Multiple clients, same client names.
(VmService service, IsolateRef isolateRef) async {
service.requireUserPermissionToResume(onPauseStart: false);
// ignore: unused_local_variable
final client1 = await createClient(
service: service,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ void fooBar() {
final test = <IsolateTest>[
// Multiple clients, different client names.
(VmService service, IsolateRef isolateRef) async {
service.requireUserPermissionToResume(onPauseStart: false);
final isolateId = isolateRef.id!;
final client1 = await createClient(
service: service,
Expand Down
1 change: 1 addition & 0 deletions pkg/dds/test/client_resume_approvals_name_change_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ void fooBar() {
final test = <IsolateTest>[
// Remove required approvals via name change.
(VmService service, IsolateRef isolateRef) async {
service.requireUserPermissionToResume(onPauseStart: false);
final isolateId = isolateRef.id!;

// Create two clients with the same name.
Expand Down

0 comments on commit 9382e58

Please sign in to comment.