Merge pull request #2474 from johnmccutchan/refactor_log

Refactor DeviceLogReader

packages/flutter_tools/lib/src/android/android_device.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:path/path.dart' as path;
@@ -49,6 +50,8 @@ class AndroidDevice extends Device {
 
   bool get isLocalEmulator => false;
 
+  _AdbLogReader _logReader;
+
   List<String> adbCommandForDevice(List<String> args) {
     return <String>[androidSdk.adbPath, '-s', id]..addAll(args);
   }
@@ -283,7 +286,12 @@ class AndroidDevice extends Device {
     runSync(adbCommandForDevice(<String>['-s', id, 'logcat', '-c']));
   }
 
-  DeviceLogReader createLogReader() => new _AdbLogReader(this);
+  DeviceLogReader get logReader {
+    if (_logReader == null)
+      _logReader = new _AdbLogReader(this);
+
+    return _logReader;
+  }
 
   void startTracing(AndroidApk apk) {
     runCheckedSync(adbCommandForDevice(<String>[
@@ -460,26 +468,76 @@ class _AdbLogReader extends DeviceLogReader {
 
   final AndroidDevice device;
 
+  final StreamController<String> _linesStreamController =
+      new StreamController<String>.broadcast();
+
+  Process _process;
+  StreamSubscription _stdoutSubscription;
+  StreamSubscription _stderrSubscription;
+
+  Stream<String> get lines => _linesStreamController.stream;
+
   String get name => device.name;
 
-  Future<int> logs({ bool clear: false, bool showPrefix: false }) async {
-    if (clear)
-      device.clearLogs();
+  bool get isReading => _process != null;
 
-    return await runCommandAndStreamOutput(device.adbCommandForDevice(<String>[
-      '-s',
-      device.id,
-      'logcat',
-      '-v',
-      'tag', // Only log the tag and the message
-      '-T',
-      device.lastLogcatTimestamp,
-      '-s',
-      'flutter:V',
-      'ActivityManager:W',
-      'System.err:W',
-      '*:F',
-    ]), prefix: showPrefix ? '[$name] ' : '');
+  Future get finished =>
+      _process != null ? _process.exitCode : new Future.value(0);
+
+  Future start() async {
+    if (_process != null) {
+      throw new StateError(
+          '_AdbLogReader must be stopped before it can be started.');
+    }
+
+    // Start the adb logcat process.
+    _process = await runCommand(device.adbCommandForDevice(
+      <String>[
+        '-s',
+        device.id,
+        'logcat',
+        '-v',
+        'tag', // Only log the tag and the message
+        '-T',
+        device.lastLogcatTimestamp,
+        '-s',
+        'flutter:V',
+        'ActivityManager:W',
+        'System.err:W',
+        '*:F',
+      ]));
+    _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);
+  }
+
+  Future stop() async {
+    if (_process == null) {
+      throw new StateError(
+          '_AdbLogReader must be started before it can be stopped.');
+    }
+    _stdoutSubscription?.cancel();
+    _stdoutSubscription = null;
+    _stderrSubscription?.cancel();
+    _stderrSubscription = null;
+    await _process.kill();
+    _process = null;
+  }
+
+  void _onExit(int exitCode) {
+    _stdoutSubscription?.cancel();
+    _stdoutSubscription = null;
+    _stderrSubscription?.cancel();
+    _stderrSubscription = null;
+    _process = null;
+  }
+
+  void _onLine(String line) {
+    _linesStreamController.add(line);
   }
 
   int get hashCode => name.hashCode;

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

@@ -10,20 +10,30 @@ import '../globals.dart';
 
 typedef String StringConverter(String string);
 
+/// This runs the command in the background from the specified working
+/// directory. Completes when the process has been started.
+Future<Process> runCommand(List<String> cmd, {String workingDirectory}) async {
+  printTrace(cmd.join(' '));
+  String executable = cmd[0];
+  List<String> arguments = cmd.length > 1 ? cmd.sublist(1) : [];
+  Process process = await Process.start(
+    executable,
+    arguments,
+    workingDirectory: workingDirectory
+  );
+  return process;
+}
+
 /// This runs the command and streams stdout/stderr from the child process to
-/// this process' stdout/stderr.
+/// this process' stdout/stderr. Completes with the process's exit code.
 Future<int> runCommandAndStreamOutput(List<String> cmd, {
   String workingDirectory,
   String prefix: '',
   RegExp filter,
   StringConverter mapFunction
 }) async {
-  printTrace(cmd.join(' '));
-  Process process = await Process.start(
-    cmd[0],
-    cmd.sublist(1),
-    workingDirectory: workingDirectory
-  );
+  Process process = await runCommand(cmd,
+                                     workingDirectory: workingDirectory);
   process.stdout
     .transform(UTF8.decoder)
     .transform(const LineSplitter())

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

@@ -39,13 +39,30 @@ class LogsCommand extends FlutterCommand {
 
     List<DeviceLogReader> readers = new List<DeviceLogReader>();
     for (Device device in devices) {
-      readers.add(device.createLogReader());
+      if (clear)
+        device.clearLogs();
+
+      readers.add(device.logReader);
     }
 
     printStatus('Showing ${readers.join(', ')} logs:');
 
     List<int> results = await Future.wait(readers.map((DeviceLogReader reader) async {
-      int result = await reader.logs(clear: clear, showPrefix: devices.length > 1);
+      if (!reader.isReading) {
+        // Start reading.
+        await reader.start();
+      }
+      StreamSubscription subscription = reader.lines.listen((String line) {
+        if (devices.length > 1) {
+          // Prefix with the name of the device.
+          print('[${reader.name}] $line');
+        } else {
+          print(line);
+        }
+      });
+      // Wait for the log reader to be finished.
+      int result = await reader.finished;
+      subscription.cancel();
       if (result != 0)
         printError('Error listening to $reader logs.');
       return result;

packages/flutter_tools/lib/src/device.dart

@@ -148,7 +148,11 @@ abstract class Device {
 
   TargetPlatform get platform;
 
-  DeviceLogReader createLogReader();
+  /// Get the log reader for this device.
+  DeviceLogReader get logReader;
+
+  /// Clear the device's logs.
+  void clearLogs();
 
   /// Start an app package on the current device.
   ///
@@ -189,7 +193,21 @@ abstract class Device {
 abstract class DeviceLogReader {
   String get name;
 
-  Future<int> logs({ bool clear: false, bool showPrefix: false });
+  /// 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 start();
+
+  /// Actively reading lines from the log?
+  bool get isReading;
+
+  /// Actively stop reading logs from the device.
+  Future stop();
+
+  /// Completes when the log is finished.
+  Future get finished;
 
   int get hashCode;
   bool operator ==(dynamic other);

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

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:path/path.dart' as path;
@@ -62,6 +63,8 @@ class IOSDevice extends Device {
 
   final String name;
 
+  _IOSDeviceLogReader _logReader;
+
   bool get isLocalEmulator => false;
 
   bool get supportsStartPaused => false;
@@ -220,7 +223,15 @@ class IOSDevice extends Device {
   @override
   TargetPlatform get platform => TargetPlatform.iOS;
 
-  DeviceLogReader createLogReader() => new _IOSDeviceLogReader(this);
+  DeviceLogReader get logReader {
+    if (_logReader == null)
+      _logReader = new _IOSDeviceLogReader(this);
+
+    return _logReader;
+  }
+
+  void clearLogs() {
+  }
 }
 
 class _IOSDeviceLogReader extends DeviceLogReader {
@@ -228,15 +239,65 @@ class _IOSDeviceLogReader extends DeviceLogReader {
 
   final IOSDevice device;
 
+  final StreamController<String> _linesStreamController =
+      new StreamController<String>.broadcast();
+
+  Process _process;
+  StreamSubscription _stdoutSubscription;
+  StreamSubscription _stderrSubscription;
+
+  Stream<String> get lines => _linesStreamController.stream;
+
   String get name => device.name;
 
-  // TODO(devoncarew): Support [clear].
-  Future<int> logs({ bool clear: false, bool showPrefix: false }) async {
-    return await runCommandAndStreamOutput(
-      <String>[device.loggerPath],
-      prefix: showPrefix ? '[$name] ' : '',
-      filter: new RegExp(r'Runner')
-    );
+  bool get isReading => _process != null;
+
+  Future get finished =>
+      _process != null ? _process.exitCode : new Future.value(0);
+
+  Future 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);
+  }
+
+  Future 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 _onExit(int exitCode) {
+    _stdoutSubscription?.cancel();
+    _stdoutSubscription = null;
+    _stderrSubscription?.cancel();
+    _stderrSubscription = null;
+    _process = null;
+  }
+
+  RegExp _runnerRegex = new RegExp(r'Runner');
+
+  void _onLine(String line) {
+    if (!_runnerRegex.hasMatch(line))
+      return;
+
+    _linesStreamController.add(line);
   }
 
   int get hashCode => name.hashCode;

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

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:convert' show JSON;
+import 'dart:convert';
 import 'dart:io';
 
 import 'package:path/path.dart' as path;
@@ -213,6 +213,8 @@ class IOSSimulator extends Device {
 
   bool get isLocalEmulator => true;
 
+  _IOSSimulatorLogReader _logReader;
+
   String get xcrunPath => path.join('/usr', 'bin', 'xcrun');
 
   String _getSimulatorPath() {
@@ -428,7 +430,12 @@ class IOSSimulator extends Device {
   @override
   TargetPlatform get platform => TargetPlatform.iOSSimulator;
 
-  DeviceLogReader createLogReader() => new _IOSSimulatorLogReader(this);
+  DeviceLogReader get logReader {
+    if (_logReader == null)
+      _logReader = new _IOSSimulatorLogReader(this);
+
+    return _logReader;
+  }
 
   void clearLogs() {
     File logFile = new File(logFilePath);
@@ -451,71 +458,157 @@ class _IOSSimulatorLogReader extends DeviceLogReader {
 
   final IOSSimulator device;
 
+  final StreamController<String> _linesStreamController =
+      new StreamController<String>.broadcast();
+
   bool _lastWasFiltered = false;
 
+  // We log from two logs: the device and the system log.
+  Process _deviceProcess;
+  StreamSubscription _deviceStdoutSubscription;
+  StreamSubscription _deviceStderrSubscription;
+
+  Process _systemProcess;
+  StreamSubscription _systemStdoutSubscription;
+  StreamSubscription _systemStderrSubscription;
+
+  Stream<String> get lines => _linesStreamController.stream;
+
   String get name => device.name;
 
-  Future<int> logs({ bool clear: false, bool showPrefix: false }) async {
-    if (clear)
-      device.clearLogs();
+  bool get isReading => (_deviceProcess != null) && (_systemProcess != null);
 
+  Future get finished =>
+      (_deviceProcess != null) ? _deviceProcess.exitCode : new Future.value(0);
+
+  Future start() async {
+    if (isReading) {
+      throw new StateError(
+          '_IOSSimulatorLogReader must be stopped before it can be started.');
+    }
+
+    // TODO(johnmccutchan): Add a ProcessSet abstraction that handles running
+    // N processes and merging their output.
+
+    // Device log.
     device.ensureLogsExists();
-
-    // Match the log prefix (in order to shorten it):
-    //   'Jan 29 01:31:44 devoncarew-macbookpro3 SpringBoard[96648]: ...'
-    RegExp mapRegex = new RegExp(r'\S+ +\S+ +\S+ \S+ (.+)\[\d+\]\)?: (.*)$');
-    // Jan 31 19:23:28 --- last message repeated 1 time ---
-    RegExp lastMessageRegex = new RegExp(r'\S+ +\S+ +\S+ --- (.*) ---$');
-
-    // This filter matches many Flutter lines in the log:
-    // new RegExp(r'(FlutterRunner|flutter.runner.Runner|$id)'), but it misses
-    // a fair number, including ones that would be useful in diagnosing crashes.
-    // For now, we're not filtering the log file (but do clear it with each run).
-
-    Future<int> result = runCommandAndStreamOutput(
-      <String>['tail', '-n', '+0', '-F', device.logFilePath],
-      prefix: showPrefix ? '[$name] ' : '',
-      mapFunction: (String string) {
-        Match match = mapRegex.matchAsPrefix(string);
-        if (match != null) {
-          _lastWasFiltered = true;
-
-          // Filter out some messages that clearly aren't related to Flutter.
-          if (string.contains(': could not find icon for representation -> com.apple.'))
-            return null;
-          String category = match.group(1);
-          String content = match.group(2);
-          if (category == 'Game Center' || category == 'itunesstored' || category == 'nanoregistrylaunchd' ||
-              category == 'mstreamd' || category == 'syncdefaultsd' || category == 'companionappd' ||
-              category == 'searchd')
-            return null;
-
-          _lastWasFiltered = false;
-
-          if (category == 'Runner')
-            return content;
-          return '$category: $content';
-        }
-        match = lastMessageRegex.matchAsPrefix(string);
-        if (match != null && !_lastWasFiltered)
-          return '(${match.group(1)})';
-        return string;
-      }
-    );
+    _deviceProcess = await runCommand(
+        <String>['tail', '-n', '+0', '-F', device.logFilePath]);
+    _deviceStdoutSubscription =
+        _deviceProcess.stdout.transform(UTF8.decoder)
+                             .transform(const LineSplitter()).listen(_onDeviceLine);
+    _deviceStderrSubscription =
+        _deviceProcess.stderr.transform(UTF8.decoder)
+                             .transform(const LineSplitter()).listen(_onDeviceLine);
+    _deviceProcess.exitCode.then(_onDeviceExit);
 
     // Track system.log crashes.
     // ReportCrash[37965]: Saved crash report for FlutterRunner[37941]...
-    runCommandAndStreamOutput(
-      <String>['tail', '-F', '/private/var/log/system.log'],
-      prefix: showPrefix ? '[$name] ' : '',
-      filter: new RegExp(r' FlutterRunner\[\d+\] '),
-      mapFunction: (String string) {
-        Match match = mapRegex.matchAsPrefix(string);
-        return match == null ? string : '${match.group(1)}: ${match.group(2)}';
-      }
-    );
+    _systemProcess = await runCommand(
+        <String>['tail', '-F', '/private/var/log/system.log']);
+    _systemStdoutSubscription =
+        _systemProcess.stdout.transform(UTF8.decoder)
+                             .transform(const LineSplitter()).listen(_onSystemLine);
+    _systemStderrSubscription =
+        _systemProcess.stderr.transform(UTF8.decoder)
+                             .transform(const LineSplitter()).listen(_onSystemLine);
+    _systemProcess.exitCode.then(_onSystemExit);
+  }
 
-    return await result;
+  Future stop() async {
+    if (!isReading) {
+      throw new StateError(
+          '_IOSSimulatorLogReader must be started before it can be stopped.');
+    }
+    if (_deviceProcess != null) {
+      await _deviceProcess.kill();
+      _deviceProcess = null;
+    }
+    _onDeviceExit(0);
+    if (_systemProcess != null) {
+      await _systemProcess.kill();
+      _systemProcess = null;
+    }
+    _onSystemExit(0);
+  }
+
+  void _onDeviceExit(int exitCode) {
+    _deviceStdoutSubscription?.cancel();
+    _deviceStdoutSubscription = null;
+    _deviceStderrSubscription?.cancel();
+    _deviceStderrSubscription = null;
+    _deviceProcess = null;
+  }
+
+  void _onSystemExit(int exitCode) {
+    _systemStdoutSubscription?.cancel();
+    _systemStdoutSubscription = null;
+    _systemStderrSubscription?.cancel();
+    _systemStderrSubscription = null;
+    _systemProcess = null;
+  }
+
+  // Match the log prefix (in order to shorten it):
+  //   'Jan 29 01:31:44 devoncarew-macbookpro3 SpringBoard[96648]: ...'
+  final RegExp _mapRegex =
+      new RegExp(r'\S+ +\S+ +\S+ \S+ (.+)\[\d+\]\)?: (.*)$');
+
+  // Jan 31 19:23:28 --- last message repeated 1 time ---
+  final RegExp _lastMessageRegex = new RegExp(r'\S+ +\S+ +\S+ --- (.*) ---$');
+
+  final RegExp _flutterRunnerRegex = new RegExp(r' FlutterRunner\[\d+\] ');
+
+  String _filterDeviceLine(String string) {
+    Match match = _mapRegex.matchAsPrefix(string);
+    if (match != null) {
+      _lastWasFiltered = true;
+
+      // Filter out some messages that clearly aren't related to Flutter.
+      if (string.contains(': could not find icon for representation -> com.apple.'))
+        return null;
+
+      String category = match.group(1);
+      String content = match.group(2);
+      if (category == 'Game Center' || category == 'itunesstored' ||
+          category == 'nanoregistrylaunchd' || category == 'mstreamd' ||
+          category == 'syncdefaultsd' || category == 'companionappd' ||
+          category == 'searchd')
+        return null;
+
+      _lastWasFiltered = false;
+
+      if (category == 'Runner')
+        return content;
+      return '$category: $content';
+    }
+    match = _lastMessageRegex.matchAsPrefix(string);
+    if (match != null && !_lastWasFiltered)
+      return '(${match.group(1)})';
+    return string;
+  }
+
+  void _onDeviceLine(String line) {
+    String filteredLine = _filterDeviceLine(line);
+    if (filteredLine == null)
+      return;
+
+    _linesStreamController.add(filteredLine);
+  }
+
+  String _filterSystemLog(String string) {
+    Match match = _mapRegex.matchAsPrefix(string);
+    return match == null ? string : '${match.group(1)}: ${match.group(2)}';
+  }
+
+  void _onSystemLine(String line) {
+    if (!_flutterRunnerRegex.hasMatch(line))
+      return;
+
+    String filteredLine = _filterSystemLog(line);
+    if (filteredLine == null)
+      return;
+
+    _linesStreamController.add(filteredLine);
   }
 
   int get hashCode => device.logFilePath.hashCode;