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 93132859.

* Revert "Fix a race condition in vmservice_test.dart (#23529)"

This reverts commit 5e7b0a36.

packages/flutter_tools/lib/src/resident_runner.dart

@@ -82,10 +82,8 @@ class FlutterDevice {
   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 @@ abstract class ResidentRunner {
   }
 
   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 {

packages/flutter_tools/lib/src/run_hot.dart

@@ -156,10 +156,7 @@ class HotRunner extends ResidentRunner {
   }) 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 @@ class HotRunner extends ResidentRunner {
     }
     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 @@ class HotRunner extends ResidentRunner {
 
     return attach(
       connectionInfoCompleter: connectionInfoCompleter,
-      appStartedCompleter: appStartedCompleter,
+      appStartedCompleter: appStartedCompleter
     );
   }
 

packages/flutter_tools/lib/src/vmservice.dart

@@ -105,7 +105,7 @@ const Duration kShortRequestTimeout = Duration(seconds: 5);
 // 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 @@ class VMService {
     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 @@ class VMService {
     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 @@ class VMService {
   }
 
   /// 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 @@ String _stripRef(String type) => _hasRef(type) ? type.substring(1) : type;
 /// 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 _upgradeCollection(dynamic collection,
 }
 
 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 _upgradeMap(Map<String, dynamic> map, ServiceObjectOwner owner) {
 }
 
 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 @@ abstract class ServiceObject {
 
   /// 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 @@ abstract class ServiceObject {
     // 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 @@ class VM extends ServiceObjectOwner {
   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 @@ class VM extends ServiceObjectOwner {
     _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 @@ class VM extends ServiceObjectOwner {
     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 @@ class VM extends ServiceObjectOwner {
     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 @@ class VM extends ServiceObjectOwner {
     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 @@ class Isolate extends ServiceObjectOwner {
   }
 
   @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 @@ class FlutterView extends ServiceObject {
                              packagesUri,
                              assetsDirectoryUri);
     await completer.future;
-    await owner.vm.refreshViews(waitForViews: true);
+    await owner.vm.refreshViews();
     await subscription.cancel();
   }
 

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 @@ void main() {
         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,
-    });
   });
 }