Detect exact device ID matches quickly (#62070)

packages/flutter_tools/lib/src/device.dart

@@ -104,23 +104,51 @@ abstract class DeviceManager {
   bool get hasSpecifiedAllDevices => _specifiedDeviceId == 'all';
 
   Future<List<Device>> getDevicesById(String deviceId) async {
-    final List<Device> devices = await getAllConnectedDevices();
-    deviceId = deviceId.toLowerCase();
+    final String lowerDeviceId = deviceId.toLowerCase();
     bool exactlyMatchesDeviceId(Device device) =>
-        device.id.toLowerCase() == deviceId ||
-        device.name.toLowerCase() == deviceId;
+        device.id.toLowerCase() == lowerDeviceId ||
+        device.name.toLowerCase() == lowerDeviceId;
     bool startsWithDeviceId(Device device) =>
-        device.id.toLowerCase().startsWith(deviceId) ||
-        device.name.toLowerCase().startsWith(deviceId);
+        device.id.toLowerCase().startsWith(lowerDeviceId) ||
+        device.name.toLowerCase().startsWith(lowerDeviceId);
 
-    final Device exactMatch = devices.firstWhere(
-        exactlyMatchesDeviceId, orElse: () => null);
-    if (exactMatch != null) {
-      return <Device>[exactMatch];
+    // Some discoverers have hard-coded device IDs and return quickly, and others
+    // shell out to other processes and can take longer.
+    // Process discoverers as they can return results, so if an exact match is
+    // found quickly, we don't wait for all the discoverers to complete.
+    final List<Device> prefixMatches = <Device>[];
+    final Completer<Device> exactMatchCompleter = Completer<Device>();
+    final List<Future<List<Device>>> futureDevices = <Future<List<Device>>>[
+      for (final DeviceDiscovery discoverer in _platformDiscoverers)
+        discoverer
+        .devices
+        .then((List<Device> devices) {
+          for (final Device device in devices) {
+            if (exactlyMatchesDeviceId(device)) {
+              exactMatchCompleter.complete(device);
+              return null;
+            }
+            if (startsWithDeviceId(device)) {
+              prefixMatches.add(device);
+            }
+          }
+          return null;
+        }, onError: (dynamic error, StackTrace stackTrace) {
+          // Return matches from other discoverers even if one fails.
+          globals.printTrace('Ignored error discovering $deviceId: $error');
+        })
+    ];
+
+    // Wait for an exact match, or for all discoverers to return results.
+    await Future.any<dynamic>(<Future<dynamic>>[
+      exactMatchCompleter.future,
+      Future.wait<List<Device>>(futureDevices),
+    ]);
+
+    if (exactMatchCompleter.isCompleted) {
+      return <Device>[await exactMatchCompleter.future];
     }
-
-    // Match on a id or name starting with [deviceId].
-    return devices.where(startsWithDeviceId).toList();
+    return prefixMatches;
   }
 
   /// Returns the list of connected devices, filtered by any user-specified device id.

packages/flutter_tools/test/general.shard/device_test.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/terminal.dart';
 import 'package:flutter_tools/src/artifacts.dart';
 import 'package:flutter_tools/src/build_info.dart';
@@ -22,9 +23,11 @@ import '../src/mocks.dart';
 
 void main() {
   MockCache cache;
+  BufferLogger logger;
 
   setUp(() {
     cache = MockCache();
+    logger = BufferLogger.test();
     when(cache.dyLdLibEntry).thenReturn(const MapEntry<String, String>('foo', 'bar'));
   });
 
@@ -41,25 +44,67 @@ void main() {
       Cache: () => cache,
     });
 
-    testUsingContext('getDeviceById', () async {
+    testUsingContext('getDeviceById exact matcher', () async {
       final FakeDevice device1 = FakeDevice('Nexus 5', '0553790d0a4e726f');
       final FakeDevice device2 = FakeDevice('Nexus 5X', '01abfc49119c410e');
       final FakeDevice device3 = FakeDevice('iPod touch', '82564b38861a9a5');
       final List<Device> devices = <Device>[device1, device2, device3];
-      final DeviceManager deviceManager = TestDeviceManager(devices);
+
+      // Include different device discoveries:
+      // 1. One that never completes to prove the first exact match is
+      // returned quickly.
+      // 2. One that throws, to prove matches can return when some succeed
+      // and others fail.
+      // 3. A device discoverer that succeeds.
+      final DeviceManager deviceManager = TestDeviceManager(
+        devices,
+        testLongPollingDeviceDiscovery: true,
+        testThrowingDeviceDiscovery: true,
+      );
 
       Future<void> expectDevice(String id, List<Device> expected) async {
         expect(await deviceManager.getDevicesById(id), expected);
       }
       await expectDevice('01abfc49119c410e', <Device>[device2]);
+      expect(logger.traceText, contains('Ignored error discovering 01abfc49119c410e'));
       await expectDevice('Nexus 5X', <Device>[device2]);
+      expect(logger.traceText, contains('Ignored error discovering Nexus 5X'));
       await expectDevice('0553790d0a4e726f', <Device>[device1]);
-      await expectDevice('Nexus 5', <Device>[device1]);
-      await expectDevice('0553790', <Device>[device1]);
-      await expectDevice('Nexus', <Device>[device1, device2]);
+      expect(logger.traceText, contains('Ignored error discovering 0553790d0a4e726f'));
     }, overrides: <Type, Generator>{
       Artifacts: () => Artifacts.test(),
       Cache: () => cache,
+      Logger: () => logger,
+    });
+
+    testUsingContext('getDeviceById prefix matcher', () async {
+      final FakeDevice device1 = FakeDevice('Nexus 5', '0553790d0a4e726f');
+      final FakeDevice device2 = FakeDevice('Nexus 5X', '01abfc49119c410e');
+      final FakeDevice device3 = FakeDevice('iPod touch', '82564b38861a9a5');
+      final List<Device> devices = <Device>[device1, device2, device3];
+
+      // Include different device discoveries:
+      // 1. One that throws, to prove matches can return when some succeed
+      // and others fail.
+      // 2. A device discoverer that succeeds.
+      final DeviceManager deviceManager = TestDeviceManager(
+        devices,
+        testThrowingDeviceDiscovery: true
+      );
+
+      Future<void> expectDevice(String id, List<Device> expected) async {
+        expect(await deviceManager.getDevicesById(id), expected);
+      }
+      await expectDevice('Nexus 5', <Device>[device1]);
+      expect(logger.traceText, contains('Ignored error discovering Nexus 5'));
+      await expectDevice('0553790', <Device>[device1]);
+      expect(logger.traceText, contains('Ignored error discovering 0553790'));
+      await expectDevice('Nexus', <Device>[device1, device2]);
+      expect(logger.traceText, contains('Ignored error discovering Nexus'));
+    }, overrides: <Type, Generator>{
+      Artifacts: () => Artifacts.test(),
+      Cache: () => cache,
+      Logger: () => logger,
     });
 
     testUsingContext('getAllConnectedDevices caches', () async {
@@ -374,16 +419,27 @@ void main() {
 }
 
 class TestDeviceManager extends DeviceManager {
-  TestDeviceManager(List<Device> allDevices) {
-    _deviceDiscoverer = FakePollingDeviceDiscovery();
+    TestDeviceManager(List<Device> allDevices, {
+    bool testLongPollingDeviceDiscovery = false,
+    bool testThrowingDeviceDiscovery = false,
+  }) {
+    _fakeDeviceDiscoverer = FakePollingDeviceDiscovery();
+    _deviceDiscoverers = <DeviceDiscovery>[
+      if (testLongPollingDeviceDiscovery)
+        LongPollingDeviceDiscovery(),
+      if (testThrowingDeviceDiscovery)
+        ThrowingPollingDeviceDiscovery(),
+      _fakeDeviceDiscoverer,
+    ];
     resetDevices(allDevices);
   }
   @override
-  List<DeviceDiscovery> get deviceDiscoverers => <DeviceDiscovery>[_deviceDiscoverer];
-  FakePollingDeviceDiscovery _deviceDiscoverer;
+  List<DeviceDiscovery> get deviceDiscoverers => _deviceDiscoverers;
+  List<DeviceDiscovery> _deviceDiscoverers;
+  FakePollingDeviceDiscovery _fakeDeviceDiscoverer;
 
   void resetDevices(List<Device> allDevices) {
-    _deviceDiscoverer.setDevices(allDevices);
+    _fakeDeviceDiscoverer.setDevices(allDevices);
   }
 
   bool isAlwaysSupportedOverride;

packages/flutter_tools/test/src/mocks.dart

@@ -526,6 +526,48 @@ class FakePollingDeviceDiscovery extends PollingDeviceDiscovery {
   Stream<Device> get onRemoved => _onRemovedController.stream;
 }
 
+class LongPollingDeviceDiscovery extends PollingDeviceDiscovery {
+  LongPollingDeviceDiscovery() : super('forever');
+
+  final Completer<List<Device>> _completer = Completer<List<Device>>();
+
+  @override
+  Future<List<Device>> pollingGetDevices({ Duration timeout }) async {
+    return _completer.future;
+  }
+
+  @override
+  Future<void> stopPolling() async {
+    _completer.complete();
+  }
+
+  @override
+  Future<void> dispose() async {
+    _completer.complete();
+  }
+
+  @override
+  bool get supportsPlatform => true;
+
+  @override
+  bool get canListAnything => true;
+}
+
+class ThrowingPollingDeviceDiscovery extends PollingDeviceDiscovery {
+  ThrowingPollingDeviceDiscovery() : super('throw');
+
+  @override
+  Future<List<Device>> pollingGetDevices({ Duration timeout }) async {
+    throw const ProcessException('fake-discovery', <String>[]);
+  }
+
+  @override
+  bool get supportsPlatform => true;
+
+  @override
+  bool get canListAnything => true;
+}
+
 class MockIosProject extends Mock implements IosProject {
   static const String bundleId = 'com.example.test';
   static const String appBundleName = 'My Super Awesome App.app';