Skip to content

Commit

Permalink
Wait for a resume event to run the main() method after a page ref…
Browse files Browse the repository at this point in the history
…resh (#2431)
  • Loading branch information
elliette authored May 14, 2024
1 parent d46cf50 commit 99abc53
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 22 deletions.
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 24.1.0-wip

- Fix bug where debugging clients are not aware of service extensions when connecting to a new web app. - [#2388](https://github.com/dart-lang/webdev/pull/2388)
- Respect the value of `pause_isolates_on_start` during page-refreshes. - [#2431](https://github.com/dart-lang/webdev/pull/2431)

## 24.0.0

Expand Down
3 changes: 0 additions & 3 deletions dwds/lib/dart_web_debug_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,4 @@ class Dwds {
debugSettings.enableDebugging,
);
}

bool shouldPauseIsolatesOnStart(String appId) =>
_devHandler.shouldPauseIsolatesOnStart(appId);
}
9 changes: 8 additions & 1 deletion dwds/lib/src/connections/app_connection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ class AppConnection {
final _startedCompleter = Completer<void>();
final _doneCompleter = Completer<void>();
final SocketConnection _connection;
final Future<void> _readyToRunMain;

AppConnection(this.request, this._connection) {
AppConnection(this.request, this._connection, this._readyToRunMain) {
safeUnawaited(_connection.sink.done.then((v) => _doneCompleter.complete()));
}

Expand All @@ -34,6 +35,12 @@ class AppConnection {
if (_startedCompleter.isCompleted) {
throw StateError('Main has already started.');
}

safeUnawaited(_runMain());
}

Future<void> _runMain() async {
await _readyToRunMain;
_connection.sink.add(jsonEncode(serializers.serialize(RunRequest())));
_startedCompleter.complete();
}
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/dwds_vm_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void _waitForResumeEventToRunMain(
final issuedReadyToRunMainCompleter = Completer<void>();

final resumeEventsSubscription =
chromeProxyService.resumeAfterHotRestartEventsStream.listen((_) async {
chromeProxyService.resumeAfterRestartEventsStream.listen((_) async {
await chromeProxyService.inspector.jsEvaluate('\$dartReadyToRunMain();');
if (!issuedReadyToRunMainCompleter.isCompleted) {
issuedReadyToRunMainCompleter.complete();
Expand Down
50 changes: 46 additions & 4 deletions dwds/lib/src/handlers/dev_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ class DevHandler {
_servicesByAppId.clear();
}();

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

void _emitBuildResults(BuildResult result) {
if (result.status != BuildStatus.succeeded) return;
for (var injectedConnection in _injectedConnections) {
Expand Down Expand Up @@ -441,7 +438,12 @@ class DevHandler {
// were previously launched and create the new isolate.
final services = _servicesByAppId[message.appId];
final existingConnection = _appConnectionByAppId[message.appId];
final connection = AppConnection(message, sseConnection);
// Completer to indicate when the app's main() method is ready to be run.
// Its future is passed to the AppConnection so that it can be awaited on
// before running the app's main() method:
final readyToRunMainCompleter = Completer<void>();
final connection =
AppConnection(message, sseConnection, readyToRunMainCompleter.future);

// We can take over a connection if there is no connectedInstanceId (this
// means the client completely disconnected), or if the existing
Expand All @@ -460,13 +462,53 @@ class DevHandler {

// Reconnect to existing service.
services.connectedInstanceId = message.instanceId;

if (services.chromeProxyService.pauseIsolatesOnStart) {
// If the pause-isolates-on-start flag is set, we need to wait for
// the resume event to run the app's main() method.
_waitForResumeEventToRunMain(
services.chromeProxyService.resumeAfterRestartEventsStream,
readyToRunMainCompleter,
);
} else {
// Otherwise, we can run the app's main() method immediately.
readyToRunMainCompleter.complete();
}

await services.chromeProxyService.createIsolate(connection);
} else {
// If this is the initial app connection, we can run the app's main()
// method immediately.
readyToRunMainCompleter.complete();
}
_appConnectionByAppId[message.appId] = connection;
_connectedApps.add(connection);
return connection;
}

/// Waits for a resume event to trigger the app's main() method.
///
/// The [readyToRunMainCompleter]'s future will be passed to the
/// [AppConnection] so that it can be awaited on before running the app's
/// main() method.
void _waitForResumeEventToRunMain(
Stream<String> resumeEventsStream,
Completer<void> readyToRunMainCompleter,
) {
final resumeEventsSubscription = resumeEventsStream.listen((_) {
readyToRunMainCompleter.complete();
if (!readyToRunMainCompleter.isCompleted) {
readyToRunMainCompleter.complete();
}
});

safeUnawaited(
readyToRunMainCompleter.future.then((_) {
resumeEventsSubscription.cancel();
}),
);
}

void _handleIsolateExit(AppConnection appConnection) {
_servicesByAppId[appConnection.request.appId]
?.chromeProxyService
Expand Down
16 changes: 8 additions & 8 deletions dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,18 @@ class ChromeProxyService implements VmServiceInterface {
/// This value can be updated at runtime via [setFlag].
bool get pauseIsolatesOnStart => _pauseIsolatesOnStart;

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

/// A global stream of resume events.
///
/// The values in the stream are the isolates IDs for the resume event.
///
/// IMPORTANT: This should only be listened to during a hot-restart. The
/// debugger ignores any resume events as long as there is a subscriber to
/// this stream.
Stream<String> get resumeAfterHotRestartEventsStream =>
_resumeAfterHotRestartEventsController.stream;
/// IMPORTANT: This should only be listened to during a hot-restart or page
/// refresh. The debugger ignores any resume events as long as there is a
/// subscriber to this stream.
Stream<String> get resumeAfterRestartEventsStream =>
_resumeAfterRestartEventsController.stream;

final _logger = Logger('ChromeProxyService');

Expand Down Expand Up @@ -1147,8 +1147,8 @@ ${globalToolConfiguration.loadStrategy.loadModuleSnippet}("dart_sdk").developer.
}) async {
// If there is a subscriber listening for a resume event after hot-restart,
// then add the event to the stream and skip processing it.
if (_resumeAfterHotRestartEventsController.hasListener) {
_resumeAfterHotRestartEventsController.add(isolateId);
if (_resumeAfterRestartEventsController.hasListener) {
_resumeAfterRestartEventsController.add(isolateId);
return Success();
}
if (inspector.appConnection.isStarted) {
Expand Down
6 changes: 2 additions & 4 deletions dwds/test/chrome_proxy_service_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2070,9 +2070,8 @@ void main() {
service.setFlag('pause_isolates_on_start', 'true'),
completion(_isSuccess),
);
final appId = context.appConnection.request.appId;
expect(
context.dwds!.shouldPauseIsolatesOnStart(appId),
context.service.pauseIsolatesOnStart,
equals(true),
);
});
Expand All @@ -2083,9 +2082,8 @@ void main() {
service.setFlag('pause_isolates_on_start', 'false'),
completion(_isSuccess),
);
final appId = context.appConnection.request.appId;
expect(
context.dwds!.shouldPauseIsolatesOnStart(appId),
context.service.pauseIsolatesOnStart,
equals(false),
);
});
Expand Down
33 changes: 32 additions & 1 deletion dwds/test/reload_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,8 @@ void main() {
undoEdit();
});

test('does not run app until there is a resume event', () async {
test('after hot-restart, does not run app until there is a resume event',
() async {
await makeEditAndWaitForRebuild();

final eventsDone = expectLater(
Expand Down Expand Up @@ -563,6 +564,36 @@ void main() {
final sourceAfterResume = await context.webDriver.pageSource;
expect(sourceAfterResume.contains(newString), isTrue);
});

test('after page refresh, does not run app until there is a resume event',
() async {
await makeEditAndWaitForRebuild();

await context.webDriver.driver.refresh();

final eventsDone = expectLater(
client.onIsolateEvent,
emitsThrough(
emitsInOrder([
_hasKind(EventKind.kIsolateExit),
_hasKind(EventKind.kIsolateStart),
_hasKind(EventKind.kIsolateRunnable),
]),
),
);

await eventsDone;

final sourceBeforeResume = await context.webDriver.pageSource;
expect(sourceBeforeResume.contains(newString), isFalse);

final vm = await client.getVM();
final isolateId = vm.isolates!.first.id!;
await client.resume(isolateId);

final sourceAfterResume = await context.webDriver.pageSource;
expect(sourceAfterResume.contains(newString), isTrue);
});
});
}

Expand Down

0 comments on commit 99abc53

Please sign in to comment.