Move platform-specific log-reading implementation details from ResidentRunner/FlutterDevice to DeviceLogReader implementations (#156181)

Cleans up flutter/flutter#155800. In summary, `ResidentRunner`/`FlutterDevice` have branching behavior around logging that depends on the type of `DeviceLogReader` on the `FlutterDevice` instance.  Let's instead move this behavior to the `DeviceLogReader` implementations.

My apologies for the large diff. Much of this is a refactor that was a bit too difficult to separate into its own commits. 

Here are the main two changes

* Replaces the mutable `connectedVmService` field on the `DeviceLogReader` class with a new method `provideVmService`. This serves largely the same purpose as the mutable field, but it allows for asynchronous code. This is where we put the logic that used to exist in `FlutterDevice.tryInitLogReader`.
* Removes the `tryInitLogReader` method from `FlutterDevice`. This method served to set the `appPid` field on the `FlutterDevice`'s `DeviceLogReader` instance. This was only used in the case of Android to filter out logs unrelated to the flutter app coming from the device, so we can move this logic to `AdbLogReader`'s implementation of `provideVmService`.

Author: andrewkolos

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

@@ -6,6 +6,7 @@ import 'dart:async';
 
 import 'package:meta/meta.dart';
 import 'package:process/process.dart';
+import 'package:vm_service/vm_service.dart';
 
 import '../application_package.dart';
 import '../base/common.dart' show throwToolExit;
@@ -21,6 +22,7 @@ import '../device_port_forwarder.dart';
 import '../device_vm_service_discovery_for_attach.dart';
 import '../project.dart';
 import '../protocol_discovery.dart';
+import '../vmservice.dart';
 import 'android.dart';
 import 'android_builder.dart';
 import 'android_console.dart';
@@ -618,6 +620,7 @@ class AndroidDevice extends Device {
         await AdbLogReader.createLogReader(
           this,
           _processManager,
+          _logger,
         ),
         portForwarder: portForwarder,
         hostPort: debuggingOptions.hostVmServicePort,
@@ -796,12 +799,14 @@ class AndroidDevice extends Device {
       return _pastLogReader ??= await AdbLogReader.createLogReader(
         this,
         _processManager,
+        _logger,
         includePastLogs: true,
       );
     } else {
       return _logReader ??= await AdbLogReader.createLogReader(
         this,
         _processManager,
+        _logger,
       );
     }
   }
@@ -1042,15 +1047,20 @@ class AndroidMemoryInfo extends MemoryInfo {
 
 /// A log reader that logs from `adb logcat`.
 class AdbLogReader extends DeviceLogReader {
-  AdbLogReader._(this._adbProcess, this.name);
+  AdbLogReader._(this._adbProcess, this.name, this._logger);
 
   @visibleForTesting
-  factory AdbLogReader.test(Process adbProcess, String name) = AdbLogReader._;
+  factory AdbLogReader.test(
+    Process adbProcess,
+    String name,
+    Logger logger,
+  ) = AdbLogReader._;
 
   /// Create a new [AdbLogReader] from an [AndroidDevice] instance.
   static Future<AdbLogReader> createLogReader(
     AndroidDevice device,
-    ProcessManager processManager, {
+    ProcessManager processManager,
+    Logger logger, {
     bool includePastLogs = false,
   }) async {
     // logcat -T is not supported on Android releases before Lollipop.
@@ -1085,11 +1095,15 @@ class AdbLogReader extends DeviceLogReader {
       ]);
     }
     final Process process = await processManager.start(device.adbCommandForDevice(args));
-    return AdbLogReader._(process, device.name);
+    return AdbLogReader._(process, device.name, logger);
   }
 
+  int? _appPid;
+
   final Process _adbProcess;
 
+  final Logger _logger;
+
   @override
   final String name;
 
@@ -1101,6 +1115,17 @@ class AdbLogReader extends DeviceLogReader {
   @override
   Stream<String> get logLines => _linesController.stream;
 
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async {
+    final VM? vm = await connectedVmService.getVmGuarded();
+    if (vm == null) {
+      _logger.printError('An error occurred when setting up filtering for adb logs. '
+        'Unable to communicate with the VM service.');
+    } else {
+      _appPid = vm.pid;
+    }
+  }
+
   void _start() {
     // We expect logcat streams to occasionally contain invalid utf-8,
     // see: https://github.com/flutter/flutter/pull/8864.
@@ -1189,7 +1214,7 @@ class AdbLogReader extends DeviceLogReader {
             _fatalCrash = false;
           }
         }
-      } else if (appPid != null && int.parse(logMatch.group(1)!) == appPid) {
+      } else if (_appPid != null && int.parse(logMatch.group(1)!) == _appPid) {
         acceptLine = !_surfaceSyncerSpam.hasMatch(line);
 
         if (_fatalLog.hasMatch(line)) {

packages/flutter_tools/lib/src/custom_devices/custom_device.dart

@@ -23,6 +23,7 @@ import '../device_port_forwarder.dart';
 import '../features.dart';
 import '../project.dart';
 import '../protocol_discovery.dart';
+import '../vmservice.dart';
 import 'custom_device_config.dart';
 import 'custom_device_workflow.dart';
 import 'custom_devices_config.dart';
@@ -111,6 +112,9 @@ class CustomDeviceLogReader extends DeviceLogReader {
 
   @override
   Stream<String> get logLines => logLinesController.stream;
+
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async { }
 }
 
 /// A [DevicePortForwarder] that uses commands to forward / unforward a port.

packages/flutter_tools/lib/src/desktop_device.dart

@@ -19,6 +19,7 @@ import 'device_port_forwarder.dart';
 import 'globals.dart' as globals;
 import 'macos/macos_device.dart';
 import 'protocol_discovery.dart';
+import 'vmservice.dart';
 
 /// A partial implementation of Device for desktop-class devices to inherit
 /// from, containing implementations that are common to all desktop devices.
@@ -384,4 +385,7 @@ class DesktopLogReader extends DeviceLogReader {
   void dispose() {
     // Nothing to dispose.
   }
+
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async { }
 }

packages/flutter_tools/lib/src/device.dart

@@ -1440,14 +1440,11 @@ abstract class DeviceLogReader {
 
   /// Some logs can be obtained from a VM service stream.
   /// Set this after the VM services are connected.
-  FlutterVmService? connectedVMService;
+  Future<void> provideVmService(FlutterVmService connectedVmService);
 
   @override
   String toString() => name;
 
-  /// Process ID of the app on the device.
-  int? appPid;
-
   // Clean up resources allocated by log reader e.g. subprocesses
   void dispose();
 }
@@ -1466,17 +1463,14 @@ class NoOpDeviceLogReader implements DeviceLogReader {
   @override
   final String name;
 
-  @override
-  int? appPid;
-
-  @override
-  FlutterVmService? connectedVMService;
-
   @override
   Stream<String> get logLines => const Stream<String>.empty();
 
   @override
   void dispose() { }
+
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async {}
 }
 
 /// Append --null_assertions to any existing Dart VM flags if

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

@@ -231,9 +231,7 @@ class FlutterDriverService extends DriverService {
     _vmService = await _vmServiceConnector(uri, device: _device, logger: _logger);
     final DeviceLogReader logReader = await device.getLogReader(app: _applicationPackage);
     logReader.logLines.listen(_logger.printStatus);
-
-    final vm_service.VM vm = await _vmService.service.getVM();
-    logReader.appPid = vm.pid;
+    await logReader.provideVmService(_vmService);
   }
 
   @override

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

@@ -1300,16 +1300,12 @@ class IOSDeviceLogReader extends DeviceLogReader {
   @override
   Stream<String> get logLines => linesController.stream;
 
-  @override
-  FlutterVmService? get connectedVMService => _connectedVMService;
-  FlutterVmService? _connectedVMService;
+  FlutterVmService? _connectedVmService;
 
   @override
-  set connectedVMService(FlutterVmService? connectedVmService) {
-    if (connectedVmService != null) {
-      _listenToUnifiedLoggingEvents(connectedVmService);
-    }
-    _connectedVMService = connectedVmService;
+  Future<void> provideVmService(FlutterVmService connectedVmService) async {
+    await _listenToUnifiedLoggingEvents(connectedVmService);
+    _connectedVmService = connectedVmService;
   }
 
   static const int minimumUniversalLoggingSdkVersion = 13;
@@ -1360,7 +1356,7 @@ class IOSDeviceLogReader extends DeviceLogReader {
     // CoreDevice and has iOS 13 or greater.
     // When using `ios-deploy` and the Dart VM, prefer the more complete logs
     // from the attached debugger, if available.
-    if (connectedVMService != null && (_iosDeployDebugger == null || !_iosDeployDebugger!.debuggerAttached)) {
+    if (_connectedVmService != null && (_iosDeployDebugger == null || !_iosDeployDebugger!.debuggerAttached)) {
       return _IOSDeviceLogSources(
         primarySource: IOSDeviceLogSource.unifiedLogging,
         fallbackSource: IOSDeviceLogSource.iosDeploy,

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

@@ -26,6 +26,7 @@ import '../globals.dart' as globals;
 import '../macos/xcode.dart';
 import '../project.dart';
 import '../protocol_discovery.dart';
+import '../vmservice.dart';
 import 'application_package.dart';
 import 'mac.dart';
 import 'plist_parser.dart';
@@ -1022,6 +1023,9 @@ class _IOSSimulatorLogReader extends DeviceLogReader {
   void dispose() {
     _stop();
   }
+
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async { }
 }
 
 class _IOSSimulatorDevicePortForwarder extends DevicePortForwarder {

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

@@ -20,6 +20,7 @@ import '../device_port_forwarder.dart';
 import '../device_vm_service_discovery_for_attach.dart';
 import '../project.dart';
 import '../resident_runner.dart';
+import '../vmservice.dart';
 import 'debounce_data_stream.dart';
 import 'file_transfer.dart';
 
@@ -517,6 +518,9 @@ class _ProxiedLogReader extends DeviceLogReader {
       });
     }
   }
+
+  @override
+  Future<void> provideVmService(FlutterVmService connectedVmService) async { }
 }
 
 /// A port forwarded by a [ProxiedPortForwarder].

packages/flutter_tools/lib/src/resident_runner.dart

@@ -8,7 +8,6 @@ import 'package:meta/meta.dart';
 import 'package:package_config/package_config.dart';
 import 'package:vm_service/vm_service.dart' as vm_service;
 
-import 'android/android_device.dart';
 import 'application_package.dart';
 import 'artifacts.dart';
 import 'asset.dart';
@@ -346,7 +345,8 @@ class FlutterDevice {
       globals.printTrace('Successfully connected to service protocol: $vmServiceUri');
 
       vmService = service;
-      (await device!.getLogReader(app: package)).connectedVMService = vmService;
+      await (await device!.getLogReader(app: package))
+          .provideVmService(vmService!);
       completer.complete();
       await subscription.cancel();
     }, onError: (dynamic error) {
@@ -417,24 +417,6 @@ class FlutterDevice {
     _loggingSubscription = null;
   }
 
-  /// Attempts to set up reading logs from the Flutter app on the device.
-  ///
-  /// This can fail if the device if no longer connected.
-  Future<void> tryInitLogReader() async {
-    final vm_service.VM? vm = await vmService!.getVmGuarded();
-    final DeviceLogReader logReader = await device!.getLogReader(app: package);
-    if (vm == null && logReader is AdbLogReader) {
-      // TODO(andrewkolos): This is a temporary, hacky fix for
-      //  https://github.com/flutter/flutter/issues/155795 that emphasizes
-      //  simplicity for the sake of being suitable for cherry-picking.
-      globals.printError(
-        'Unable to initiate adb log filtering for device'
-        '${device?.name}. Logs from the device may be more verbose than usual.',
-      );
-    }
-    logReader.appPid = vm?.pid;
-  }
-
   Future<int> runHot({
     required HotRunner hotRunner,
     String? route,

packages/flutter_tools/lib/src/run_cold.dart

@@ -104,7 +104,6 @@ class ColdRunner extends ResidentRunner {
       if (device!.vmService == null) {
         continue;
       }
-      await device.tryInitLogReader();
       globals.printTrace('Connected to ${device.device!.name}');
     }
 
@@ -153,9 +152,6 @@ class ColdRunner extends ResidentRunner {
       return 2;
     }
 
-    for (final FlutterDevice? device in flutterDevices) {
-      await device!.tryInitLogReader();
-    }
     for (final FlutterDevice? device in flutterDevices) {
       final List<FlutterView> views = await device!.vmService!.getFlutterViews();
       for (final FlutterView view in views) {