Add ipv6 and observatory port support to the attach command (#25303)

* Add ipv6 and observatory port support to the attach command.

* Remove whitespace

* Explain why a name change is useful here

* Refactor common flags out into the parent

* Add an additional test case for when observatory port is provided but debug port is not

* Remove whitespace

* Fix flag validation

packages/flutter_tools/lib/src/commands/attach.dart

@@ -48,12 +48,14 @@ class AttachCommand extends FlutterCommand {
     addBuildModeFlags(defaultToRelease: false);
     usesIsolateFilterOption(hide: !verboseHelp);
     usesTargetOption();
+    usesPortOptions();
+    usesIpv6Flag();
     usesFilesystemOptions(hide: !verboseHelp);
     usesFuchsiaOptions(hide: !verboseHelp);
     argParser
       ..addOption(
         'debug-port',
-        help: 'Local port where the observatory is listening.',
+        help: 'Device port where the observatory is listening.',
       )..addOption('pid-file',
         help: 'Specify a file to write the process id to. '
               'You can send SIGUSR1 to trigger a hot reload '
@@ -79,7 +81,7 @@ class AttachCommand extends FlutterCommand {
   @override
   final String description = 'Attach to a running application.';
 
-  int get observatoryPort {
+  int get debugPort {
     if (argResults['debug-port'] == null)
       return null;
     try {
@@ -95,7 +97,19 @@ class AttachCommand extends FlutterCommand {
     await super.validateCommand();
     if (await findTargetDevice() == null)
       throwToolExit(null);
-    observatoryPort;
+    debugPort;
+    if (debugPort == null && argResults.wasParsed(FlutterCommand.ipv6Flag)) {
+      throwToolExit(
+        'When the --debug-port is unknown, this command determines '
+        'the value of --ipv6 on its own.',
+      );
+    }
+    if (debugPort == null && argResults.wasParsed(FlutterCommand.observatoryPortOption)) {
+      throwToolExit(
+        'When the --debug-port is unknown, this command does not use '
+        'the value of --observatory-port.',
+      );
+    }
   }
 
   @override
@@ -107,7 +121,7 @@ class AttachCommand extends FlutterCommand {
     writePidFile(argResults['pid-file']);
 
     final Device device = await findTargetDevice();
-    final int devicePort = observatoryPort;
+    final int devicePort = debugPort;
 
     final Daemon daemon = argResults['machine']
       ? Daemon(stdinCommandStream, stdoutCommandResponse,
@@ -115,7 +129,7 @@ class AttachCommand extends FlutterCommand {
       : null;
 
     Uri observatoryUri;
-    bool ipv6 = false;
+    bool usesIpv6 = false;
     bool attachLogger = false;
     if (devicePort == null) {
       if (device is FuchsiaDevice) {
@@ -124,7 +138,7 @@ class AttachCommand extends FlutterCommand {
         if (module == null) {
           throwToolExit('\'--module\' is requried for attaching to a Fuchsia device');
         }
-        ipv6 = _isIpv6(device.id);
+        usesIpv6 = _isIpv6(device.id);
         final List<int> ports = await device.servicePorts();
         if (ports.isEmpty) {
           throwToolExit('No active service ports on ${device.name}');
@@ -142,7 +156,7 @@ class AttachCommand extends FlutterCommand {
           if (localPort == null) {
             throwToolExit('No active Observatory running module \'$module\' on ${device.name}');
           }
-          observatoryUri = ipv6
+          observatoryUri = usesIpv6
             ? Uri.parse('http://[$ipv6Loopback]:$localPort/')
             : Uri.parse('http://$ipv4Loopback:$localPort/');
           status.stop();
@@ -163,14 +177,20 @@ class AttachCommand extends FlutterCommand {
           );
           printStatus('Waiting for a connection from Flutter on ${device.name}...');
           observatoryUri = await observatoryDiscovery.uri;
+          // Determine ipv6 status from the scanned logs.
+          usesIpv6 = observatoryDiscovery.ipv6;
           printStatus('Done.');
         } finally {
           await observatoryDiscovery?.cancel();
         }
       }
     } else {
-      final int localPort = await device.portForwarder.forward(devicePort);
-      observatoryUri = Uri.parse('http://$ipv4Loopback:$localPort/');
+      usesIpv6 = ipv6;
+      final int localPort = observatoryPort
+        ?? await device.portForwarder.forward(devicePort);
+      observatoryUri = usesIpv6
+        ? Uri.parse('http://[$ipv6Loopback]:$localPort/')
+        : Uri.parse('http://$ipv4Loopback:$localPort/');
     }
     try {
       final FlutterDevice flutterDevice = FlutterDevice(
@@ -191,7 +211,7 @@ class AttachCommand extends FlutterCommand {
         usesTerminalUI: daemon == null,
         projectRootPath: argResults['project-root'],
         dillOutputPath: argResults['output-dill'],
-        ipv6: ipv6,
+        ipv6: usesIpv6,
       );
       if (attachLogger) {
         flutterDevice.startEchoingDeviceLog();

packages/flutter_tools/lib/src/commands/run.dart

@@ -30,12 +30,6 @@ abstract class RunCommandBase extends FlutterCommand {
         negatable: false,
         help: 'Start tracing during startup.',
       )
-      ..addFlag('ipv6',
-        hide: true,
-        negatable: false,
-        help: 'Binds to IPv6 localhost instead of IPv4 when the flutter tool '
-              'forwards the host port to a device port.',
-      )
       ..addOption('route',
         help: 'Which route to load when running the app.',
       )
@@ -46,31 +40,13 @@ abstract class RunCommandBase extends FlutterCommand {
               'Android device.\nIgnored on iOS.');
     usesTargetOption();
     usesPortOptions();
+    usesIpv6Flag();
     usesPubOption();
     usesIsolateFilterOption(hide: !verboseHelp);
   }
 
   bool get traceStartup => argResults['trace-startup'];
-  bool get ipv6 => argResults['ipv6'];
   String get route => argResults['route'];
-
-  void usesPortOptions() {
-    argParser.addOption('observatory-port',
-        help: 'Listen to the given port for an observatory debugger connection.\n'
-              'Specifying port 0 (the default) will find a random free port.'
-    );
-  }
-
-  int get observatoryPort {
-    if (argResults['observatory-port'] != null) {
-      try {
-        return int.parse(argResults['observatory-port']);
-      } catch (error) {
-        throwToolExit('Invalid port for `--observatory-port`: $error');
-      }
-    }
-    return null;
-  }
 }
 
 class RunCommand extends RunCommandBase {

packages/flutter_tools/lib/src/runner/flutter_command.dart

@@ -72,6 +72,12 @@ abstract class FlutterCommand extends Command<void> {
   /// Will be `null` until the top-most command has begun execution.
   static FlutterCommand get current => context[FlutterCommand];
 
+  /// The option name for a custom observatory port.
+  static const String observatoryPortOption = 'observatory-port';
+
+  /// The flag name for whether or not to use ipv6.
+  static const String ipv6Flag = 'ipv6';
+
   @override
   ArgParser get argParser => _argParser;
   final ArgParser _argParser = ArgParser(allowTrailingOptions: false);
@@ -86,6 +92,10 @@ abstract class FlutterCommand extends Command<void> {
 
   bool _usesPubOption = false;
 
+  bool _usesPortOption = false;
+
+  bool _usesIpv6Flag = false;
+
   bool get shouldRunPub => _usesPubOption && argResults['pub'];
 
   bool get shouldUpdateCache => true;
@@ -148,6 +158,43 @@ abstract class FlutterCommand extends Command<void> {
       );
   }
 
+  /// 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'
+              'Specifying port 0 (the default) will find a random free port.'
+    );
+    _usesPortOption = true;
+  }
+
+  /// Gets the observatory port provided to in the 'observatory-port' option.
+  ///
+  /// If no port is set, returns null.
+  int get observatoryPort {
+    if (!_usesPortOption || argResults['observatory-port'] == null) {
+      return null;
+    }
+    try {
+      return int.parse(argResults['observatory-port']);
+    } catch (error) {
+      throwToolExit('Invalid port for `--observatory-port`: $error');
+    }
+    return null;
+  }
+
+  void usesIpv6Flag() {
+    argParser.addFlag(ipv6Flag,
+      hide: true,
+      negatable: false,
+      help: 'Binds to IPv6 localhost instead of IPv4 when the flutter tool '
+            'forwards the host port to a device port. Not used when the '
+            '--debug-port flag is not set.',
+    );
+    _usesIpv6Flag = true;
+  }
+
+  bool get ipv6 => _usesIpv6Flag ? argResults['ipv6'] : null;
+
   void usesBuildNumberOption() {
     argParser.addOption('build-number',
         help: 'An integer used as an internal version number.\n'

packages/flutter_tools/test/commands/attach_test.dart

@@ -104,6 +104,7 @@ void main() {
             debuggingOptions: anyNamed('debuggingOptions'),
             packagesFilePath: anyNamed('packagesFilePath'),
             usesTerminalUI: anyNamed('usesTerminalUI'),
+            ipv6: false,
           ),
         )..thenReturn(MockHotRunner());
 
@@ -134,6 +135,7 @@ void main() {
             debuggingOptions: anyNamed('debuggingOptions'),
             packagesFilePath: anyNamed('packagesFilePath'),
             usesTerminalUI: anyNamed('usesTerminalUI'),
+            ipv6: false,
           ),
         )..called(1);
 
@@ -150,6 +152,36 @@ void main() {
       }, overrides: <Type, Generator>{
         FileSystem: () => testFileSystem,
       });
+
+      testUsingContext('exits when ipv6 is specified and debug-port is not', () async {
+        testDeviceManager.addDevice(device);
+
+        final AttachCommand command = AttachCommand();
+        await expectLater(
+          createTestCommandRunner(command).run(<String>['attach', '--ipv6']),
+          throwsToolExit(
+            message: 'When the --debug-port is unknown, this command determines '
+                     'the value of --ipv6 on its own.',
+          ),
+        );
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      },);
+
+      testUsingContext('exits when observatory-port is specified and debug-port is not', () async {
+        testDeviceManager.addDevice(device);
+
+        final AttachCommand command = AttachCommand();
+        await expectLater(
+          createTestCommandRunner(command).run(<String>['attach', '--observatory-port', '100']),
+          throwsToolExit(
+            message: 'When the --debug-port is unknown, this command does not use '
+                     'the value of --observatory-port.',
+          ),
+        );
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      },);
     });
 
 
@@ -170,7 +202,8 @@ void main() {
           target: anyNamed('target'),
           debuggingOptions: anyNamed('debuggingOptions'),
           packagesFilePath: anyNamed('packagesFilePath'),
-          usesTerminalUI: anyNamed('usesTerminalUI'))).thenReturn(
+          usesTerminalUI: anyNamed('usesTerminalUI'),
+          ipv6: false)).thenReturn(
           MockHotRunner());
 
       testDeviceManager.addDevice(device);
@@ -199,33 +232,92 @@ void main() {
           target: foo.path,
           debuggingOptions: anyNamed('debuggingOptions'),
           packagesFilePath: anyNamed('packagesFilePath'),
-          usesTerminalUI: anyNamed('usesTerminalUI'))).called(1);
+          usesTerminalUI: anyNamed('usesTerminalUI'),
+          ipv6: false)).called(1);
     }, overrides: <Type, Generator>{
       FileSystem: () => testFileSystem,
     },);
 
-    testUsingContext('forwards to given port', () async {
+    group('forwarding to given port', () {
       const int devicePort = 499;
       const int hostPort = 42;
-      final MockPortForwarder portForwarder = MockPortForwarder();
-      final MockAndroidDevice device = MockAndroidDevice();
+      MockPortForwarder portForwarder;
+      MockAndroidDevice device;
 
-      when(device.portForwarder).thenReturn(portForwarder);
-      when(portForwarder.forward(devicePort)).thenAnswer((_) async => hostPort);
-      when(portForwarder.forwardedPorts).thenReturn(
-          <ForwardedPort>[ForwardedPort(hostPort, devicePort)]);
-      when(portForwarder.unforward(any)).thenAnswer((_) async => null);
-      testDeviceManager.addDevice(device);
+      setUp(() {
+        portForwarder = MockPortForwarder();
+        device = MockAndroidDevice();
 
-      final AttachCommand command = AttachCommand();
+        when(device.portForwarder).thenReturn(portForwarder);
+        when(portForwarder.forward(devicePort)).thenAnswer((_) async => hostPort);
+        when(portForwarder.forwardedPorts).thenReturn(
+            <ForwardedPort>[ForwardedPort(hostPort, devicePort)]);
+        when(portForwarder.unforward(any)).thenAnswer((_) async => null);
+      });
 
-      await createTestCommandRunner(command).run(
-          <String>['attach', '--debug-port', '$devicePort']);
+      testUsingContext('succeeds in ipv4 mode', () async {
+        testDeviceManager.addDevice(device);
+        final AttachCommand command = AttachCommand();
 
-      verify(portForwarder.forward(devicePort)).called(1);
-    }, overrides: <Type, Generator>{
-      FileSystem: () => testFileSystem,
-    },);
+        await createTestCommandRunner(command).run(
+            <String>['attach', '--debug-port', '$devicePort']);
+
+        verify(portForwarder.forward(devicePort)).called(1);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      });
+
+      testUsingContext('succeeds in ipv6 mode', () async {
+        testDeviceManager.addDevice(device);
+        final AttachCommand command = AttachCommand();
+
+        await createTestCommandRunner(command).run(
+            <String>['attach', '--debug-port', '$devicePort', '--ipv6']);
+
+        verify(portForwarder.forward(devicePort)).called(1);
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      });
+
+      testUsingContext('skips in ipv4 mode with a provided observatory port', () async {
+        testDeviceManager.addDevice(device);
+        final AttachCommand command = AttachCommand();
+
+        await createTestCommandRunner(command).run(
+            <String>[
+              'attach',
+              '--debug-port',
+              '$devicePort',
+              '--observatory-port',
+              '$hostPort',
+            ],
+        );
+
+        verifyNever(portForwarder.forward(devicePort));
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      });
+
+      testUsingContext('skips in ipv6 mode with a provided observatory port', () async {
+        testDeviceManager.addDevice(device);
+        final AttachCommand command = AttachCommand();
+
+        await createTestCommandRunner(command).run(
+            <String>[
+              'attach',
+              '--debug-port',
+              '$devicePort',
+              '--observatory-port',
+              '$hostPort',
+              '--ipv6',
+            ],
+        );
+
+        verifyNever(portForwarder.forward(devicePort));
+      }, overrides: <Type, Generator>{
+        FileSystem: () => testFileSystem,
+      });
+    });
 
     testUsingContext('exits when no device connected', () async {
       final AttachCommand command = AttachCommand();