Commit 1d9f0095 authored by Chris Bracken's avatar Chris Bracken Committed by GitHub

Re-enable use of instruments for iOS device lookup (#10838)

This reverts commit b2909a24.

This resubmits the following patches:

1. Use Xcode instruments to list devices (#10801)
Eliminates the dependency on idevice_id from libimobiledevice. Instead,
uses Xcode built-in functionality.

2. Make device discovery asynchronous (#10803)
Migrates DeviceDiscovery.devices and all device-specific lookup to be
asynchronous.
parent f8c4a6e0
...@@ -48,7 +48,7 @@ class AndroidDevices extends PollingDeviceDiscovery { ...@@ -48,7 +48,7 @@ class AndroidDevices extends PollingDeviceDiscovery {
bool get canListAnything => androidWorkflow.canListDevices; bool get canListAnything => androidWorkflow.canListDevices;
@override @override
List<Device> pollingGetDevices() => getAdbDevices(); Future<List<Device>> pollingGetDevices() async => getAdbDevices();
} }
class AndroidDevice extends Device { class AndroidDevice extends Device {
......
...@@ -305,7 +305,7 @@ class AppDomain extends Domain { ...@@ -305,7 +305,7 @@ class AppDomain extends Domain {
final String target = _getStringArg(args, 'target'); final String target = _getStringArg(args, 'target');
final bool enableHotReload = _getBoolArg(args, 'hot') ?? kHotReloadDefault; final bool enableHotReload = _getBoolArg(args, 'hot') ?? kHotReloadDefault;
final Device device = daemon.deviceDomain._getOrLocateDevice(deviceId); final Device device = await daemon.deviceDomain._getOrLocateDevice(deviceId);
if (device == null) if (device == null)
throw "device '$deviceId' not found"; throw "device '$deviceId' not found";
...@@ -493,7 +493,7 @@ class AppDomain extends Domain { ...@@ -493,7 +493,7 @@ class AppDomain extends Domain {
Future<List<Map<String, dynamic>>> discover(Map<String, dynamic> args) async { Future<List<Map<String, dynamic>>> discover(Map<String, dynamic> args) async {
final String deviceId = _getStringArg(args, 'deviceId', required: true); final String deviceId = _getStringArg(args, 'deviceId', required: true);
final Device device = daemon.deviceDomain._getDevice(deviceId); final Device device = await daemon.deviceDomain._getDevice(deviceId);
if (device == null) if (device == null)
throw "device '$deviceId' not found"; throw "device '$deviceId' not found";
...@@ -575,11 +575,12 @@ class DeviceDomain extends Domain { ...@@ -575,11 +575,12 @@ class DeviceDomain extends Domain {
final List<PollingDeviceDiscovery> _discoverers = <PollingDeviceDiscovery>[]; final List<PollingDeviceDiscovery> _discoverers = <PollingDeviceDiscovery>[];
Future<List<Device>> getDevices([Map<String, dynamic> args]) { Future<List<Device>> getDevices([Map<String, dynamic> args]) async {
final List<Device> devices = _discoverers.expand((PollingDeviceDiscovery discoverer) { final List<Device> devices = <Device>[];
return discoverer.devices; for (PollingDeviceDiscovery discoverer in _discoverers) {
}).toList(); devices.addAll(await discoverer.devices);
return new Future<List<Device>>.value(devices); }
return devices;
} }
/// Enable device events. /// Enable device events.
...@@ -602,7 +603,7 @@ class DeviceDomain extends Domain { ...@@ -602,7 +603,7 @@ class DeviceDomain extends Domain {
final int devicePort = _getIntArg(args, 'devicePort', required: true); final int devicePort = _getIntArg(args, 'devicePort', required: true);
int hostPort = _getIntArg(args, 'hostPort'); int hostPort = _getIntArg(args, 'hostPort');
final Device device = daemon.deviceDomain._getDevice(deviceId); final Device device = await daemon.deviceDomain._getDevice(deviceId);
if (device == null) if (device == null)
throw "device '$deviceId' not found"; throw "device '$deviceId' not found";
...@@ -617,7 +618,7 @@ class DeviceDomain extends Domain { ...@@ -617,7 +618,7 @@ class DeviceDomain extends Domain {
final int devicePort = _getIntArg(args, 'devicePort', required: true); final int devicePort = _getIntArg(args, 'devicePort', required: true);
final int hostPort = _getIntArg(args, 'hostPort', required: true); final int hostPort = _getIntArg(args, 'hostPort', required: true);
final Device device = daemon.deviceDomain._getDevice(deviceId); final Device device = await daemon.deviceDomain._getDevice(deviceId);
if (device == null) if (device == null)
throw "device '$deviceId' not found"; throw "device '$deviceId' not found";
...@@ -631,23 +632,25 @@ class DeviceDomain extends Domain { ...@@ -631,23 +632,25 @@ class DeviceDomain extends Domain {
} }
/// Return the device matching the deviceId field in the args. /// Return the device matching the deviceId field in the args.
Device _getDevice(String deviceId) { Future<Device> _getDevice(String deviceId) async {
final List<Device> devices = _discoverers.expand((PollingDeviceDiscovery discoverer) { for (PollingDeviceDiscovery discoverer in _discoverers) {
return discoverer.devices; final Device device = (await discoverer.devices).firstWhere((Device device) => device.id == deviceId, orElse: () => null);
}).toList(); if (device != null)
return devices.firstWhere((Device device) => device.id == deviceId, orElse: () => null); return device;
}
return null;
} }
/// Return a known matching device, or scan for devices if no known match is found. /// Return a known matching device, or scan for devices if no known match is found.
Device _getOrLocateDevice(String deviceId) { Future<Device> _getOrLocateDevice(String deviceId) async {
// Look for an already known device. // Look for an already known device.
final Device device = _getDevice(deviceId); final Device device = await _getDevice(deviceId);
if (device != null) if (device != null)
return device; return device;
// Scan the different device providers for a match. // Scan the different device providers for a match.
for (PollingDeviceDiscovery discoverer in _discoverers) { for (PollingDeviceDiscovery discoverer in _discoverers) {
final List<Device> devices = discoverer.pollingGetDevices(); final List<Device> devices = await discoverer.pollingGetDevices();
for (Device device in devices) for (Device device in devices)
if (device.id == deviceId) if (device.id == deviceId)
return device; return device;
......
...@@ -81,11 +81,17 @@ class DeviceManager { ...@@ -81,11 +81,17 @@ class DeviceManager {
: getAllConnectedDevices(); : getAllConnectedDevices();
} }
Iterable<DeviceDiscovery> get _platformDiscoverers {
return _deviceDiscoverers.where((DeviceDiscovery discoverer) => discoverer.supportsPlatform);
}
/// Return the list of all connected devices. /// Return the list of all connected devices.
Stream<Device> getAllConnectedDevices() { Stream<Device> getAllConnectedDevices() async* {
return new Stream<Device>.fromIterable(_deviceDiscoverers for (DeviceDiscovery discoverer in _platformDiscoverers) {
.where((DeviceDiscovery discoverer) => discoverer.supportsPlatform) for (Device device in await discoverer.devices) {
.expand((DeviceDiscovery discoverer) => discoverer.devices)); yield device;
}
}
} }
} }
...@@ -97,7 +103,7 @@ abstract class DeviceDiscovery { ...@@ -97,7 +103,7 @@ abstract class DeviceDiscovery {
/// current environment configuration. /// current environment configuration.
bool get canListAnything; bool get canListAnything;
List<Device> get devices; Future<List<Device>> get devices;
} }
/// A [DeviceDiscovery] implementation that uses polling to discover device adds /// A [DeviceDiscovery] implementation that uses polling to discover device adds
...@@ -111,13 +117,13 @@ abstract class PollingDeviceDiscovery extends DeviceDiscovery { ...@@ -111,13 +117,13 @@ abstract class PollingDeviceDiscovery extends DeviceDiscovery {
ItemListNotifier<Device> _items; ItemListNotifier<Device> _items;
Timer _timer; Timer _timer;
List<Device> pollingGetDevices(); Future<List<Device>> pollingGetDevices();
void startPolling() { void startPolling() {
if (_timer == null) { if (_timer == null) {
_items ??= new ItemListNotifier<Device>(); _items ??= new ItemListNotifier<Device>();
_timer = new Timer.periodic(_pollingDuration, (Timer timer) { _timer = new Timer.periodic(_pollingDuration, (Timer timer) async {
_items.updateWithNewList(pollingGetDevices()); _items.updateWithNewList(await pollingGetDevices());
}); });
} }
} }
...@@ -128,8 +134,8 @@ abstract class PollingDeviceDiscovery extends DeviceDiscovery { ...@@ -128,8 +134,8 @@ abstract class PollingDeviceDiscovery extends DeviceDiscovery {
} }
@override @override
List<Device> get devices { Future<List<Device>> get devices async {
_items ??= new ItemListNotifier<Device>.from(pollingGetDevices()); _items ??= new ItemListNotifier<Device>.from(await pollingGetDevices());
return _items.items; return _items.items;
} }
......
...@@ -36,11 +36,11 @@ class IOSDevices extends PollingDeviceDiscovery { ...@@ -36,11 +36,11 @@ class IOSDevices extends PollingDeviceDiscovery {
bool get canListAnything => iosWorkflow.canListDevices; bool get canListAnything => iosWorkflow.canListDevices;
@override @override
List<Device> pollingGetDevices() => IOSDevice.getAttachedDevices(); Future<List<Device>> pollingGetDevices() => IOSDevice.getAttachedDevices();
} }
class IOSDevice extends Device { class IOSDevice extends Device {
IOSDevice(String id, { this.name }) : super(id) { IOSDevice(String id, { this.name, String sdkVersion }) : _sdkVersion = sdkVersion, super(id) {
_installerPath = _checkForCommand('ideviceinstaller'); _installerPath = _checkForCommand('ideviceinstaller');
_iproxyPath = _checkForCommand('iproxy'); _iproxyPath = _checkForCommand('iproxy');
_pusherPath = _checkForCommand( _pusherPath = _checkForCommand(
...@@ -55,6 +55,8 @@ class IOSDevice extends Device { ...@@ -55,6 +55,8 @@ class IOSDevice extends Device {
String _iproxyPath; String _iproxyPath;
String _pusherPath; String _pusherPath;
final String _sdkVersion;
@override @override
bool get supportsHotMode => true; bool get supportsHotMode => true;
...@@ -71,14 +73,30 @@ class IOSDevice extends Device { ...@@ -71,14 +73,30 @@ class IOSDevice extends Device {
@override @override
bool get supportsStartPaused => false; bool get supportsStartPaused => false;
static List<IOSDevice> getAttachedDevices() { // Physical device line format to be matched:
if (!iMobileDevice.isInstalled) // My iPhone (10.3.2) [75b90e947c5f429fa67f3e9169fda0d89f0492f1]
//
// Other formats in output (desktop, simulator) to be ignored:
// my-mac-pro [2C10513E-4dA5-405C-8EF5-C44353DB3ADD]
// iPhone 6s (9.3) [F6CEE7CF-81EB-4448-81B4-1755288C7C11] (Simulator)
static final RegExp _deviceRegex = new RegExp(r'^(.*) +\((.*)\) +\[(.*)\]$');
static Future<List<IOSDevice>> getAttachedDevices() async {
if (!xcode.isInstalled)
return <IOSDevice>[]; return <IOSDevice>[];
final List<IOSDevice> devices = <IOSDevice>[]; final List<IOSDevice> devices = <IOSDevice>[];
for (String id in iMobileDevice.getAttachedDeviceIDs()) { final Iterable<String> deviceLines = (await xcode.getAvailableDevices())
final String name = iMobileDevice.getInfoForDevice(id, 'DeviceName'); .split('\n')
devices.add(new IOSDevice(id, name: name)); .map((String line) => line.trim());
for (String line in deviceLines) {
final Match match = _deviceRegex.firstMatch(line);
if (match != null) {
final String deviceName = match.group(1);
final String sdkVersion = match.group(2);
final String deviceID = match.group(3);
devices.add(new IOSDevice(deviceID, name: deviceName, sdkVersion: sdkVersion));
}
} }
return devices; return devices;
} }
...@@ -311,11 +329,7 @@ class IOSDevice extends Device { ...@@ -311,11 +329,7 @@ class IOSDevice extends Device {
Future<TargetPlatform> get targetPlatform async => TargetPlatform.ios; Future<TargetPlatform> get targetPlatform async => TargetPlatform.ios;
@override @override
Future<String> get sdkNameAndVersion async => 'iOS $_sdkVersion ($_buildVersion)'; Future<String> get sdkNameAndVersion async => 'iOS $_sdkVersion';
String get _sdkVersion => iMobileDevice.getInfoForDevice(id, 'ProductVersion');
String get _buildVersion => iMobileDevice.getInfoForDevice(id, 'BuildVersion');
@override @override
DeviceLogReader getLogReader({ApplicationPackage app}) { DeviceLogReader getLogReader({ApplicationPackage app}) {
......
...@@ -70,14 +70,6 @@ class IMobileDevice { ...@@ -70,14 +70,6 @@ class IMobileDevice {
return await exitsHappyAsync(<String>['idevicename']); return await exitsHappyAsync(<String>['idevicename']);
} }
List<String> getAttachedDeviceIDs() {
return runSync(<String>['idevice_id', '-l'])
.trim()
.split('\n')
.where((String line) => line.isNotEmpty)
.toList();
}
/// Returns the value associated with the specified `ideviceinfo` key for a device. /// Returns the value associated with the specified `ideviceinfo` key for a device.
/// ///
/// If either the specified key or device does not exist, returns the empty string. /// If either the specified key or device does not exist, returns the empty string.
...@@ -165,6 +157,13 @@ class Xcode { ...@@ -165,6 +157,13 @@ class Xcode {
return _xcodeVersionCheckValid(_xcodeMajorVersion, _xcodeMinorVersion); return _xcodeVersionCheckValid(_xcodeMajorVersion, _xcodeMinorVersion);
} }
Future<String> getAvailableDevices() async {
final RunResult result = await runAsync(<String>['/usr/bin/instruments', '-s', 'devices']);
if (result.exitCode != 0)
throw new ToolExit('Failed to invoke /usr/bin/instruments. Is Xcode installed?');
return result.stdout;
}
} }
bool _xcodeVersionCheckValid(int major, int minor) { bool _xcodeVersionCheckValid(int major, int minor) {
......
...@@ -37,7 +37,7 @@ class IOSSimulators extends PollingDeviceDiscovery { ...@@ -37,7 +37,7 @@ class IOSSimulators extends PollingDeviceDiscovery {
bool get canListAnything => iosWorkflow.canListDevices; bool get canListAnything => iosWorkflow.canListDevices;
@override @override
List<Device> pollingGetDevices() => IOSSimulatorUtils.instance.getAttachedDevices(); Future<List<Device>> pollingGetDevices() async => IOSSimulatorUtils.instance.getAttachedDevices();
} }
class IOSSimulatorUtils { class IOSSimulatorUtils {
......
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async';
import 'package:file/file.dart'; import 'package:file/file.dart';
import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/ios/devices.dart'; import 'package:flutter_tools/src/ios/devices.dart';
...@@ -14,7 +16,7 @@ import 'package:test/test.dart'; ...@@ -14,7 +16,7 @@ import 'package:test/test.dart';
import '../src/context.dart'; import '../src/context.dart';
class MockProcessManager extends Mock implements ProcessManager {} class MockProcessManager extends Mock implements ProcessManager {}
class MockIMobileDevice extends Mock implements IMobileDevice {} class MockXcode extends Mock implements Xcode {}
class MockFile extends Mock implements File {} class MockFile extends Mock implements File {}
void main() { void main() {
...@@ -22,46 +24,48 @@ void main() { ...@@ -22,46 +24,48 @@ void main() {
osx.operatingSystem = 'macos'; osx.operatingSystem = 'macos';
group('getAttachedDevices', () { group('getAttachedDevices', () {
MockIMobileDevice mockIMobileDevice; MockXcode mockXcode;
setUp(() { setUp(() {
mockIMobileDevice = new MockIMobileDevice(); mockXcode = new MockXcode();
}); });
testUsingContext('return no devices if libimobiledevice is not installed', () async { testUsingContext('return no devices if Xcode is not installed', () async {
when(mockIMobileDevice.isInstalled).thenReturn(false); when(mockXcode.isInstalled).thenReturn(false);
expect(IOSDevice.getAttachedDevices(), isEmpty); expect(await IOSDevice.getAttachedDevices(), isEmpty);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
IMobileDevice: () => mockIMobileDevice, Xcode: () => mockXcode,
}); });
testUsingContext('returns no devices if none are attached', () async { testUsingContext('returns no devices if none are attached', () async {
when(mockIMobileDevice.isInstalled).thenReturn(true); when(mockXcode.isInstalled).thenReturn(true);
when(mockIMobileDevice.getAttachedDeviceIDs()).thenReturn(<String>[]); when(mockXcode.getAvailableDevices()).thenReturn(new Future<String>.value(''));
final List<IOSDevice> devices = IOSDevice.getAttachedDevices(); final List<IOSDevice> devices = await IOSDevice.getAttachedDevices();
expect(devices, isEmpty); expect(devices, isEmpty);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
IMobileDevice: () => mockIMobileDevice, Xcode: () => mockXcode,
}); });
testUsingContext('returns attached devices', () async { testUsingContext('returns attached devices', () async {
when(mockIMobileDevice.isInstalled).thenReturn(true); when(mockXcode.isInstalled).thenReturn(true);
when(mockIMobileDevice.getAttachedDeviceIDs()).thenReturn(<String>[ when(mockXcode.getAvailableDevices()).thenReturn(new Future<String>.value('''
'98206e7a4afd4aedaff06e687594e089dede3c44', Known Devices:
'f577a7903cc54959be2e34bc4f7f80b7009efcf4', je-mappelle-horse [ED6552C4-B774-5A4E-8B5A-606710C87C77]
]); La tele me regarde (10.3.2) [98206e7a4afd4aedaff06e687594e089dede3c44]
when(mockIMobileDevice.getInfoForDevice('98206e7a4afd4aedaff06e687594e089dede3c44', 'DeviceName')) Puits sans fond (10.3.2) [f577a7903cc54959be2e34bc4f7f80b7009efcf4]
.thenReturn('La tele me regarde'); iPhone 6 Plus (9.3) [FBA880E6-4020-49A5-8083-DCD50CA5FA09] (Simulator)
when(mockIMobileDevice.getInfoForDevice('f577a7903cc54959be2e34bc4f7f80b7009efcf4', 'DeviceName')) iPhone 6s (11.0) [E805F496-FC6A-4EA4-92FF-B7901FF4E7CC] (Simulator)
.thenReturn('Puits sans fond'); iPhone 7 (11.0) + Apple Watch Series 2 - 38mm (4.0) [60027FDD-4A7A-42BF-978F-C2209D27AD61] (Simulator)
final List<IOSDevice> devices = IOSDevice.getAttachedDevices(); iPhone SE (11.0) [667E8DCD-5DCD-4C80-93A9-60D1D995206F] (Simulator)
'''));
final List<IOSDevice> devices = await IOSDevice.getAttachedDevices();
expect(devices, hasLength(2)); expect(devices, hasLength(2));
expect(devices[0].id, '98206e7a4afd4aedaff06e687594e089dede3c44'); expect(devices[0].id, '98206e7a4afd4aedaff06e687594e089dede3c44');
expect(devices[0].name, 'La tele me regarde'); expect(devices[0].name, 'La tele me regarde');
expect(devices[1].id, 'f577a7903cc54959be2e34bc4f7f80b7009efcf4'); expect(devices[1].id, 'f577a7903cc54959be2e34bc4f7f80b7009efcf4');
expect(devices[1].name, 'Puits sans fond'); expect(devices[1].name, 'Puits sans fond');
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
IMobileDevice: () => mockIMobileDevice, Xcode: () => mockXcode,
}); });
}); });
......
...@@ -37,7 +37,7 @@ class MockPollingDeviceDiscovery extends PollingDeviceDiscovery { ...@@ -37,7 +37,7 @@ class MockPollingDeviceDiscovery extends PollingDeviceDiscovery {
MockPollingDeviceDiscovery() : super('mock'); MockPollingDeviceDiscovery() : super('mock');
@override @override
List<Device> pollingGetDevices() => _devices; Future<List<Device>> pollingGetDevices() async => _devices;
@override @override
bool get supportsPlatform => true; bool get supportsPlatform => true;
...@@ -52,7 +52,7 @@ class MockPollingDeviceDiscovery extends PollingDeviceDiscovery { ...@@ -52,7 +52,7 @@ class MockPollingDeviceDiscovery extends PollingDeviceDiscovery {
} }
@override @override
List<Device> get devices => _devices; Future<List<Device>> get devices async => _devices;
@override @override
Stream<Device> get onAdded => _onAddedController.stream; Stream<Device> get onAdded => _onAddedController.stream;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment