Flutter run (#3553)

* rework flutter run

* fix npe with --debug-port

* connect to obs and exit when that conneciton closes

* update todos

packages/flutter_tools/lib/src/base/common.dart

@@ -2,5 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-const int observatoryDefaultPort = 8181;
-const int diagnosticDefaultPort = 8182;
+const int defaultObservatoryPort = 8100;
+const int defaultDiagnosticPort  = 8101;
+const int defaultDrivePort       = 8183;

packages/flutter_tools/lib/src/base/os.dart

@@ -109,3 +109,30 @@ Future<int> findAvailablePort() async {
   await socket.close();
   return port;
 }
+
+const int _kMaxSearchIterations = 5;
+
+/// This method will attempt to return a port close to or the same as
+/// [defaultPort]. Failing that, it will return any available port.
+Future<int> findPreferredPort(int defaultPort, { int searchStep: 2 }) async {
+  int iterationCount = 0;
+
+  while (iterationCount < _kMaxSearchIterations) {
+    int port = defaultPort + iterationCount * searchStep;
+    if (await _isPortAvailable(port))
+      return port;
+    iterationCount++;
+  }
+
+  return findAvailablePort();
+}
+
+Future<bool> _isPortAvailable(int port) async {
+  try {
+    ServerSocket socket = await ServerSocket.bind(InternetAddress.LOOPBACK_IP_V4, port);
+    await socket.close();
+    return true;
+  } catch (error) {
+    return false;
+  }
+}

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

@@ -267,8 +267,7 @@ class AppDomain extends Domain {
         command.toolchain,
         stop: true,
         target: args['target'],
-        route: args['route'],
-        checked: args['checked'] ?? true
+        route: args['route']
       );
 
       if (result != 0)

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

@@ -11,6 +11,7 @@ import 'package:test/src/executable.dart' as executable; // ignore: implementati
 import '../android/android_device.dart' show AndroidDevice;
 import '../application_package.dart';
 import '../base/file_system.dart';
+import '../base/common.dart';
 import '../base/os.dart';
 import '../device.dart';
 import '../globals.dart';
@@ -60,8 +61,9 @@ class DriveCommand extends RunCommandBase {
     );
 
     argParser.addOption('debug-port',
-        defaultsTo: '8183',
-        help: 'Listen to the given port for a debug connection.');
+      defaultsTo: defaultDrivePort.toString(),
+      help: 'Listen to the given port for a debug connection.'
+    );
   }
 
   @override
@@ -261,25 +263,22 @@ Future<int> startApp(DriveCommand command) async {
   await command.device.installApp(package);
 
   printTrace('Starting application.');
-  bool started = await command.device.startApp(
+  LaunchResult result = await command.device.startApp(
     package,
     command.toolchain,
     mainPath: mainPath,
     route: command.route,
-    checked: command.checked,
-    clearLogs: true,
-    startPaused: true,
-    observatoryPort: command.debugPort,
+    debuggingOptions: new DebuggingOptions.enabled(
+      checked: command.checked,
+      startPaused: true,
+      observatoryPort: command.debugPort
+    ),
     platformArgs: <String, dynamic>{
       'trace-startup': command.traceStartup,
     }
   );
 
-  if (started && command.device.supportsStartPaused) {
-    await delayUntilObservatoryAvailable('localhost', command.debugPort);
-  }
-
-  return started ? 0 : 2;
+  return result.started ? 0 : 2;
 }
 
 /// Runs driver tests.

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

@@ -7,6 +7,7 @@ import 'dart:io';
 
 import '../base/os.dart';
 import '../base/process.dart';
+import '../device.dart';
 import '../globals.dart';
 import 'run.dart';
 
@@ -62,7 +63,7 @@ class ListenCommand extends RunCommandBase {
         target: target,
         install: firstTime,
         stop: true,
-        checked: checked,
+        debuggingOptions: new DebuggingOptions.enabled(checked: checked),
         traceStartup: traceStartup,
         route: route
       );

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

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:io';
 
 import '../device.dart';
 import '../globals.dart';
@@ -40,17 +41,33 @@ class LogsCommand extends FlutterCommand {
 
     printStatus('Showing $logReader logs:');
 
+    Completer<int> exitCompleter = new Completer<int>();
+
     // Start reading.
-    if (!logReader.isReading)
-      await logReader.start();
+    StreamSubscription<String> subscription = logReader.logLines.listen(
+      printStatus,
+      onDone: () {
+        exitCompleter.complete(0);
+      },
+      onError: (dynamic error) {
+        exitCompleter.complete(error is int ? error : 1);
+      }
+    );
 
-    StreamSubscription<String> subscription = logReader.lines.listen(printStatus);
+    // When terminating, close down the log reader.
+    ProcessSignal.SIGINT.watch().listen((ProcessSignal signal) {
+      subscription.cancel();
+      printStatus('');
+      exitCompleter.complete(0);
+    });
+    ProcessSignal.SIGTERM.watch().listen((ProcessSignal signal) {
+      subscription.cancel();
+      exitCompleter.complete(0);
+    });
 
     // Wait for the log reader to be finished.
-    int result = await logReader.finished;
-
+    int result = await exitCompleter.future;
     subscription.cancel();
-
     if (result != 0)
       printError('Error listening to $logReader logs.');
     return result;

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

@@ -39,7 +39,8 @@ class RefreshCommand extends FlutterCommand {
       String snapshotPath = path.join(tempDir.path, 'snapshot_blob.bin');
 
       int result = await toolchain.compiler.createSnapshot(
-          mainPath: argResults['target'], snapshotPath: snapshotPath
+        mainPath: argResults['target'],
+        snapshotPath: snapshotPath
       );
       if (result != 0) {
         printError('Failed to run the Flutter compiler. Exit code: $result');

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

@@ -16,7 +16,7 @@ class SkiaCommand extends FlutterCommand {
     argParser.addOption('output-file', help: 'Write the Skia picture file to this path.');
     argParser.addOption('skiaserve', help: 'Post the picture to a skiaserve debugger at this URL.');
     argParser.addOption('diagnostic-port',
-        defaultsTo: diagnosticDefaultPort.toString(),
+        defaultsTo: defaultDiagnosticPort.toString(),
         help: 'Local port where the diagnostic server is listening.');
   }
 

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

@@ -18,7 +18,7 @@ class TraceCommand extends FlutterCommand {
     argParser.addOption('duration',
         defaultsTo: '10', abbr: 'd', help: 'Duration in seconds to trace.');
     argParser.addOption('debug-port',
-        defaultsTo: observatoryDefaultPort.toString(),
+        defaultsTo: defaultObservatoryPort.toString(),
         help: 'Local port where the observatory is listening.');
   }
 

packages/flutter_tools/lib/src/device.dart

@@ -10,6 +10,7 @@ import 'android/android_device.dart';
 import 'application_package.dart';
 import 'base/common.dart';
 import 'base/utils.dart';
+import 'base/os.dart';
 import 'build_configuration.dart';
 import 'globals.dart';
 import 'ios/devices.dart';
@@ -176,16 +177,12 @@ abstract class Device {
   ///
   /// [platformArgs] allows callers to pass platform-specific arguments to the
   /// start call.
-  Future<bool> startApp(
+  Future<LaunchResult> startApp(
     ApplicationPackage package,
     Toolchain toolchain, {
     String mainPath,
     String route,
-    bool checked: true,
-    bool clearLogs: false,
-    bool startPaused: false,
-    int observatoryPort: observatoryDefaultPort,
-    int diagnosticPort: diagnosticDefaultPort,
+    DebuggingOptions debuggingOptions,
     Map<String, dynamic> platformArgs
   });
 
@@ -229,6 +226,68 @@ abstract class Device {
   }
 }
 
+class DebuggingOptions {
+  DebuggingOptions.enabled({
+    this.checked: true,
+    this.startPaused: false,
+    this.observatoryPort,
+    this.diagnosticPort
+   }) : debuggingEnabled = true;
+
+  DebuggingOptions.disabled() :
+    debuggingEnabled = false,
+    checked = false,
+    startPaused = false,
+    observatoryPort = null,
+    diagnosticPort = null;
+
+  final bool debuggingEnabled;
+
+  final bool checked;
+  final bool startPaused;
+  final int observatoryPort;
+  final int diagnosticPort;
+
+  bool get hasObservatoryPort => observatoryPort != null;
+
+  /// Return the user specified observatory port. If that isn't available,
+  /// return [defaultObservatoryPort], or a port close to that one.
+  Future<int> findBestObservatoryPort() {
+    if (hasObservatoryPort)
+      return new Future<int>.value(observatoryPort);
+    return findPreferredPort(observatoryPort ?? defaultObservatoryPort);
+  }
+
+  bool get hasDiagnosticPort => diagnosticPort != null;
+
+  /// Return the user specified diagnostic port. If that isn't available,
+  /// return [defaultObservatoryPort], or a port close to that one.
+  Future<int> findBestDiagnosticPort() {
+    return findPreferredPort(diagnosticPort ?? defaultDiagnosticPort);
+  }
+}
+
+class LaunchResult {
+  LaunchResult.succeeded({ this.observatoryPort, this.diagnosticPort }) : started = true;
+  LaunchResult.failed() : started = false, observatoryPort = null, diagnosticPort = null;
+
+  bool get hasObservatory => observatoryPort != null;
+
+  final bool started;
+  final int observatoryPort;
+  final int diagnosticPort;
+
+  @override
+  String toString() {
+    StringBuffer buf = new StringBuffer('started=$started');
+    if (observatoryPort != null)
+      buf.write(', observatory=$observatoryPort');
+    if (diagnosticPort != null)
+      buf.write(', diagnostic=$diagnosticPort');
+    return buf.toString();
+  }
+}
+
 class ForwardedPort {
   ForwardedPort(this.hostPort, this.devicePort);
 
@@ -248,40 +307,18 @@ abstract class DevicePortForwarder {
   /// Forward [hostPort] on the host to [devicePort] on the device.
   /// If [hostPort] is null, will auto select a host port.
   /// Returns a Future that completes with the host port.
-  Future<int> forward(int devicePort, {int hostPort: null});
+  Future<int> forward(int devicePort, { int hostPort: null });
 
   /// Stops forwarding [forwardedPort].
   Future<Null> unforward(ForwardedPort forwardedPort);
 }
 
-/// Read the log for a particular device. Subclasses must implement `hashCode`
-/// and `operator ==` so that log readers that read from the same location can be
-/// de-duped. For example, two Android devices will both try and log using
-/// `adb logcat`; we don't want to display two identical log streams.
+/// Read the log for a particular device.
 abstract class DeviceLogReader {
   String get name;
 
-  /// A broadcast stream where each element in the string is a line of log
-  /// output.
-  Stream<String> get lines;
-
-  /// Start reading logs from the device.
-  Future<Null> start();
-
-  /// Actively reading lines from the log?
-  bool get isReading;
-
-  /// Actively stop reading logs from the device.
-  Future<Null> stop();
-
-  /// Completes when the log is finished.
-  Future<int> get finished;
-
-  @override
-  int get hashCode;
-
-  @override
-  bool operator ==(dynamic other);
+  /// A broadcast stream where each element in the string is a line of log output.
+  Stream<String> get logLines;
 
   @override
   String toString() => name;

packages/flutter_tools/lib/src/ios/devices.dart

@@ -9,7 +9,6 @@ import 'dart:io';
 import 'package:path/path.dart' as path;
 
 import '../application_package.dart';
-import '../base/common.dart';
 import '../base/os.dart';
 import '../base/process.dart';
 import '../build_configuration.dart';
@@ -154,19 +153,15 @@ class IOSDevice extends Device {
   }
 
   @override
-  Future<bool> startApp(
+  Future<LaunchResult> startApp(
     ApplicationPackage app,
     Toolchain toolchain, {
     String mainPath,
     String route,
-    bool checked: true,
-    bool clearLogs: false,
-    bool startPaused: false,
-    int observatoryPort: observatoryDefaultPort,
-    int diagnosticPort: diagnosticDefaultPort,
+    DebuggingOptions debuggingOptions,
     Map<String, dynamic> platformArgs
   }) async {
-    // TODO(chinmaygarde): Use checked, mainPath, route, clearLogs.
+    // TODO(chinmaygarde): Use checked, mainPath, route.
     // TODO(devoncarew): Handle startPaused, debugPort.
     printTrace('Building ${app.name} for $id');
 
@@ -174,7 +169,7 @@ class IOSDevice extends Device {
     bool buildResult = await buildIOSXcodeProject(app, buildForDevice: true);
     if (!buildResult) {
       printError('Could not build the precompiled application for the device.');
-      return false;
+      return new LaunchResult.failed();
     }
 
     // Step 2: Check that the application exists at the specified path.
@@ -182,7 +177,7 @@ class IOSDevice extends Device {
     bool bundleExists = bundle.existsSync();
     if (!bundleExists) {
       printError('Could not find the built application bundle at ${bundle.path}.');
-      return false;
+      return new LaunchResult.failed();
     }
 
     // Step 3: Attempt to install the application on the device.
@@ -197,10 +192,10 @@ class IOSDevice extends Device {
 
     if (installationResult != 0) {
       printError('Could not install ${bundle.path} on $id.');
-      return false;
+      return new LaunchResult.failed();
     }
 
-    return true;
+    return new LaunchResult.succeeded();
   }
 
   @override
@@ -266,90 +261,46 @@ class IOSDevice extends Device {
 }
 
 class _IOSDeviceLogReader extends DeviceLogReader {
-  _IOSDeviceLogReader(this.device);
+  _IOSDeviceLogReader(this.device) {
+    _linesController = new StreamController<String>.broadcast(
+     onListen: _start,
+     onCancel: _stop
+   );
+  }
 
   final IOSDevice device;
 
-  final StreamController<String> _linesStreamController =
-      new StreamController<String>.broadcast();
-
+  StreamController<String> _linesController;
   Process _process;
-  StreamSubscription<String> _stdoutSubscription;
-  StreamSubscription<String> _stderrSubscription;
 
   @override
-  Stream<String> get lines => _linesStreamController.stream;
+  Stream<String> get logLines => _linesController.stream;
 
   @override
   String get name => device.name;
 
-  @override
-  bool get isReading => _process != null;
-
-  @override
-  Future<int> get finished {
-    return _process != null ? _process.exitCode : new Future<int>.value(0);
-  }
-
-  @override
-  Future<Null> start() async {
-    if (_process != null) {
-      throw new StateError(
-        '_IOSDeviceLogReader must be stopped before it can be started.'
-      );
-    }
-    _process = await runCommand(<String>[device.loggerPath]);
-    _stdoutSubscription =
-        _process.stdout.transform(UTF8.decoder)
-                       .transform(const LineSplitter()).listen(_onLine);
-    _stderrSubscription =
-        _process.stderr.transform(UTF8.decoder)
-                       .transform(const LineSplitter()).listen(_onLine);
-    _process.exitCode.then(_onExit);
-  }
-
-  @override
-  Future<Null> stop() async {
-    if (_process == null) {
-      throw new StateError(
-        '_IOSDeviceLogReader must be started before it can be stopped.'
-      );
-    }
-    _stdoutSubscription?.cancel();
-    _stdoutSubscription = null;
-    _stderrSubscription?.cancel();
-    _stderrSubscription = null;
-    await _process.kill();
-    _process = null;
-  }
+  void _start() {
+    runCommand(<String>[device.loggerPath]).then((Process process) {
+      _process = process;
+      _process.stdout.transform(UTF8.decoder).transform(const LineSplitter()).listen(_onLine);
+      _process.stderr.transform(UTF8.decoder).transform(const LineSplitter()).listen(_onLine);
 
-  void _onExit(int exitCode) {
-    _stdoutSubscription?.cancel();
-    _stdoutSubscription = null;
-    _stderrSubscription?.cancel();
-    _stderrSubscription = null;
-    _process = null;
+      _process.exitCode.then((int code) {
+        if (_linesController.hasListener)
+          _linesController.close();
+      });
+    });
   }
 
-  RegExp _runnerRegex = new RegExp(r'Runner');
+  static final RegExp _runnerRegex = new RegExp(r'FlutterRunner');
 
   void _onLine(String line) {
-    if (!_runnerRegex.hasMatch(line))
-      return;
-
-    _linesStreamController.add(line);
+    if (_runnerRegex.hasMatch(line))
+      _linesController.add(line);
   }
 
-  @override
-  int get hashCode => name.hashCode;
-
-  @override
-  bool operator ==(dynamic other) {
-    if (identical(this, other))
-      return true;
-    if (other is! _IOSDeviceLogReader)
-      return false;
-    return other.name == name;
+  void _stop() {
+    _process?.kill();
   }
 }
 

packages/flutter_tools/lib/src/service_protocol.dart

@@ -8,28 +8,30 @@ import 'device.dart';
 
 /// Discover service protocol ports on devices.
 class ServiceProtocolDiscovery {
-  static const String kObservatoryService = 'Observatory';
-  static const String kDiagnosticService = 'Diagnostic server';
-
-  /// [logReader] A [DeviceLogReader] to look for service messages in.
+  /// [logReader] - a [DeviceLogReader] to look for service messages in.
   ServiceProtocolDiscovery(DeviceLogReader logReader, String serviceName)
-      : _logReader = logReader,
-        _serviceName = serviceName {
+      : _logReader = logReader, _serviceName = serviceName {
     assert(_logReader != null);
-    if (!_logReader.isReading)
-      _logReader.start();
-
-    _logReader.lines.listen(_onLine);
+    _subscription = _logReader.logLines.listen(_onLine);
   }
 
+  static const String kObservatoryService = 'Observatory';
+  static const String kDiagnosticService = 'Diagnostic server';
+
   final DeviceLogReader _logReader;
   final String _serviceName;
+
   Completer<int> _completer = new Completer<int>();
+  StreamSubscription<String> _subscription;
 
   /// The [Future] returned by this function will complete when the next service
   /// protocol port is found.
   Future<int> nextPort() => _completer.future;
 
+  void cancel() {
+    _subscription.cancel();
+  }
+
   void _onLine(String line) {
     int portNumber = 0;
     if (line.contains('$_serviceName listening on http://')) {
@@ -48,6 +50,7 @@ class ServiceProtocolDiscovery {
   void _located(int port) {
     assert(_completer != null);
     assert(!_completer.isCompleted);
+
     _completer.complete(port);
     _completer = new Completer<int>();
   }

packages/flutter_tools/lib/src/toolchain.dart

@@ -26,6 +26,9 @@ class SnapshotCompiler {
     String depfilePath,
     String buildOutputPath
   }) {
+    assert(mainPath != null);
+    assert(snapshotPath != null);
+
     final List<String> args = [
       _path,
       mainPath,

packages/flutter_tools/test/service_protocol_test.dart

@@ -15,36 +15,39 @@ void main() {
       MockDeviceLogReader logReader = new MockDeviceLogReader();
       ServiceProtocolDiscovery discoverer =
           new ServiceProtocolDiscovery(logReader, ServiceProtocolDiscovery.kObservatoryService);
+
       // Get next port future.
       Future<int> nextPort = discoverer.nextPort();
       expect(nextPort, isNotNull);
+
       // Inject some lines.
       logReader.addLine('HELLO WORLD');
-      logReader.addLine(
-          'Observatory listening on http://127.0.0.1:9999');
+      logReader.addLine('Observatory listening on http://127.0.0.1:9999');
       // Await the port.
       expect(await nextPort, 9999);
+
       // Get next port future.
       nextPort = discoverer.nextPort();
-      logReader.addLine(
-          'Observatory listening on http://127.0.0.1:3333');
+      logReader.addLine('Observatory listening on http://127.0.0.1:3333');
       expect(await nextPort, 3333);
+
       // Get next port future.
       nextPort = discoverer.nextPort();
       // Inject some bad lines.
       logReader.addLine('Observatory listening on http://127.0.0.1');
       logReader.addLine('Observatory listening on http://127.0.0.1:');
-      logReader.addLine(
-      'Observatory listening on http://127.0.0.1:apple');
+      logReader.addLine('Observatory listening on http://127.0.0.1:apple');
       int port = await nextPort.timeout(
       const Duration(milliseconds: 100), onTimeout: () => 77);
       // Expect the timeout port.
       expect(port, 77);
+
       // Get next port future.
       nextPort = discoverer.nextPort();
-      logReader.addLine(
-          'I/flutter : Observatory listening on http://127.0.0.1:52584');
+      logReader.addLine('I/flutter : Observatory listening on http://127.0.0.1:52584');
       expect(await nextPort, 52584);
+
+      discoverer.cancel();
     });
   });
 }

packages/flutter_tools/test/src/mocks.dart

@@ -63,38 +63,12 @@ class MockDeviceLogReader extends DeviceLogReader {
   @override
   String get name => 'MockLogReader';
 
-  final StreamController<String> _linesStreamController =
-      new StreamController<String>.broadcast();
-
-  final Completer<int> _finishedCompleter = new Completer<int>();
-
-  @override
-  Stream<String> get lines => _linesStreamController.stream;
-
-  void addLine(String line) {
-    _linesStreamController.add(line);
-  }
-
-  bool _started = false;
+  final StreamController<String> _linesController = new StreamController<String>.broadcast();
 
   @override
-  Future<Null> start() async {
-    assert(!_started);
-    _started = true;
-  }
+  Stream<String> get logLines => _linesController.stream;
 
-  @override
-  bool get isReading => _started;
-
-  @override
-  Future<Null> stop() {
-    assert(_started);
-    _started = false;
-    return new Future<Null>.value();
-  }
-
-  @override
-  Future<int> get finished => _finishedCompleter.future;
+  void addLine(String line) => _linesController.add(line);
 }
 
 void applyMocksToCommand(FlutterCommand command) {