From c0af77bf878a78057bc9b51a5b601ecaf746eb5c Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 13 Nov 2019 13:01:41 -0800 Subject: [PATCH] Allow specifying device-vmservice-port and host-vmservice-port (#44027) --- .../lib/src/android/android_device.dart | 3 +- .../lib/src/commands/attach.dart | 8 ++- .../flutter_tools/lib/src/commands/drive.dart | 2 +- .../flutter_tools/lib/src/commands/run.dart | 3 +- .../flutter_tools/lib/src/desktop_device.dart | 6 ++- packages/flutter_tools/lib/src/device.dart | 11 ++-- .../flutter_tools/lib/src/ios/devices.dart | 7 +-- .../flutter_tools/lib/src/ios/simulators.dart | 8 ++- .../flutter_tools/lib/src/mdns_discovery.dart | 29 ++++++++--- .../lib/src/protocol_discovery.dart | 27 +++++++--- .../lib/src/runner/flutter_command.dart | 51 ++++++++++++++++--- .../lib/src/tester/flutter_tester.dart | 6 ++- .../commands.shard/hermetic/attach_test.dart | 4 +- .../test/general.shard/ios/devices_test.dart | 8 +-- .../protocol_discovery_test.dart | 35 ++++++++++++- 15 files changed, 161 insertions(+), 47 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 1e6b0f35f3..5aea8b3549 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -549,7 +549,8 @@ class AndroidDevice extends Device { observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(), portForwarder: portForwarder, - hostPort: debuggingOptions.observatoryPort, + hostPort: debuggingOptions.hostVmServicePort, + devicePort: debuggingOptions.deviceVmServicePort, ipv6: ipv6, ); } diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index b9f35b57f1..ee6a0e06c6 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -232,7 +232,8 @@ class AttachCommand extends FlutterCommand { observatoryUri = await MDnsObservatoryDiscovery.instance.getObservatoryUri( appId, device, - usesIpv6, + usesIpv6: usesIpv6, + deviceVmservicePort: deviceVmservicePort, ); } // If MDNS discovery fails or we're not on iOS, fallback to ProtocolDiscovery. @@ -242,6 +243,9 @@ class AttachCommand extends FlutterCommand { observatoryDiscovery = ProtocolDiscovery.observatory( device.getLogReader(), portForwarder: device.portForwarder, + ipv6: ipv6, + devicePort: deviceVmservicePort, + hostPort: hostVmservicePort, ); printStatus('Waiting for a connection from Flutter on ${device.name}...'); observatoryUri = await observatoryDiscovery.uri; @@ -259,7 +263,7 @@ class AttachCommand extends FlutterCommand { device, debugUri?.host ?? hostname, devicePort ?? debugUri.port, - observatoryPort, + hostVmservicePort, debugUri?.path, ); } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 4d89864f61..680a81709c 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -273,7 +273,7 @@ Future _startApp(DriveCommand command) async { debuggingOptions: DebuggingOptions.enabled( command.getBuildInfo(), startPaused: true, - observatoryPort: command.observatoryPort, + hostVmServicePort: command.hostVmservicePort, verboseSystemLogs: command.verboseSystemLogs, cacheSkSL: command.cacheSkSL, dumpSkpOnShaderCompilation: command.dumpSkpOnShaderCompilation, diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 6f103579b4..e87b586937 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -315,7 +315,8 @@ class RunCommand extends RunCommandBase { traceSystrace: argResults['trace-systrace'], dumpSkpOnShaderCompilation: dumpSkpOnShaderCompilation, cacheSkSL: cacheSkSL, - observatoryPort: observatoryPort, + deviceVmServicePort: deviceVmservicePort, + hostVmServicePort: hostVmservicePort, verboseSystemLogs: argResults['verbose-system-logs'], initializePlatform: argResults['web-initialize-platform'], hostname: featureFlags.isWebEnabled ? argResults['web-hostname'] : '', diff --git a/packages/flutter_tools/lib/src/desktop_device.dart b/packages/flutter_tools/lib/src/desktop_device.dart index 341aa1d01c..f5779892fd 100644 --- a/packages/flutter_tools/lib/src/desktop_device.dart +++ b/packages/flutter_tools/lib/src/desktop_device.dart @@ -109,7 +109,11 @@ abstract class DesktopDevice extends Device { return LaunchResult.succeeded(); } _deviceLogReader.initializeProcess(process); - final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader); + final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader, + devicePort: debuggingOptions?.deviceVmServicePort, + hostPort: debuggingOptions?.hostVmServicePort, + ipv6: ipv6, + ); try { final Uri observatoryUri = await observatoryDiscovery.uri; onAttached(package, buildMode, process); diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index adb04a36ad..6fe8a060b0 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -502,7 +502,8 @@ class DebuggingOptions { this.cacheSkSL = false, this.useTestFonts = false, this.verboseSystemLogs = false, - this.observatoryPort, + this.hostVmServicePort, + this.deviceVmServicePort, this.initializePlatform = true, this.hostname, this.port, @@ -522,7 +523,8 @@ class DebuggingOptions { traceSystrace = false, dumpSkpOnShaderCompilation = false, verboseSystemLogs = false, - observatoryPort = null, + hostVmServicePort = null, + deviceVmServicePort = null, browserLaunch = true, vmserviceOutFile = null; @@ -542,14 +544,15 @@ class DebuggingOptions { final bool verboseSystemLogs; /// Whether to invoke webOnlyInitializePlatform in Flutter for web. final bool initializePlatform; - final int observatoryPort; + final int hostVmServicePort; + final int deviceVmServicePort; final String port; final String hostname; final bool browserLaunch; /// A file where the vmservice uri should be written after the application is started. final String vmserviceOutFile; - bool get hasObservatoryPort => observatoryPort != null; + bool get hasObservatoryPort => hostVmServicePort != null; } class LaunchResult { diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index 3fff840f88..bb481a581a 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -356,7 +356,8 @@ class IOSDevice extends Device { observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(app: package), portForwarder: portForwarder, - hostPort: debuggingOptions.observatoryPort, + hostPort: debuggingOptions.hostVmServicePort, + devicePort: debuggingOptions.deviceVmServicePort, ipv6: ipv6, ); } @@ -383,8 +384,8 @@ class IOSDevice extends Device { localUri = await MDnsObservatoryDiscovery.instance.getObservatoryUri( packageId, this, - ipv6, - debuggingOptions.observatoryPort, + usesIpv6: ipv6, + hostVmservicePort: debuggingOptions.hostVmServicePort, ); if (localUri != null) { UsageEvent('ios-mdns', 'success').send(); diff --git a/packages/flutter_tools/lib/src/ios/simulators.dart b/packages/flutter_tools/lib/src/ios/simulators.dart index 5084d7ef8d..05847fe5de 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -374,14 +374,18 @@ class IOSSimulator extends Device { if (debuggingOptions.disableServiceAuthCodes) '--disable-service-auth-codes', if (debuggingOptions.skiaDeterministicRendering) '--skia-deterministic-rendering', if (debuggingOptions.useTestFonts) '--use-test-fonts', - '--observatory-port=${debuggingOptions.observatoryPort ?? 0}', + '--observatory-port=${debuggingOptions.hostVmServicePort ?? 0}', ], ]; ProtocolDiscovery observatoryDiscovery; if (debuggingOptions.debuggingEnabled) { observatoryDiscovery = ProtocolDiscovery.observatory( - getLogReader(app: package), ipv6: ipv6); + getLogReader(app: package), + ipv6: ipv6, + hostPort: debuggingOptions.hostVmServicePort, + devicePort: debuggingOptions.deviceVmServicePort, + ); } // Launch the updated application in the simulator. diff --git a/packages/flutter_tools/lib/src/mdns_discovery.dart b/packages/flutter_tools/lib/src/mdns_discovery.dart index 6fc3bd4ec5..d824fb3332 100644 --- a/packages/flutter_tools/lib/src/mdns_discovery.dart +++ b/packages/flutter_tools/lib/src/mdns_discovery.dart @@ -50,7 +50,8 @@ class MDnsObservatoryDiscovery { /// If it is null and there is only one available instance of Observatory, /// it will return that instance's information regardless of what application /// the Observatory instance is for. - Future query({String applicationId}) async { + // TODO(jonahwilliams): use `deviceVmservicePort` to filter mdns results. + Future query({String applicationId, int deviceVmservicePort}) async { printTrace('Checking for advertised Dart observatories...'); try { await client.start(); @@ -136,14 +137,27 @@ class MDnsObservatoryDiscovery { } } - Future getObservatoryUri(String applicationId, Device device, [bool usesIpv6 = false, int observatoryPort]) async { - final MDnsObservatoryDiscoveryResult result = await query(applicationId: applicationId); + Future getObservatoryUri(String applicationId, Device device, { + bool usesIpv6 = false, + int hostVmservicePort, + int deviceVmservicePort, + }) async { + final MDnsObservatoryDiscoveryResult result = await query( + applicationId: applicationId, + deviceVmservicePort: deviceVmservicePort, + ); Uri observatoryUri; if (result != null) { final String host = usesIpv6 ? InternetAddress.loopbackIPv6.address : InternetAddress.loopbackIPv4.address; - observatoryUri = await buildObservatoryUri(device, host, result.port, observatoryPort, result.authCode); + observatoryUri = await buildObservatoryUri( + device, + host, + result.port, + hostVmservicePort, + result.authCode, + ); } return observatoryUri; } @@ -159,7 +173,7 @@ Future buildObservatoryUri( Device device, String host, int devicePort, [ - int observatoryPort, + int hostVmservicePort, String authCode, ]) async { String path = '/'; @@ -171,7 +185,6 @@ Future buildObservatoryUri( if (!path.endsWith('/')) { path += '/'; } - final int localPort = observatoryPort - ?? await device.portForwarder.forward(devicePort); - return Uri(scheme: 'http', host: host, port: localPort, path: path); + final int actualHostPort = await device.portForwarder.forward(devicePort, hostPort: hostVmservicePort); + return Uri(scheme: 'http', host: host, port: actualHostPort, path: path); } diff --git a/packages/flutter_tools/lib/src/protocol_discovery.dart b/packages/flutter_tools/lib/src/protocol_discovery.dart index 33768ede47..803c957eee 100644 --- a/packages/flutter_tools/lib/src/protocol_discovery.dart +++ b/packages/flutter_tools/lib/src/protocol_discovery.dart @@ -4,6 +4,8 @@ import 'dart:async'; +import 'package:meta/meta.dart'; + import 'base/io.dart'; import 'device.dart'; import 'globals.dart'; @@ -16,6 +18,7 @@ class ProtocolDiscovery { this.serviceName, { this.portForwarder, this.hostPort, + this.devicePort, this.ipv6, }) : assert(logReader != null) { _deviceLogSubscription = logReader.logLines.listen(_handleLine); @@ -24,8 +27,9 @@ class ProtocolDiscovery { factory ProtocolDiscovery.observatory( DeviceLogReader logReader, { DevicePortForwarder portForwarder, - int hostPort, - bool ipv6 = false, + @required int hostPort, + @required int devicePort, + @required bool ipv6, }) { const String kObservatoryService = 'Observatory'; return ProtocolDiscovery._( @@ -33,6 +37,7 @@ class ProtocolDiscovery { kObservatoryService, portForwarder: portForwarder, hostPort: hostPort, + devicePort: devicePort, ipv6: ipv6, ); } @@ -41,6 +46,7 @@ class ProtocolDiscovery { final String serviceName; final DevicePortForwarder portForwarder; final int hostPort; + final int devicePort; final bool ipv6; final Completer _completer = Completer(); @@ -71,17 +77,22 @@ class ProtocolDiscovery { if (match != null) { try { uri = Uri.parse(match[1]); - } catch (error, stackTrace) { + } on FormatException catch (error, stackTrace) { _stopScrapingLogs(); _completer.completeError(error, stackTrace); } } - - if (uri != null) { - assert(!_completer.isCompleted); - _stopScrapingLogs(); - _completer.complete(uri); + if (uri == null) { + return; } + if (devicePort != null && uri.port != devicePort) { + printTrace('skipping potential observatory $uri due to device port mismatch'); + return; + } + + assert(!_completer.isCompleted); + _stopScrapingLogs(); + _completer.complete(uri); } Future _forwardPort(Uri deviceUri) async { diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index 81ec0702af..4f4c3e2496 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -216,23 +216,60 @@ abstract class FlutterCommand extends Command { /// Adds options for connecting to the Dart VM observatory port. void usesPortOptions() { argParser.addOption(observatoryPortOption, - help: 'Listen to the given port for an observatory debugger connection.\n' + help: '(deprecated use host-vmservice-port instead)' + 'Listen to the given port for an observatory debugger connection.\n' 'Specifying port 0 (the default) will find a random free port.', ); + argParser.addOption('device-vmservice-port', + help: 'Look for vmservice connections only from the specified port.\n' + 'Specifying port 0 (the default) will accept the first vmservice ' + 'discovered.', + ); + argParser.addOption('host-vmservice-port', + help: 'When a device-side vmservice port is forwarded to a host-side ' + 'port, use this value as the host port.\nSpecifying port 0 ' + '(the default) will find a random free host port.' + ); _usesPortOption = true; } - /// Gets the observatory port provided to in the 'observatory-port' option. + /// Gets the vmservice port provided to in the 'observatory-port' or + /// 'host-vmservice-port option. + /// + /// Only one of "host-vmservice-port" and "observatory-port" may be + /// specified. /// /// If no port is set, returns null. - int get observatoryPort { - if (!_usesPortOption || argResults['observatory-port'] == null) { + int get hostVmservicePort { + if (!_usesPortOption || + argResults['observatory-port'] == null || + argResults['host-vmservice-port'] == null) { + return null; + } + if (argResults.wasParsed('observatory-port') && + argResults.wasParsed('host-vmservice-port')) { + throwToolExit('Only one of "--observatory-port" and ' + '"--host-vmservice-port" may be specified.'); + } + try { + return int.parse(argResults['observatory-port'] ?? argResults['host-vmservice-port']); + } on FormatException catch (error) { + throwToolExit('Invalid port for `--observatory-port/--host-vmservice-port`: $error'); + } + return null; + } + + /// Gets the vmservice port provided to in the 'device-vmservice-port' option. + /// + /// If no port is set, returns null. + int get deviceVmservicePort { + if (!_usesPortOption || argResults['device-vmservice-port'] == null) { return null; } try { - return int.parse(argResults['observatory-port']); - } catch (error) { - throwToolExit('Invalid port for `--observatory-port`: $error'); + return int.parse(argResults['device-vmservice-port']); + } on FormatException catch (error) { + throwToolExit('Invalid port for `--device-vmservice-port`: $error'); } return null; } diff --git a/packages/flutter_tools/lib/src/tester/flutter_tester.dart b/packages/flutter_tools/lib/src/tester/flutter_tester.dart index 75844c9828..213cf3c06b 100644 --- a/packages/flutter_tools/lib/src/tester/flutter_tester.dart +++ b/packages/flutter_tools/lib/src/tester/flutter_tester.dart @@ -134,7 +134,7 @@ class FlutterTesterDevice extends Device { command.add('--disable-service-auth-codes'); } if (debuggingOptions.hasObservatoryPort) { - command.add('--observatory-port=${debuggingOptions.observatoryPort}'); + command.add('--observatory-port=${debuggingOptions.hostVmServicePort}'); } } @@ -187,7 +187,9 @@ class FlutterTesterDevice extends Device { final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(), - hostPort: debuggingOptions.observatoryPort, + hostPort: debuggingOptions.hostVmServicePort, + devicePort: debuggingOptions.deviceVmServicePort, + ipv6: ipv6, ); final Uri observatoryUri = await observatoryDiscovery.uri; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart index b4fb892c52..f257a098d8 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart @@ -400,7 +400,7 @@ void main() { ], ); await completer.future; - verifyNever(portForwarder.forward(devicePort)); + verifyNever(portForwarder.forward(devicePort, hostPort: anyNamed('hostPort'))); await expectLoggerInterruptEndsTask(task, logger); await loggerSubscription.cancel(); @@ -432,7 +432,7 @@ void main() { ], ); await completer.future; - verifyNever(portForwarder.forward(devicePort)); + verifyNever(portForwarder.forward(devicePort, hostPort: anyNamed('hostPort'))); await expectLoggerInterruptEndsTask(task, logger); await loggerSubscription.cancel(); diff --git a/packages/flutter_tools/test/general.shard/ios/devices_test.dart b/packages/flutter_tools/test/general.shard/ios/devices_test.dart index 1bea91683e..56d7b72733 100644 --- a/packages/flutter_tools/test/general.shard/ios/devices_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/devices_test.dart @@ -259,7 +259,7 @@ void main() { port: 1234, path: 'observatory', ); - when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, usesIpv6: anyNamed('usesIpv6'))) .thenAnswer((Invocation invocation) => Future.value(uri)); final LaunchResult launchResult = await device.startApp(mockApp, @@ -329,7 +329,7 @@ void main() { mockLogReader.addLine('Foo'); mockLogReader.addLine('Observatory listening on http://127.0.0.1:$devicePort'); }); - when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, usesIpv6: anyNamed('usesIpv6'))) .thenAnswer((Invocation invocation) => Future.value(null)); final LaunchResult launchResult = await device.startApp(mockApp, @@ -362,7 +362,7 @@ void main() { mockLogReader.addLine('Foo'); mockLogReader.addLine('Observatory listening on http:/:/127.0.0.1:$devicePort'); }); - when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, usesIpv6: anyNamed('usesIpv6'))) .thenAnswer((Invocation invocation) => Future.value(null)); final LaunchResult launchResult = await device.startApp(mockApp, @@ -411,7 +411,7 @@ void main() { port: 1234, path: 'observatory', ); - when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, usesIpv6: anyNamed('usesIpv6'))) .thenAnswer((Invocation invocation) => Future.value(uri)); List args; diff --git a/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart b/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart index 6ad073f495..db074a0980 100644 --- a/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart +++ b/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart @@ -17,6 +17,8 @@ void main() { ProtocolDiscovery discoverer; group('no port forwarding', () { + int devicePort; + /// Performs test set-up functionality that must be performed as part of /// the `test()` pass and not part of the `setUp()` pass. /// @@ -35,7 +37,7 @@ void main() { /// See also: [runZoned] void initialize() { logReader = MockDeviceLogReader(); - discoverer = ProtocolDiscovery.observatory(logReader); + discoverer = ProtocolDiscovery.observatory(logReader, ipv6: false, hostPort: null, devicePort: devicePort); } tearDown(() { @@ -119,6 +121,28 @@ void main() { expect(uri.port, 54804); expect('$uri', 'http://127.0.0.1:54804/PTwjm8Ii8qg=/'); }); + + testUsingContext('skips uri if port does not match the requested vmservice - requested last', () async { + devicePort = 12346; + initialize(); + final Future uriFuture = discoverer.uri; + logReader.addLine('I/flutter : Observatory listening on http://127.0.0.1:12345/PTwjm8Ii8qg=/'); + logReader.addLine('I/flutter : Observatory listening on http://127.0.0.1:12346/PTwjm8Ii8qg=/'); + final Uri uri = await uriFuture; + expect(uri.port, 12346); + expect('$uri', 'http://127.0.0.1:12346/PTwjm8Ii8qg=/'); + }); + + testUsingContext('skips uri if port does not match the requested vmservice - requested first', () async { + devicePort = 12346; + initialize(); + final Future uriFuture = discoverer.uri; + logReader.addLine('I/flutter : Observatory listening on http://127.0.0.1:12346/PTwjm8Ii8qg=/'); + logReader.addLine('I/flutter : Observatory listening on http://127.0.0.1:12345/PTwjm8Ii8qg=/'); + final Uri uri = await uriFuture; + expect(uri.port, 12346); + expect('$uri', 'http://127.0.0.1:12346/PTwjm8Ii8qg=/'); + }); }); group('port forwarding', () { @@ -127,6 +151,9 @@ void main() { final ProtocolDiscovery discoverer = ProtocolDiscovery.observatory( logReader, portForwarder: MockPortForwarder(99), + hostPort: null, + devicePort: null, + ipv6: false, ); // Get next port future. @@ -146,6 +173,8 @@ void main() { logReader, portForwarder: MockPortForwarder(99), hostPort: 1243, + devicePort: null, + ipv6: false, ); // Get next port future. @@ -165,6 +194,8 @@ void main() { logReader, portForwarder: MockPortForwarder(99), hostPort: 0, + devicePort: null, + ipv6: false, ); // Get next port future. @@ -185,6 +216,7 @@ void main() { portForwarder: MockPortForwarder(99), hostPort: 54777, ipv6: true, + devicePort: null, ); // Get next port future. @@ -205,6 +237,7 @@ void main() { portForwarder: MockPortForwarder(99), hostPort: 54777, ipv6: true, + devicePort: null, ); // Get next port future.