From 985da83139d35869abb5a99af88690d87a8a9375 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 13 Nov 2019 14:35:07 -0800 Subject: [PATCH] Revert "Allow specifying device-vmservice-port and host-vmservice-port (#44027)" (#44843) This reverts commit c0af77bf878a78057bc9b51a5b601ecaf746eb5c. --- .../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, 47 insertions(+), 161 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/android_device.dart b/packages/flutter_tools/lib/src/android/android_device.dart index 5aea8b3549a..1e6b0f35f33 100644 --- a/packages/flutter_tools/lib/src/android/android_device.dart +++ b/packages/flutter_tools/lib/src/android/android_device.dart @@ -549,8 +549,7 @@ class AndroidDevice extends Device { observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(), portForwarder: portForwarder, - hostPort: debuggingOptions.hostVmServicePort, - devicePort: debuggingOptions.deviceVmServicePort, + hostPort: debuggingOptions.observatoryPort, ipv6: ipv6, ); } diff --git a/packages/flutter_tools/lib/src/commands/attach.dart b/packages/flutter_tools/lib/src/commands/attach.dart index ee6a0e06c68..b9f35b57f1d 100644 --- a/packages/flutter_tools/lib/src/commands/attach.dart +++ b/packages/flutter_tools/lib/src/commands/attach.dart @@ -232,8 +232,7 @@ class AttachCommand extends FlutterCommand { observatoryUri = await MDnsObservatoryDiscovery.instance.getObservatoryUri( appId, device, - usesIpv6: usesIpv6, - deviceVmservicePort: deviceVmservicePort, + usesIpv6, ); } // If MDNS discovery fails or we're not on iOS, fallback to ProtocolDiscovery. @@ -243,9 +242,6 @@ 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; @@ -263,7 +259,7 @@ class AttachCommand extends FlutterCommand { device, debugUri?.host ?? hostname, devicePort ?? debugUri.port, - hostVmservicePort, + observatoryPort, debugUri?.path, ); } diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index 680a81709c0..4d89864f619 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, - hostVmServicePort: command.hostVmservicePort, + observatoryPort: command.observatoryPort, 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 e87b5869372..6f103579b49 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -315,8 +315,7 @@ class RunCommand extends RunCommandBase { traceSystrace: argResults['trace-systrace'], dumpSkpOnShaderCompilation: dumpSkpOnShaderCompilation, cacheSkSL: cacheSkSL, - deviceVmServicePort: deviceVmservicePort, - hostVmServicePort: hostVmservicePort, + observatoryPort: observatoryPort, 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 f5779892fdd..341aa1d01c3 100644 --- a/packages/flutter_tools/lib/src/desktop_device.dart +++ b/packages/flutter_tools/lib/src/desktop_device.dart @@ -109,11 +109,7 @@ abstract class DesktopDevice extends Device { return LaunchResult.succeeded(); } _deviceLogReader.initializeProcess(process); - final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader, - devicePort: debuggingOptions?.deviceVmServicePort, - hostPort: debuggingOptions?.hostVmServicePort, - ipv6: ipv6, - ); + final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader); 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 6fe8a060b07..adb04a36ad8 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -502,8 +502,7 @@ class DebuggingOptions { this.cacheSkSL = false, this.useTestFonts = false, this.verboseSystemLogs = false, - this.hostVmServicePort, - this.deviceVmServicePort, + this.observatoryPort, this.initializePlatform = true, this.hostname, this.port, @@ -523,8 +522,7 @@ class DebuggingOptions { traceSystrace = false, dumpSkpOnShaderCompilation = false, verboseSystemLogs = false, - hostVmServicePort = null, - deviceVmServicePort = null, + observatoryPort = null, browserLaunch = true, vmserviceOutFile = null; @@ -544,15 +542,14 @@ class DebuggingOptions { final bool verboseSystemLogs; /// Whether to invoke webOnlyInitializePlatform in Flutter for web. final bool initializePlatform; - final int hostVmServicePort; - final int deviceVmServicePort; + final int observatoryPort; 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 => hostVmServicePort != null; + bool get hasObservatoryPort => observatoryPort != null; } class LaunchResult { diff --git a/packages/flutter_tools/lib/src/ios/devices.dart b/packages/flutter_tools/lib/src/ios/devices.dart index bb481a581a8..3fff840f889 100644 --- a/packages/flutter_tools/lib/src/ios/devices.dart +++ b/packages/flutter_tools/lib/src/ios/devices.dart @@ -356,8 +356,7 @@ class IOSDevice extends Device { observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(app: package), portForwarder: portForwarder, - hostPort: debuggingOptions.hostVmServicePort, - devicePort: debuggingOptions.deviceVmServicePort, + hostPort: debuggingOptions.observatoryPort, ipv6: ipv6, ); } @@ -384,8 +383,8 @@ class IOSDevice extends Device { localUri = await MDnsObservatoryDiscovery.instance.getObservatoryUri( packageId, this, - usesIpv6: ipv6, - hostVmservicePort: debuggingOptions.hostVmServicePort, + ipv6, + debuggingOptions.observatoryPort, ); 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 05847fe5de6..5084d7ef8d3 100644 --- a/packages/flutter_tools/lib/src/ios/simulators.dart +++ b/packages/flutter_tools/lib/src/ios/simulators.dart @@ -374,18 +374,14 @@ 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.hostVmServicePort ?? 0}', + '--observatory-port=${debuggingOptions.observatoryPort ?? 0}', ], ]; ProtocolDiscovery observatoryDiscovery; if (debuggingOptions.debuggingEnabled) { observatoryDiscovery = ProtocolDiscovery.observatory( - getLogReader(app: package), - ipv6: ipv6, - hostPort: debuggingOptions.hostVmServicePort, - devicePort: debuggingOptions.deviceVmServicePort, - ); + getLogReader(app: package), ipv6: ipv6); } // 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 d824fb33327..6fc3bd4ec5c 100644 --- a/packages/flutter_tools/lib/src/mdns_discovery.dart +++ b/packages/flutter_tools/lib/src/mdns_discovery.dart @@ -50,8 +50,7 @@ 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. - // TODO(jonahwilliams): use `deviceVmservicePort` to filter mdns results. - Future query({String applicationId, int deviceVmservicePort}) async { + Future query({String applicationId}) async { printTrace('Checking for advertised Dart observatories...'); try { await client.start(); @@ -137,27 +136,14 @@ class MDnsObservatoryDiscovery { } } - Future getObservatoryUri(String applicationId, Device device, { - bool usesIpv6 = false, - int hostVmservicePort, - int deviceVmservicePort, - }) async { - final MDnsObservatoryDiscoveryResult result = await query( - applicationId: applicationId, - deviceVmservicePort: deviceVmservicePort, - ); + Future getObservatoryUri(String applicationId, Device device, [bool usesIpv6 = false, int observatoryPort]) async { + final MDnsObservatoryDiscoveryResult result = await query(applicationId: applicationId); Uri observatoryUri; if (result != null) { final String host = usesIpv6 ? InternetAddress.loopbackIPv6.address : InternetAddress.loopbackIPv4.address; - observatoryUri = await buildObservatoryUri( - device, - host, - result.port, - hostVmservicePort, - result.authCode, - ); + observatoryUri = await buildObservatoryUri(device, host, result.port, observatoryPort, result.authCode); } return observatoryUri; } @@ -173,7 +159,7 @@ Future buildObservatoryUri( Device device, String host, int devicePort, [ - int hostVmservicePort, + int observatoryPort, String authCode, ]) async { String path = '/'; @@ -185,6 +171,7 @@ Future buildObservatoryUri( if (!path.endsWith('/')) { path += '/'; } - final int actualHostPort = await device.portForwarder.forward(devicePort, hostPort: hostVmservicePort); - return Uri(scheme: 'http', host: host, port: actualHostPort, path: path); + final int localPort = observatoryPort + ?? await device.portForwarder.forward(devicePort); + return Uri(scheme: 'http', host: host, port: localPort, path: path); } diff --git a/packages/flutter_tools/lib/src/protocol_discovery.dart b/packages/flutter_tools/lib/src/protocol_discovery.dart index 803c957eee2..33768ede47e 100644 --- a/packages/flutter_tools/lib/src/protocol_discovery.dart +++ b/packages/flutter_tools/lib/src/protocol_discovery.dart @@ -4,8 +4,6 @@ import 'dart:async'; -import 'package:meta/meta.dart'; - import 'base/io.dart'; import 'device.dart'; import 'globals.dart'; @@ -18,7 +16,6 @@ class ProtocolDiscovery { this.serviceName, { this.portForwarder, this.hostPort, - this.devicePort, this.ipv6, }) : assert(logReader != null) { _deviceLogSubscription = logReader.logLines.listen(_handleLine); @@ -27,9 +24,8 @@ class ProtocolDiscovery { factory ProtocolDiscovery.observatory( DeviceLogReader logReader, { DevicePortForwarder portForwarder, - @required int hostPort, - @required int devicePort, - @required bool ipv6, + int hostPort, + bool ipv6 = false, }) { const String kObservatoryService = 'Observatory'; return ProtocolDiscovery._( @@ -37,7 +33,6 @@ class ProtocolDiscovery { kObservatoryService, portForwarder: portForwarder, hostPort: hostPort, - devicePort: devicePort, ipv6: ipv6, ); } @@ -46,7 +41,6 @@ class ProtocolDiscovery { final String serviceName; final DevicePortForwarder portForwarder; final int hostPort; - final int devicePort; final bool ipv6; final Completer _completer = Completer(); @@ -77,22 +71,17 @@ class ProtocolDiscovery { if (match != null) { try { uri = Uri.parse(match[1]); - } on FormatException catch (error, stackTrace) { + } catch (error, stackTrace) { _stopScrapingLogs(); _completer.completeError(error, stackTrace); } } - 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); + if (uri != null) { + 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 4f4c3e24960..81ec0702afd 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -216,60 +216,23 @@ abstract class FlutterCommand extends Command { /// Adds options for connecting to the Dart VM observatory port. void usesPortOptions() { argParser.addOption(observatoryPortOption, - help: '(deprecated use host-vmservice-port instead)' - 'Listen to the given port for an observatory debugger connection.\n' + help: '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 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. + /// Gets the observatory port provided to in the 'observatory-port' option. /// /// If no port is set, returns 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) { + int get observatoryPort { + if (!_usesPortOption || argResults['observatory-port'] == null) { return null; } try { - return int.parse(argResults['device-vmservice-port']); - } on FormatException catch (error) { - throwToolExit('Invalid port for `--device-vmservice-port`: $error'); + return int.parse(argResults['observatory-port']); + } catch (error) { + throwToolExit('Invalid port for `--observatory-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 213cf3c06b4..75844c98289 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.hostVmServicePort}'); + command.add('--observatory-port=${debuggingOptions.observatoryPort}'); } } @@ -187,9 +187,7 @@ class FlutterTesterDevice extends Device { final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory( getLogReader(), - hostPort: debuggingOptions.hostVmServicePort, - devicePort: debuggingOptions.deviceVmServicePort, - ipv6: ipv6, + hostPort: debuggingOptions.observatoryPort, ); 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 f257a098d8e..b4fb892c522 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, hostPort: anyNamed('hostPort'))); + verifyNever(portForwarder.forward(devicePort)); await expectLoggerInterruptEndsTask(task, logger); await loggerSubscription.cancel(); @@ -432,7 +432,7 @@ void main() { ], ); await completer.future; - verifyNever(portForwarder.forward(devicePort, hostPort: anyNamed('hostPort'))); + verifyNever(portForwarder.forward(devicePort)); 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 56d7b72733f..1bea91683e2 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, usesIpv6: anyNamed('usesIpv6'))) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) .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, usesIpv6: anyNamed('usesIpv6'))) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) .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, usesIpv6: anyNamed('usesIpv6'))) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) .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, usesIpv6: anyNamed('usesIpv6'))) + when(mockMDnsObservatoryDiscovery.getObservatoryUri(any, any, any)) .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 db074a09804..6ad073f4958 100644 --- a/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart +++ b/packages/flutter_tools/test/general.shard/protocol_discovery_test.dart @@ -17,8 +17,6 @@ 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. /// @@ -37,7 +35,7 @@ void main() { /// See also: [runZoned] void initialize() { logReader = MockDeviceLogReader(); - discoverer = ProtocolDiscovery.observatory(logReader, ipv6: false, hostPort: null, devicePort: devicePort); + discoverer = ProtocolDiscovery.observatory(logReader); } tearDown(() { @@ -121,28 +119,6 @@ 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', () { @@ -151,9 +127,6 @@ void main() { final ProtocolDiscovery discoverer = ProtocolDiscovery.observatory( logReader, portForwarder: MockPortForwarder(99), - hostPort: null, - devicePort: null, - ipv6: false, ); // Get next port future. @@ -173,8 +146,6 @@ void main() { logReader, portForwarder: MockPortForwarder(99), hostPort: 1243, - devicePort: null, - ipv6: false, ); // Get next port future. @@ -194,8 +165,6 @@ void main() { logReader, portForwarder: MockPortForwarder(99), hostPort: 0, - devicePort: null, - ipv6: false, ); // Get next port future. @@ -216,7 +185,6 @@ void main() { portForwarder: MockPortForwarder(99), hostPort: 54777, ipv6: true, - devicePort: null, ); // Get next port future. @@ -237,7 +205,6 @@ void main() { portForwarder: MockPortForwarder(99), hostPort: 54777, ipv6: true, - devicePort: null, ); // Get next port future.