Skip to content

Commit 51bb11f

Browse files
authored
Revert "Reland "Make sure all isolates start during flutter driver tests" (#64432)" (#65635)
This reverts commit ccd4f6d.
1 parent 3dfbdac commit 51bb11f

File tree

2 files changed

+60
-111
lines changed

2 files changed

+60
-111
lines changed

packages/flutter_driver/lib/src/driver/vmservice_driver.dart

+57-87
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,8 @@ class VMServiceFlutterDriver extends FlutterDriver {
3737
bool logCommunicationToFile = true,
3838
}) : _printCommunication = printCommunication,
3939
_logCommunicationToFile = logCommunicationToFile,
40-
_isolateSubscription = _initIsolateHandler(_serviceClient),
4140
_driverId = _nextDriverId++;
4241

43-
// The Dart VM may be running with --pause-isolates-on-start.
44-
// Set a listener to unpause new isolates as they are ready to run,
45-
// otherwise they'll hang indefinitely.
46-
static StreamSubscription<VMIsolateRef> _initIsolateHandler(
47-
VMServiceClient serviceClient) {
48-
return serviceClient.onIsolateRunnable.listen(
49-
(VMIsolateRef isolateRef) async {
50-
await _resumeLeniently(isolateRef);
51-
}
52-
);
53-
}
54-
5542
/// Connects to a Flutter application.
5643
///
5744
/// See [FlutterDriver.connect] for more documentation.
@@ -113,12 +100,9 @@ class VMServiceFlutterDriver extends FlutterDriver {
113100
null ? vm.isolates.first :
114101
vm.isolates.firstWhere(
115102
(VMIsolateRef isolate) => isolate.number == isolateNumber);
116-
117103
_log('Isolate found with number: ${isolateRef.number}');
118-
// There is a race condition here, the isolate may have been destroyed before
119-
// we get around to loading it.
104+
120105
VMIsolate isolate = await isolateRef.loadRunnable();
121-
_log('Loaded isolate: ${isolate.number}');
122106

123107
// TODO(yjbanov): vm_service_client does not support "None" pause event yet.
124108
// It is currently reported as null, but we cannot rely on it because
@@ -133,22 +117,9 @@ class VMServiceFlutterDriver extends FlutterDriver {
133117
isolate.pauseEvent is! VMPauseExceptionEvent &&
134118
isolate.pauseEvent is! VMPauseInterruptedEvent &&
135119
isolate.pauseEvent is! VMResumeEvent) {
136-
_log('Reloading first isolate.');
137120
isolate = await isolateRef.loadRunnable();
138-
_log('Reloaded first isolate.');
139121
}
140122

141-
// Tells the Dart VM Service to notify us about "Isolate" events.
142-
//
143-
// This is a workaround for an issue in package:vm_service_client, which
144-
// subscribes to the "Isolate" stream lazily upon subscription, which
145-
// results in lost events.
146-
//
147-
// Details: https://github.com/dart-lang/vm_service_client/issues/17
148-
await connection.peer.sendRequest('streamListen', <String, String>{
149-
'streamId': 'Isolate',
150-
});
151-
152123
final VMServiceFlutterDriver driver = VMServiceFlutterDriver.connectedTo(
153124
client, connection.peer, isolate,
154125
printCommunication: printCommunication,
@@ -157,6 +128,27 @@ class VMServiceFlutterDriver extends FlutterDriver {
157128

158129
driver._dartVmReconnectUrl = dartVmServiceUrl;
159130

131+
// Attempts to resume the isolate, but does not crash if it fails because
132+
// the isolate is already resumed. There could be a race with other tools,
133+
// such as a debugger, any of which could have resumed the isolate.
134+
Future<dynamic> resumeLeniently() {
135+
_log('Attempting to resume isolate');
136+
return isolate.resume().catchError((dynamic e) {
137+
const int vmMustBePausedCode = 101;
138+
if (e is rpc.RpcException && e.code == vmMustBePausedCode) {
139+
// No biggie; something else must have resumed the isolate
140+
_log(
141+
'Attempted to resume an already resumed isolate. This may happen '
142+
'when we lose a race with another tool (usually a debugger) that '
143+
'is connected to the same isolate.'
144+
);
145+
} else {
146+
// Failed to resume due to another reason. Fail hard.
147+
throw e;
148+
}
149+
});
150+
}
151+
160152
/// Waits for a signal from the VM service that the extension is registered.
161153
///
162154
/// Looks at the list of loaded extensions for the current [isolateRef], as
@@ -191,8 +183,42 @@ class VMServiceFlutterDriver extends FlutterDriver {
191183
]);
192184
}
193185

186+
/// Tells the Dart VM Service to notify us about "Isolate" events.
187+
///
188+
/// This is a workaround for an issue in package:vm_service_client, which
189+
/// subscribes to the "Isolate" stream lazily upon subscription, which
190+
/// results in lost events.
191+
///
192+
/// Details: https://github.com/dart-lang/vm_service_client/issues/17
193+
Future<void> enableIsolateStreams() async {
194+
await connection.peer.sendRequest('streamListen', <String, String>{
195+
'streamId': 'Isolate',
196+
});
197+
}
198+
194199
// Attempt to resume isolate if it was paused
195-
await _resumeLeniently(isolate);
200+
if (isolate.pauseEvent is VMPauseStartEvent) {
201+
_log('Isolate is paused at start.');
202+
203+
await resumeLeniently();
204+
} else if (isolate.pauseEvent is VMPauseExitEvent ||
205+
isolate.pauseEvent is VMPauseBreakpointEvent ||
206+
isolate.pauseEvent is VMPauseExceptionEvent ||
207+
isolate.pauseEvent is VMPauseInterruptedEvent) {
208+
// If the isolate is paused for any other reason, assume the extension is
209+
// already there.
210+
_log('Isolate is paused mid-flight.');
211+
await resumeLeniently();
212+
} else if (isolate.pauseEvent is VMResumeEvent) {
213+
_log('Isolate is not paused. Assuming application is ready.');
214+
} else {
215+
_log(
216+
'Unknown pause event type ${isolate.pauseEvent.runtimeType}. '
217+
'Assuming application is ready.'
218+
);
219+
}
220+
221+
await enableIsolateStreams();
196222

197223
// We will never receive the extension event if the user does not register
198224
// it. If that happens, show a message but continue waiting.
@@ -215,58 +241,6 @@ class VMServiceFlutterDriver extends FlutterDriver {
215241
return driver;
216242
}
217243

218-
static bool _alreadyRunning(VMIsolate isolate) {
219-
// Expected pause events.
220-
if (isolate.pauseEvent is VMPauseStartEvent ||
221-
isolate.pauseEvent is VMPauseExitEvent ||
222-
isolate.pauseEvent is VMPauseBreakpointEvent ||
223-
isolate.pauseEvent is VMPauseExceptionEvent ||
224-
isolate.pauseEvent is VMPauseInterruptedEvent ||
225-
isolate.pauseEvent is VMNoneEvent) {
226-
return false;
227-
}
228-
// Already running.
229-
if (isolate.pauseEvent is VMResumeEvent) {
230-
_log('Isolate is not paused. Assuming application is ready.');
231-
return true;
232-
}
233-
_log('Unknown pause event type ${isolate.pauseEvent.runtimeType}. '
234-
'Assuming application is ready.');
235-
return true;
236-
}
237-
238-
/// Attempts to resume the isolate, but does not crash if it fails because
239-
/// the isolate is already resumed. There could be a race with other tools,
240-
/// such as a debugger, any of which could have resumed the isolate.
241-
/// Returns the isolate if it is runnable, null otherwise.
242-
static Future<void> _resumeLeniently(VMIsolateRef isolateRef) async {
243-
try {
244-
_log('New runnable isolate ${isolateRef.number}');
245-
final VMIsolate isolate = await isolateRef.load();
246-
if (_alreadyRunning(isolate)) {
247-
return;
248-
}
249-
_log('Attempting to resume isolate ${isolate.number}');
250-
await isolate.resume();
251-
_log('Resumed isolate ${isolate.number}');
252-
} on VMSentinelException {
253-
// Probably OK, the isolate vanished before we got to it.
254-
_log('Failed to load isolate, proceeding.');
255-
} on rpc.RpcException catch (e) {
256-
const int vmMustBePausedCode = 101;
257-
if (e.code == vmMustBePausedCode) {
258-
// No biggie; something else must have resumed the isolate
259-
_log('Attempted to resume an already resumed isolate. This may happen '
260-
'when we lose a race with another tool (usually a debugger) that '
261-
'is connected to the same isolate.'
262-
);
263-
} else {
264-
// Failed to resume due to another reason. Fail hard.
265-
rethrow;
266-
}
267-
}
268-
}
269-
270244
static int _nextDriverId = 0;
271245

272246
static const String _flutterExtensionMethodName = 'ext.flutter.driver';
@@ -301,9 +275,6 @@ class VMServiceFlutterDriver extends FlutterDriver {
301275
/// would like to instrument.
302276
final VMServiceClient _serviceClient;
303277

304-
/// Subscription to isolate events happening in the app.
305-
final StreamSubscription<VMIsolateRef> _isolateSubscription;
306-
307278
/// JSON-RPC client useful for sending raw JSON requests.
308279
rpc.Peer _peer;
309280

@@ -581,7 +552,6 @@ class VMServiceFlutterDriver extends FlutterDriver {
581552
@override
582553
Future<void> close() async {
583554
// Don't leak vm_service_client-specific objects, if any
584-
await _isolateSubscription.cancel();
585555
await _serviceClient.close();
586556
await _peer.close();
587557
}

packages/flutter_driver/test/flutter_driver_test.dart

+3-24
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ void main() {
3535
MockVM mockVM;
3636
MockIsolate mockIsolate;
3737
MockPeer mockPeer;
38-
MockIsolate otherIsolate;
3938

4039
void expectLogContains(String message) {
4140
expect(log, anyElement(contains(message)));
@@ -46,15 +45,10 @@ void main() {
4645
mockClient = MockVMServiceClient();
4746
mockVM = MockVM();
4847
mockIsolate = MockIsolate();
49-
otherIsolate = MockIsolate();
5048
mockPeer = MockPeer();
5149
when(mockClient.getVM()).thenAnswer((_) => Future<MockVM>.value(mockVM));
52-
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
53-
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[otherIsolate]);
54-
});
5550
when(mockVM.isolates).thenReturn(<VMRunnableIsolate>[mockIsolate]);
5651
when(mockIsolate.loadRunnable()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate));
57-
when(mockIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(mockIsolate));
5852
when(mockIsolate.extensionRpcs).thenReturn(<String>[]);
5953
when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) {
6054
return Stream<String>.fromIterable(<String>['ext.flutter.driver']);
@@ -66,10 +60,6 @@ void main() {
6660
VMServiceClientConnection(mockClient, mockPeer)
6761
);
6862
};
69-
when(otherIsolate.load()).thenAnswer((_) => Future<MockIsolate>.value(otherIsolate));
70-
when(otherIsolate.resume()).thenAnswer((Invocation invocation) {
71-
return Future<dynamic>.value(null);
72-
});
7363
});
7464

7565
tearDown(() async {
@@ -87,20 +77,15 @@ void main() {
8777
connectionLog.add('resume');
8878
return Future<dynamic>.value(null);
8979
});
90-
when(otherIsolate.pauseEvent).thenReturn(MockVMPauseStartEvent());
9180
when(mockIsolate.onExtensionAdded).thenAnswer((Invocation invocation) {
9281
connectionLog.add('onExtensionAdded');
9382
return Stream<String>.fromIterable(<String>['ext.flutter.driver']);
9483
});
95-
when(otherIsolate.resume()).thenAnswer((Invocation invocation) {
96-
connectionLog.add('other-resume');
97-
return Future<dynamic>.value(null);
98-
});
9984

10085
final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: '');
10186
expect(driver, isNotNull);
102-
expectLogContains('Attempting to resume isolate');
103-
expect(connectionLog, <String>['streamListen', 'resume', 'other-resume', 'onExtensionAdded']);
87+
expectLogContains('Isolate is paused at start');
88+
expect(connectionLog, <String>['resume', 'streamListen', 'onExtensionAdded']);
10489
});
10590

10691
test('connects to isolate paused mid-flight', () async {
@@ -109,7 +94,7 @@ void main() {
10994

11095
final FlutterDriver driver = await FlutterDriver.connect(dartVmServiceUrl: '');
11196
expect(driver, isNotNull);
112-
expectLogContains('Attempting to resume isolate');
97+
expectLogContains('Isolate is paused mid-flight');
11398
});
11499

115100
// This test simulates a situation when we believe that the isolate is
@@ -175,9 +160,6 @@ void main() {
175160
mockClient = MockVMServiceClient();
176161
mockPeer = MockPeer();
177162
mockIsolate = MockIsolate();
178-
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
179-
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]);
180-
});
181163
driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate);
182164
});
183165

@@ -808,9 +790,6 @@ void main() {
808790
mockClient = MockVMServiceClient();
809791
mockPeer = MockPeer();
810792
mockIsolate = MockIsolate();
811-
when(mockClient.onIsolateRunnable).thenAnswer((Invocation invocation) {
812-
return Stream<VMIsolateRef>.fromIterable(<VMIsolateRef>[]);
813-
});
814793
driver = VMServiceFlutterDriver.connectedTo(mockClient, mockPeer, mockIsolate);
815794
});
816795

0 commit comments

Comments
 (0)