Revert "[O] Fix a race condition in vmservice_test.dart" (#23647)
* Revert "[H] Created a variant of InheritedWidget specifically for Listenables (#23393)"
This reverts commit 931328596ac48a848953f330a192fc33425a378a.
* Revert "Fix a race condition in vmservice_test.dart (#23529)"
This reverts commit 5e7b0a366b3fe55b99fa79461a94140c59ab3f07.
diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
index 76d7552..160a9cf 100644
--- a/packages/flutter_tools/lib/src/resident_runner.dart
+++ b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -82,10 +82,8 @@
Future<void> refreshViews() async {
if (vmServices == null || vmServices.isEmpty)
return;
- final List<Future<void>> futures = <Future<void>>[];
for (VMService service in vmServices)
- futures.add(service.vm.refreshViews(waitForViews: true));
- await Future.wait(futures);
+ await service.vm.refreshViews();
}
List<FlutterView> get views {
@@ -485,10 +483,8 @@
}
Future<void> refreshViews() async {
- final List<Future<void>> futures = <Future<void>>[];
for (FlutterDevice device in flutterDevices)
- futures.add(device.refreshViews());
- await Future.wait(futures);
+ await device.refreshViews();
}
Future<void> _debugDumpApp() async {
diff --git a/packages/flutter_tools/lib/src/run_hot.dart b/packages/flutter_tools/lib/src/run_hot.dart
index 66adf1e..3ac58e9 100644
--- a/packages/flutter_tools/lib/src/run_hot.dart
+++ b/packages/flutter_tools/lib/src/run_hot.dart
@@ -156,10 +156,7 @@
}) async {
_didAttach = true;
try {
- await connectToServiceProtocol(
- reloadSources: _reloadSourcesService,
- compileExpression: _compileExpressionService,
- );
+ await connectToServiceProtocol(reloadSources: _reloadSourcesService, compileExpression: _compileExpressionService);
} catch (error) {
printError('Error connecting to the service protocol: $error');
return 2;
@@ -185,10 +182,8 @@
}
final Stopwatch initialUpdateDevFSsTimer = Stopwatch()..start();
final bool devfsResult = await _updateDevFS(fullRestart: true);
- _addBenchmarkData(
- 'hotReloadInitialDevFSSyncMilliseconds',
- initialUpdateDevFSsTimer.elapsed.inMilliseconds,
- );
+ _addBenchmarkData('hotReloadInitialDevFSSyncMilliseconds',
+ initialUpdateDevFSsTimer.elapsed.inMilliseconds);
if (!devfsResult)
return 3;
@@ -274,7 +269,7 @@
return attach(
connectionInfoCompleter: connectionInfoCompleter,
- appStartedCompleter: appStartedCompleter,
+ appStartedCompleter: appStartedCompleter
);
}
diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart
index d9c9fe6..bc1586f 100644
--- a/packages/flutter_tools/lib/src/vmservice.dart
+++ b/packages/flutter_tools/lib/src/vmservice.dart
@@ -105,7 +105,7 @@
// TODO(flutter/flutter#23031): Test this.
/// A connection to the Dart VM Service.
class VMService {
- VMService(
+ VMService._(
this._peer,
this.httpAddress,
this.wsAddress,
@@ -236,7 +236,7 @@
final Uri wsUri = httpUri.replace(scheme: 'ws', path: fs.path.join(httpUri.path, 'ws'));
final StreamChannel<String> channel = await _openChannel(wsUri);
final rpc.Peer peer = rpc.Peer.withoutJson(jsonDocument.bind(channel));
- final VMService service = VMService(peer, httpUri, wsUri, requestTimeout, reloadSources, compileExpression);
+ final VMService service = VMService._(peer, httpUri, wsUri, requestTimeout, reloadSources, compileExpression);
// This call is to ensure we are able to establish a connection instead of
// keeping on trucking and failing farther down the process.
await service._sendRequest('getVersion', const <String, dynamic>{});
@@ -311,7 +311,7 @@
final Map<String, dynamic> eventIsolate = eventData['isolate'];
// Log event information.
- printTrace('Notification from VM: $data');
+ printTrace(data.toString());
ServiceEvent event;
if (eventIsolate != null) {
@@ -341,9 +341,15 @@
}
/// Reloads the VM.
- Future<void> getVM() async => await vm.reload();
+ Future<VM> getVM() async {
+ return await _vm.reload();
+ }
- Future<void> refreshViews({ bool waitForViews = false }) => vm.refreshViews(waitForViews: waitForViews);
+ Future<void> refreshViews() async {
+ if (!vm.isFlutterEngine)
+ return;
+ await vm.refreshViews();
+ }
}
/// An error that is thrown when constructing/updating a service object.
@@ -365,8 +371,9 @@
/// to return a cached / canonicalized object.
void _upgradeCollection(dynamic collection,
ServiceObjectOwner owner) {
- if (collection is ServiceMap)
+ if (collection is ServiceMap) {
return;
+ }
if (collection is Map<String, dynamic>) {
_upgradeMap(collection, owner);
} else if (collection is List) {
@@ -375,7 +382,7 @@
}
void _upgradeMap(Map<String, dynamic> map, ServiceObjectOwner owner) {
- map.forEach((String k, Object v) {
+ map.forEach((String k, dynamic v) {
if ((v is Map<String, dynamic>) && _isServiceMap(v)) {
map[k] = owner.getFromMap(v);
} else if (v is List) {
@@ -387,8 +394,8 @@
}
void _upgradeList(List<dynamic> list, ServiceObjectOwner owner) {
- for (int i = 0; i < list.length; i += 1) {
- final Object v = list[i];
+ for (int i = 0; i < list.length; i++) {
+ final dynamic v = list[i];
if ((v is Map<String, dynamic>) && _isServiceMap(v)) {
list[i] = owner.getFromMap(v);
} else if (v is List) {
@@ -470,8 +477,9 @@
/// If this is not already loaded, load it. Otherwise reload.
Future<ServiceObject> load() async {
- if (loaded)
+ if (loaded) {
return this;
+ }
return reload();
}
@@ -491,12 +499,15 @@
// We should always reload the VM.
// We can't reload objects without an id.
// We shouldn't reload an immutable and already loaded object.
- if (!isVM && (!hasId || (immutable && loaded)))
+ final bool skipLoad = !isVM && (!hasId || (immutable && loaded));
+ if (skipLoad) {
return this;
+ }
if (_inProgressReload == null) {
final Completer<ServiceObject> completer = Completer<ServiceObject>();
_inProgressReload = completer.future;
+
try {
final Map<String, dynamic> response = await _fetchDirect();
if (_stripRef(response['type']) == 'Sentinel') {
@@ -650,7 +661,9 @@
VM get vm => this;
@override
- Future<Map<String, dynamic>> _fetchDirect() => invokeRpcRaw('getVM');
+ Future<Map<String, dynamic>> _fetchDirect() async {
+ return invokeRpcRaw('getVM');
+ }
@override
void _update(Map<String, dynamic> map, bool mapIsRef) {
@@ -662,9 +675,11 @@
_upgradeCollection(map, this);
_loaded = true;
+ // TODO(johnmccutchan): Extract any properties we care about here.
_pid = map['pid'];
- if (map['_heapAllocatedMemoryUsage'] != null)
+ if (map['_heapAllocatedMemoryUsage'] != null) {
_heapAllocatedMemoryUsage = map['_heapAllocatedMemoryUsage'];
+ }
_maxRSS = map['_maxRSS'];
_embedder = map['_embedder'];
@@ -803,13 +818,6 @@
return Future<Isolate>.value(_isolateCache[isolateId]);
}
- static String _truncate(String message, int width, String ellipsis) {
- assert(ellipsis.length < width);
- if (message.length <= width)
- return message;
- return message.substring(0, width - ellipsis.length) + ellipsis;
- }
-
/// Invoke the RPC and return the raw response.
///
/// If `timeoutFatal` is false, then a timeout will result in a null return
@@ -819,14 +827,14 @@
Duration timeout,
bool timeoutFatal = true,
}) async {
- printTrace('Sending to VM service: $method($params)');
+ printTrace('$method: $params');
+
assert(params != null);
timeout ??= _vmService._requestTimeout;
try {
final Map<String, dynamic> result = await _vmService
._sendRequest(method, params)
.timeout(timeout);
- printTrace('Result: ${_truncate(result.toString(), 250, '...')}');
return result;
} on TimeoutException {
printTrace('Request to Dart VM Service timed out: $method($params)');
@@ -941,32 +949,14 @@
return invokeRpcRaw('_getVMTimeline', timeout: kLongRequestTimeout);
}
- Future<void> refreshViews({ bool waitForViews = false }) async {
- assert(waitForViews != null);
- assert(loaded);
+ Future<void> refreshViews() async {
if (!isFlutterEngine)
return;
- int failCount = 0;
- while (true) {
- _viewCache.clear();
- final List<Future<void>> futures = <Future<void>>[];
- for (Isolate isolate in isolates.toList()) {
- // When the future returned by invokeRpc() below returns,
- // the _viewCache will have been updated.
- futures.add(vmService.vm.invokeRpc<ServiceObject>(
- '_flutter.listViews',
+ _viewCache.clear();
+ for (Isolate isolate in isolates.toList()) {
+ await vmService.vm.invokeRpc<ServiceObject>('_flutter.listViews',
timeout: kLongRequestTimeout,
- params: <String, dynamic> {'isolateId': isolate.id},
- ));
- }
- await Future.wait(futures);
- if (_viewCache.values.isNotEmpty || !waitForViews)
- return;
- failCount += 1;
- if (failCount == 5) // waited 200ms
- printStatus('Flutter is taking longer than expected to report its views. Still trying...');
- await Future<void>.delayed(const Duration(milliseconds: 50));
- await reload();
+ params: <String, dynamic> {'isolateId': isolate.id});
}
}
@@ -1093,7 +1083,9 @@
}
@override
- Future<Map<String, dynamic>> _fetchDirect() => invokeRpcRaw('getIsolate');
+ Future<Map<String, dynamic>> _fetchDirect() {
+ return invokeRpcRaw('getIsolate');
+ }
/// Invoke the RPC and return the raw response.
Future<Map<String, dynamic>> invokeRpcRaw(String method, {
@@ -1438,7 +1430,7 @@
packagesUri,
assetsDirectoryUri);
await completer.future;
- await owner.vm.refreshViews(waitForViews: true);
+ await owner.vm.refreshViews();
await subscription.cancel();
}
diff --git a/packages/flutter_tools/test/vmservice_test.dart b/packages/flutter_tools/test/vmservice_test.dart
index 8d7809e..c96e839 100644
--- a/packages/flutter_tools/test/vmservice_test.dart
+++ b/packages/flutter_tools/test/vmservice_test.dart
@@ -2,156 +2,12 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-import 'dart:async';
-
-import 'package:flutter_tools/src/base/io.dart';
-import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/vmservice.dart';
-import 'package:json_rpc_2/json_rpc_2.dart' as rpc;
-import 'package:quiver/testing/async.dart';
import 'src/common.dart';
import 'src/context.dart';
-import 'src/mocks.dart';
-
-class MockPeer implements rpc.Peer {
- @override
- Future<dynamic> get done async {
- throw 'unexpected call to done';
- }
-
- @override
- bool get isClosed {
- throw 'unexpected call to isClosed';
- }
-
- @override
- Future<dynamic> close() async {
- throw 'unexpected call to close()';
- }
-
- @override
- Future<dynamic> listen() async {
- // this does get called
- }
-
- @override
- void registerFallback(dynamic callback(rpc.Parameters parameters)) {
- throw 'unexpected call to registerFallback';
- }
-
- @override
- void registerMethod(String name, Function callback) {
- // this does get called
- }
-
- @override
- void sendNotification(String method, [ dynamic parameters ]) {
- throw 'unexpected call to sendNotification';
- }
-
- bool isolatesEnabled = false;
-
- Future<void> _latch;
- Completer<void> _currentLatchCompleter;
-
- void tripLatch() {
- final Completer<void> lastCompleter = _currentLatchCompleter;
- _currentLatchCompleter = Completer<void>();
- _latch = _currentLatchCompleter.future;
- lastCompleter?.complete();
- }
-
- int returnedFromSendRequest = 0;
-
- @override
- Future<dynamic> sendRequest(String method, [ dynamic parameters ]) async {
- await _latch;
- returnedFromSendRequest += 1;
- if (method == 'getVM') {
- return <String, dynamic>{
- 'type': 'VM',
- 'name': 'vm',
- 'architectureBits': 64,
- 'targetCPU': 'x64',
- 'hostCPU': ' Intel(R) Xeon(R) CPU E5-1650 v2 @ 3.50GHz',
- 'version': '2.1.0-dev.7.1.flutter-45f9462398 (Fri Oct 19 19:27:56 2018 +0000) on "linux_x64"',
- '_profilerMode': 'Dart',
- '_nativeZoneMemoryUsage': 0,
- 'pid': 103707,
- 'startTime': 1540426121876,
- '_embedder': 'Flutter',
- '_maxRSS': 312614912,
- '_currentRSS': 33091584,
- 'isolates': isolatesEnabled ? <dynamic>[
- <String, dynamic>{
- 'type': '@Isolate',
- 'fixedId': true,
- 'id': 'isolates/242098474',
- 'name': 'main.dart:main()',
- 'number': 242098474,
- },
- ] : <dynamic>[],
- };
- }
- if (method == 'getIsolate') {
- return <String, dynamic>{
- 'type': 'Isolate',
- 'fixedId': true,
- 'id': 'isolates/242098474',
- 'name': 'main.dart:main()',
- 'number': 242098474,
- '_originNumber': 242098474,
- 'startTime': 1540488745340,
- '_heaps': <String, dynamic>{
- 'new': <String, dynamic>{
- 'used': 0,
- 'capacity': 0,
- 'external': 0,
- 'collections': 0,
- 'time': 0.0,
- 'avgCollectionPeriodMillis': 0.0,
- },
- 'old': <String, dynamic>{
- 'used': 0,
- 'capacity': 0,
- 'external': 0,
- 'collections': 0,
- 'time': 0.0,
- 'avgCollectionPeriodMillis': 0.0,
- },
- },
- };
- }
- if (method == '_flutter.listViews') {
- return <String, dynamic>{
- 'type': 'FlutterViewList',
- 'views': <dynamic>[
- <String, dynamic>{
- 'type': 'FlutterView',
- 'id': '_flutterView/0x4a4c1f8',
- 'isolate': <String, dynamic>{
- 'type': '@Isolate',
- 'fixedId': true,
- 'id': 'isolates/242098474',
- 'name': 'main.dart:main()',
- 'number': 242098474,
- },
- },
- ],
- };
- }
- return null;
- }
-
- @override
- dynamic withBatch(dynamic callback()) {
- throw 'unexpected call to withBatch';
- }
-}
void main() {
- final MockStdio mockStdio = MockStdio();
group('VMService', () {
testUsingContext('fails connection eagerly in the connect() method', () async {
expect(
@@ -159,73 +15,5 @@
throwsToolExit(),
);
});
-
- testUsingContext('refreshViews', () {
- FakeAsync().run((FakeAsync time) {
- bool done = false;
- final MockPeer mockPeer = MockPeer();
- expect(mockPeer.returnedFromSendRequest, 0);
- final VMService vmService = VMService(mockPeer, null, null, const Duration(seconds: 1), null, null);
- vmService.getVM().then((void value) { done = true; });
- expect(done, isFalse);
- expect(mockPeer.returnedFromSendRequest, 0);
- time.flushMicrotasks();
- expect(done, isTrue);
- expect(mockPeer.returnedFromSendRequest, 1);
-
- done = false;
- mockPeer.tripLatch();
- final Future<void> ready = vmService.refreshViews(waitForViews: true);
- ready.then((void value) { done = true; });
- time.elapse(const Duration(milliseconds: 50)); // the last getVM had no isolates, so it waits 50ms, then calls getVM
- expect(mockPeer.returnedFromSendRequest, 1);
-
- mockPeer.tripLatch();
- expect(mockPeer.returnedFromSendRequest, 1);
- time.flushMicrotasks(); // here getVM still returns no isolates
- expect(mockPeer.returnedFromSendRequest, 2);
- time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
- expect(done, isFalse);
-
- mockPeer.tripLatch();
- expect(mockPeer.returnedFromSendRequest, 2);
- time.flushMicrotasks(); // here getVM still returns no isolates
- expect(mockPeer.returnedFromSendRequest, 3);
- time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
- expect(done, isFalse);
-
- mockPeer.tripLatch();
- expect(mockPeer.returnedFromSendRequest, 3);
- time.flushMicrotasks(); // here getVM still returns no isolates
- expect(mockPeer.returnedFromSendRequest, 4);
- time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
- expect(done, isFalse);
-
- mockPeer.tripLatch();
- expect(mockPeer.returnedFromSendRequest, 4);
- time.flushMicrotasks(); // here getVM still returns no isolates
- expect(mockPeer.returnedFromSendRequest, 5);
- const String message = 'Flutter is taking longer than expected to report its views. Still trying...\n';
- expect(mockStdio.writtenToStdout.join(''), message);
- expect(mockStdio.writtenToStderr.join(''), '');
- time.elapse(const Duration(milliseconds: 50)); // so refreshViews waits another 50ms
- expect(done, isFalse);
-
- mockPeer.isolatesEnabled = true;
- mockPeer.tripLatch();
- expect(mockPeer.returnedFromSendRequest, 5);
- time.flushMicrotasks(); // now it returns an isolate
- expect(mockPeer.returnedFromSendRequest, 6);
- mockPeer.tripLatch();
- time.flushMicrotasks(); // which gets fetched, as does the flutter view, resulting in the future returning
- expect(mockPeer.returnedFromSendRequest, 8);
- expect(done, isTrue);
- expect(mockStdio.writtenToStdout.join(''), message);
- expect(mockStdio.writtenToStderr.join(''), '');
- });
- }, overrides: <Type, Generator>{
- Logger: () => StdoutLogger(),
- Stdio: () => mockStdio,
- });
});
}
