Make desktop stopApp only apply to processes Flutter started (#41519)

packages/flutter_tools/lib/src/desktop.dart

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-import 'dart:async';
-
-import 'base/io.dart';
-import 'base/process_manager.dart';
-import 'convert.dart';
-import 'device.dart';
-
-/// Kills a process on linux or macOS.
-Future<bool> killProcess(String executable) async {
-  if (executable == null) {
-    return false;
-  }
-  final RegExp whitespace = RegExp(r'\s+');
-  bool succeeded = true;
-  try {
-    final ProcessResult result = await processManager.run(<String>[
-      'ps', 'aux',
-    ]);
-    if (result.exitCode != 0) {
-      return false;
-    }
-    final List<String> lines = result.stdout.split('\n');
-    for (String line in lines) {
-      if (!line.contains(executable)) {
-        continue;
-      }
-      final List<String> values = line.split(whitespace);
-      if (values.length < 2) {
-        continue;
-      }
-      final String processPid = values[1];
-      final String currentRunningProcessPid = pid.toString();
-      // Don't kill the flutter tool process
-      if (processPid == currentRunningProcessPid) {
-        continue;
-      }
-
-      final ProcessResult killResult = await processManager.run(<String>[
-        'kill', processPid,
-      ]);
-      succeeded &= killResult.exitCode == 0;
-    }
-    return true;
-  } on ArgumentError {
-    succeeded = false;
-  }
-  return succeeded;
-}
-
-class DesktopLogReader extends DeviceLogReader {
-  final StreamController<List<int>> _inputController = StreamController<List<int>>.broadcast();
-
-  void initializeProcess(Process process) {
-    process.stdout.listen(_inputController.add);
-    process.stderr.listen(_inputController.add);
-    process.exitCode.then((int result) {
-      _inputController.close();
-    });
-  }
-
-  @override
-  Stream<String> get logLines {
-    return _inputController.stream
-      .transform(utf8.decoder)
-      .transform(const LineSplitter());
-  }
-
-  @override
-  String get name => 'desktop';
-}

packages/flutter_tools/lib/src/desktop_device.dart

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+
+import 'package:meta/meta.dart';
+
+import 'application_package.dart';
+import 'base/common.dart';
+import 'base/io.dart';
+import 'base/os.dart';
+import 'base/process_manager.dart';
+import 'build_info.dart';
+import 'cache.dart';
+import 'convert.dart';
+import 'device.dart';
+import 'globals.dart';
+import 'protocol_discovery.dart';
+
+/// A partial implementation of Device for desktop-class devices to inherit
+/// from, containing implementations that are common to all desktop devices.
+abstract class DesktopDevice extends Device {
+  DesktopDevice(String identifier, {@required PlatformType platformType, @required bool ephemeral}) : super(
+      identifier,
+      category: Category.desktop,
+      platformType: platformType,
+      ephemeral: ephemeral,
+  );
+
+  final Set<Process> _runningProcesses = <Process>{};
+
+  final DesktopLogReader _deviceLogReader = DesktopLogReader();
+
+  // Since the host and target devices are the same, no work needs to be done
+  // to install the application.
+  @override
+  Future<bool> isAppInstalled(ApplicationPackage app) async => true;
+
+  // Since the host and target devices are the same, no work needs to be done
+  // to install the application.
+  @override
+  Future<bool> isLatestBuildInstalled(ApplicationPackage app) async => true;
+
+  // Since the host and target devices are the same, no work needs to be done
+  // to install the application.
+  @override
+  Future<bool> installApp(ApplicationPackage app) async => true;
+
+  // Since the host and target devices are the same, no work needs to be done
+  // to uninstall the application.
+  @override
+  Future<bool> uninstallApp(ApplicationPackage app) async => true;
+
+  @override
+  Future<bool> get isLocalEmulator async => false;
+
+  @override
+  Future<String> get emulatorId async => null;
+
+  @override
+  DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder();
+
+  @override
+  Future<String> get sdkNameAndVersion async => os.name;
+
+  @override
+  DeviceLogReader getLogReader({ ApplicationPackage app }) {
+    return _deviceLogReader;
+  }
+
+  @override
+  void clearLogs() {}
+
+  @override
+  Future<LaunchResult> startApp(
+    ApplicationPackage package, {
+    String mainPath,
+    String route,
+    DebuggingOptions debuggingOptions,
+    Map<String, dynamic> platformArgs,
+    bool prebuiltApplication = false,
+    bool ipv6 = false,
+  }) async {
+    if (!prebuiltApplication) {
+      Cache.releaseLockEarly();
+      await buildForDevice(
+        package,
+        buildInfo: debuggingOptions?.buildInfo,
+        mainPath: mainPath,
+      );
+    }
+
+    // Ensure that the executable is locatable.
+    final BuildMode buildMode = debuggingOptions?.buildInfo?.mode;
+    final String executable = executablePathForDevice(package, buildMode);
+    if (executable == null) {
+      printError('Unable to find executable to run');
+      return LaunchResult.failed();
+    }
+
+    final Process process = await processManager.start(<String>[
+      executable,
+    ]);
+    _runningProcesses.add(process);
+    unawaited(process.exitCode.then((_) => _runningProcesses.remove(process)));
+
+    if (debuggingOptions?.buildInfo?.isRelease == true) {
+      return LaunchResult.succeeded();
+    }
+    _deviceLogReader.initializeProcess(process);
+    final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader);
+    try {
+      final Uri observatoryUri = await observatoryDiscovery.uri;
+      onAttached(package, buildMode, process);
+      return LaunchResult.succeeded(observatoryUri: observatoryUri);
+    } catch (error) {
+      printError('Error waiting for a debug connection: $error');
+      return LaunchResult.failed();
+    } finally {
+      await observatoryDiscovery.cancel();
+    }
+  }
+
+  @override
+  Future<bool> stopApp(ApplicationPackage app) async {
+    bool succeeded = true;
+    // Walk a copy of _runningProcesses, since the exit handler removes from the
+    // set.
+    for (Process process in Set<Process>.from(_runningProcesses)) {
+      succeeded &= process.kill();
+    }
+    return succeeded;
+  }
+
+  /// Builds the current project for this device, with the given options.
+  Future<void> buildForDevice(
+    ApplicationPackage package, {
+    String mainPath,
+    BuildInfo buildInfo,
+  });
+
+  /// Returns the path to the executable to run for [package] on this device for
+  /// the given [buildMode].
+  String executablePathForDevice(ApplicationPackage package, BuildMode buildMode);
+
+  /// Called after a process is attached, allowing any device-specific extra
+  /// steps to be run.
+  void onAttached(ApplicationPackage package, BuildMode buildMode, Process process) {}
+}
+
+class DesktopLogReader extends DeviceLogReader {
+  final StreamController<List<int>> _inputController = StreamController<List<int>>.broadcast();
+
+  void initializeProcess(Process process) {
+    process.stdout.listen(_inputController.add);
+    process.stderr.listen(_inputController.add);
+    process.exitCode.then((int result) {
+      _inputController.close();
+    });
+  }
+
+  @override
+  Stream<String> get logLines {
+    return _inputController.stream
+      .transform(utf8.decoder)
+      .transform(const LineSplitter());
+  }
+
+  @override
+  String get name => 'desktop';
+}

packages/flutter_tools/lib/src/linux/linux_device.dart

@@ -2,60 +2,23 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import '../application_package.dart';
-import '../base/io.dart';
-import '../base/os.dart';
 import '../base/platform.dart';
-import '../base/process_manager.dart';
 import '../build_info.dart';
-import '../desktop.dart';
+import '../desktop_device.dart';
 import '../device.dart';
-import '../globals.dart';
 import '../project.dart';
-import '../protocol_discovery.dart';
 import 'application_package.dart';
 import 'build_linux.dart';
 import 'linux_workflow.dart';
 
 /// A device that represents a desktop Linux target.
-class LinuxDevice extends Device {
+class LinuxDevice extends DesktopDevice {
   LinuxDevice() : super(
       'Linux',
-      category: Category.desktop,
       platformType: PlatformType.linux,
       ephemeral: false,
   );
 
-  @override
-  void clearLogs() { }
-
-  @override
-  DeviceLogReader getLogReader({ ApplicationPackage app }) {
-    return _logReader;
-  }
-  final DesktopLogReader _logReader = DesktopLogReader();
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> installApp(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isAppInstalled(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isLatestBuildInstalled(ApplicationPackage app) async => true;
-
-  @override
-  Future<bool> get isLocalEmulator async => false;
-
-  @override
-  Future<String> get emulatorId async => null;
-
   @override
   bool isSupported() => true;
 
@@ -63,68 +26,30 @@ class LinuxDevice extends Device {
   String get name => 'Linux';
 
   @override
-  DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder();
+  Future<TargetPlatform> get targetPlatform async => TargetPlatform.linux_x64;
 
   @override
-  Future<String> get sdkNameAndVersion async => os.name;
+  bool isSupportedForProject(FlutterProject flutterProject) {
+    return flutterProject.linux.existsSync();
+  }
 
   @override
-  Future<LaunchResult> startApp(
+  Future<void> buildForDevice(
     covariant LinuxApp package, {
     String mainPath,
-    String route,
-    DebuggingOptions debuggingOptions,
-    Map<String, dynamic> platformArgs,
-    bool prebuiltApplication = false,
-    bool ipv6 = false,
+    BuildInfo buildInfo,
   }) async {
-    _lastBuiltMode = debuggingOptions.buildInfo.mode;
-    if (!prebuiltApplication) {
-      await buildLinux(
-        FlutterProject.current().linux,
-        debuggingOptions.buildInfo,
-        target: mainPath,
-      );
-    }
-    final Process process = await processManager.start(<String>[
-      package.executable(debuggingOptions?.buildInfo?.mode),
-    ]);
-    if (debuggingOptions?.buildInfo?.isRelease == true) {
-      return LaunchResult.succeeded();
-    }
-    _logReader.initializeProcess(process);
-    final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_logReader);
-    try {
-      final Uri observatoryUri = await observatoryDiscovery.uri;
-      return LaunchResult.succeeded(observatoryUri: observatoryUri);
-    } catch (error) {
-      printError('Error waiting for a debug connection: $error');
-      return LaunchResult.failed();
-    } finally {
-      await observatoryDiscovery.cancel();
-    }
-  }
-
-  @override
-  Future<bool> stopApp(covariant LinuxApp app) async {
-    return killProcess(app.executable(_lastBuiltMode));
+    await buildLinux(
+      FlutterProject.current().linux,
+      buildInfo,
+      target: mainPath,
+    );
   }
 
   @override
-  Future<TargetPlatform> get targetPlatform async => TargetPlatform.linux_x64;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to uninstall the application.
-  @override
-  Future<bool> uninstallApp(ApplicationPackage app) async => true;
-
-  @override
-  bool isSupportedForProject(FlutterProject flutterProject) {
-    return flutterProject.linux.existsSync();
+  String executablePathForDevice(covariant LinuxApp package, BuildMode buildMode) {
+    return package.executable(buildMode);
   }
-
-  // Track the last built mode from startApp.
-  BuildMode _lastBuiltMode;
 }
 
 class LinuxDevices extends PollingDeviceDiscovery {

packages/flutter_tools/lib/src/macos/macos_device.dart

@@ -2,61 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import '../application_package.dart';
 import '../base/io.dart';
-import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/process_manager.dart';
 import '../build_info.dart';
-import '../cache.dart';
-import '../desktop.dart';
+import '../desktop_device.dart';
 import '../device.dart';
-import '../globals.dart';
 import '../macos/application_package.dart';
 import '../project.dart';
-import '../protocol_discovery.dart';
 import 'build_macos.dart';
 import 'macos_workflow.dart';
 
 /// A device that represents a desktop MacOS target.
-class MacOSDevice extends Device {
+class MacOSDevice extends DesktopDevice {
   MacOSDevice() : super(
       'macOS',
-      category: Category.desktop,
       platformType: PlatformType.macos,
       ephemeral: false,
   );
 
-  @override
-  void clearLogs() { }
-
-  @override
-  DeviceLogReader getLogReader({ ApplicationPackage app }) {
-    return _deviceLogReader;
-  }
-  final DesktopLogReader _deviceLogReader = DesktopLogReader();
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> installApp(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isAppInstalled(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isLatestBuildInstalled(ApplicationPackage app) async => true;
-
-  @override
-  Future<bool> get isLocalEmulator async => false;
-
-  @override
-  Future<String> get emulatorId async => null;
-
   @override
   bool isSupported() => true;
 
@@ -64,83 +28,45 @@ class MacOSDevice extends Device {
   String get name => 'macOS';
 
   @override
-  DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder();
+  Future<TargetPlatform> get targetPlatform async => TargetPlatform.darwin_x64;
 
   @override
-  Future<String> get sdkNameAndVersion async => os.name;
+  bool isSupportedForProject(FlutterProject flutterProject) {
+    return flutterProject.macos.existsSync();
+  }
 
   @override
-  Future<LaunchResult> startApp(
+  Future<void> buildForDevice(
     covariant MacOSApp package, {
     String mainPath,
-    String route,
-    DebuggingOptions debuggingOptions,
-    Map<String, dynamic> platformArgs,
-    bool prebuiltApplication = false,
-    bool ipv6 = false,
+    BuildInfo buildInfo,
   }) async {
-    if (!prebuiltApplication) {
-      Cache.releaseLockEarly();
-      await buildMacOS(
-        flutterProject: FlutterProject.current(),
-        buildInfo: debuggingOptions?.buildInfo,
-        targetOverride: mainPath,
-      );
-    }
-
-    // Ensure that the executable is locatable.
-    final String executable = package.executable(debuggingOptions?.buildInfo?.mode);
-    if (executable == null) {
-      printError('Unable to find executable to run');
-      return LaunchResult.failed();
-    }
-
-    _lastBuiltMode = debuggingOptions?.buildInfo?.mode;
-    final Process process = await processManager.start(<String>[
-      executable,
-    ]);
-    if (debuggingOptions?.buildInfo?.isRelease == true) {
-      return LaunchResult.succeeded();
-    }
-    _deviceLogReader.initializeProcess(process);
-    final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_deviceLogReader);
-    try {
-      final Uri observatoryUri = await observatoryDiscovery.uri;
-      // Bring app to foreground.
-      await processManager.run(<String>[
-        'open', package.applicationBundle(debuggingOptions?.buildInfo?.mode),
-      ]);
-      return LaunchResult.succeeded(observatoryUri: observatoryUri);
-    } catch (error) {
-      printError('Error waiting for a debug connection: $error');
-      return LaunchResult.failed();
-    } finally {
-      await observatoryDiscovery.cancel();
-    }
+    await buildMacOS(
+      flutterProject: FlutterProject.current(),
+      buildInfo: buildInfo,
+      targetOverride: mainPath,
+    );
   }
 
-  // TODO(jonahwilliams): implement using process manager.
-  // currently we rely on killing the isolate taking down the application.
   @override
-  Future<bool> stopApp(covariant MacOSApp app) async {
-    return killProcess(app.executable(_lastBuiltMode));
+  String executablePathForDevice(covariant MacOSApp package, BuildMode buildMode) {
+    return package.executable(buildMode);
   }
 
   @override
-  Future<TargetPlatform> get targetPlatform async => TargetPlatform.darwin_x64;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to uninstall the application.
-  @override
-  Future<bool> uninstallApp(ApplicationPackage app) async => true;
-
-  @override
-  bool isSupportedForProject(FlutterProject flutterProject) {
-    return flutterProject.macos.existsSync();
+  void onAttached(covariant MacOSApp package, BuildMode buildMode, Process process) {
+    // Bring app to foreground. Ideally this would be done post-launch rather
+    // than post-attach, since this won't run for release builds, but there's
+    // no general-purpose way of knowing when a process is far enoug along in
+    // the launch process for 'open' to foreground it.
+    processManager.run(<String>[
+      'open', package.applicationBundle(buildMode),
+    ]).then((ProcessResult result) {
+      if (result.exitCode != 0) {
+        print('Failed to foreground app; open returned ${result.exitCode}');
+      }
+    });
   }
-
-  // Track the last built mode from startApp.
-  BuildMode _lastBuiltMode;
 }
 
 class MacOSDevices extends PollingDeviceDiscovery {

packages/flutter_tools/lib/src/windows/windows_device.dart

@@ -4,60 +4,25 @@
 
 import 'package:meta/meta.dart';
 
-import '../application_package.dart';
 import '../base/io.dart';
-import '../base/os.dart';
 import '../base/platform.dart';
 import '../base/process.dart';
 import '../build_info.dart';
-import '../desktop.dart';
+import '../desktop_device.dart';
 import '../device.dart';
-import '../globals.dart';
 import '../project.dart';
-import '../protocol_discovery.dart';
 import 'application_package.dart';
 import 'build_windows.dart';
 import 'windows_workflow.dart';
 
 /// A device that represents a desktop Windows target.
-class WindowsDevice extends Device {
+class WindowsDevice extends DesktopDevice {
   WindowsDevice() : super(
       'Windows',
-      category: Category.desktop,
       platformType: PlatformType.windows,
       ephemeral: false,
   );
 
-  @override
-  void clearLogs() { }
-
-  @override
-  DeviceLogReader getLogReader({ ApplicationPackage app }) {
-    return _logReader;
-  }
-  final DesktopLogReader _logReader = DesktopLogReader();
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> installApp(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isAppInstalled(ApplicationPackage app) async => true;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to install the application.
-  @override
-  Future<bool> isLatestBuildInstalled(ApplicationPackage app) async => true;
-
-  @override
-  Future<bool> get isLocalEmulator async => false;
-
-  @override
-  Future<String> get emulatorId async => null;
-
   @override
   bool isSupported() => true;
 
@@ -65,71 +30,29 @@ class WindowsDevice extends Device {
   String get name => 'Windows';
 
   @override
-  DevicePortForwarder get portForwarder => const NoOpDevicePortForwarder();
+  Future<TargetPlatform> get targetPlatform async => TargetPlatform.windows_x64;
 
   @override
-  Future<String> get sdkNameAndVersion async => os.name;
+  bool isSupportedForProject(FlutterProject flutterProject) {
+    return flutterProject.windows.existsSync();
+  }
 
   @override
-  Future<LaunchResult> startApp(
+  Future<void> buildForDevice(
     covariant WindowsApp package, {
     String mainPath,
-    String route,
-    DebuggingOptions debuggingOptions,
-    Map<String, dynamic> platformArgs,
-    bool prebuiltApplication = false,
-    bool ipv6 = false,
+    BuildInfo buildInfo,
   }) async {
-    if (!prebuiltApplication) {
-      await buildWindows(
-        FlutterProject.current().windows,
-        debuggingOptions.buildInfo,
-        target: mainPath,
-      );
-    }
-    final Process process = await processUtils.start(<String>[
-      package.executable(debuggingOptions?.buildInfo?.mode),
-    ]);
-    if (debuggingOptions?.buildInfo?.isRelease == true) {
-      return LaunchResult.succeeded();
-    }
-    _logReader.initializeProcess(process);
-    final ProtocolDiscovery observatoryDiscovery = ProtocolDiscovery.observatory(_logReader);
-    try {
-      final Uri observatoryUri = await observatoryDiscovery.uri;
-      return LaunchResult.succeeded(observatoryUri: observatoryUri);
-    } catch (error) {
-      printError('Error waiting for a debug connection: $error');
-      return LaunchResult.failed();
-    } finally {
-      await observatoryDiscovery.cancel();
-    }
-  }
-
-  @override
-  Future<bool> stopApp(covariant WindowsApp app) async {
-    // Assume debug for now.
-    final List<String> process = runningProcess(app.executable(BuildMode.debug));
-    if (process == null) {
-      return false;
-    }
-    final RunResult result = await processUtils.run(
-      <String>['Taskkill', '/PID', process.first, '/F'],
+    await buildWindows(
+      FlutterProject.current().windows,
+      buildInfo,
+      target: mainPath,
     );
-    return result.exitCode == 0;
   }
 
   @override
-  Future<TargetPlatform> get targetPlatform async => TargetPlatform.windows_x64;
-
-  // Since the host and target devices are the same, no work needs to be done
-  // to uninstall the application.
-  @override
-  Future<bool> uninstallApp(ApplicationPackage app) async => true;
-
-  @override
-  bool isSupportedForProject(FlutterProject flutterProject) {
-    return flutterProject.windows.existsSync();
+  String executablePathForDevice(covariant WindowsApp package, BuildMode buildMode) {
+    return package.executable(buildMode);
   }
 }
 

packages/flutter_tools/test/general.shard/desktop_device_test.dart

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:convert';
+
+import 'package:flutter_tools/src/base/context.dart';
+import 'package:flutter_tools/src/base/logger.dart';
+import 'package:flutter_tools/src/project.dart';
+import 'package:mockito/mockito.dart';
+import 'package:process/process.dart';
+
+import 'package:flutter_tools/src/application_package.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/io.dart';
+import 'package:flutter_tools/src/base/os.dart';
+import 'package:flutter_tools/src/build_info.dart';
+import 'package:flutter_tools/src/desktop_device.dart';
+import 'package:flutter_tools/src/device.dart';
+
+import '../src/common.dart';
+import '../src/context.dart';
+import '../src/mocks.dart';
+
+/// A trivial subclass of DesktopDevice for testing the shared functionality.
+class FakeDesktopDevice extends DesktopDevice {
+  FakeDesktopDevice() : super(
+      'dummy',
+      platformType: PlatformType.linux,
+      ephemeral: false,
+  );
+
+  /// The [mainPath] last passed to [buildForDevice].
+  String lastBuiltMainPath;
+
+  /// The [buildInfo] last passed to [buildForDevice].
+  BuildInfo lastBuildInfo;
+
+  @override
+  String get name => 'dummy';
+
+  @override
+  Future<TargetPlatform> get targetPlatform async => TargetPlatform.tester;
+
+  @override
+  bool isSupported() => true;
+
+  @override
+  bool isSupportedForProject(FlutterProject flutterProject) => true;
+
+  @override
+  Future<void> buildForDevice(
+    ApplicationPackage package, {
+    String mainPath,
+    BuildInfo buildInfo,
+  }) async {
+    lastBuiltMainPath = mainPath;
+    lastBuildInfo = buildInfo;
+  }
+
+  // Dummy implementation that just returns the build mode name.
+  @override
+  String executablePathForDevice(ApplicationPackage package, BuildMode buildMode) {
+    return buildMode == null ? 'null' : getNameForBuildMode(buildMode);
+  }
+}
+
+/// A desktop device that returns a null executable path, for failure testing.
+class NullExecutableDesktopDevice extends FakeDesktopDevice {
+  @override
+  String executablePathForDevice(ApplicationPackage package, BuildMode buildMode) {
+    return null;
+  }
+}
+
+class MockAppplicationPackage extends Mock implements ApplicationPackage {}
+
+class MockFileSystem extends Mock implements FileSystem {}
+
+class MockFile extends Mock implements File {}
+
+class MockProcessManager extends Mock implements ProcessManager {}
+
+void main() {
+  group('Basic info', () {
+    test('Category is desktop', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      expect(device.category, Category.desktop);
+    });
+
+    test('Not an emulator', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      expect(await device.isLocalEmulator, false);
+      expect(await device.emulatorId, null);
+    });
+
+    testUsingContext('Uses OS name as SDK name', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      expect(await device.sdkNameAndVersion, os.name);
+    });
+  });
+
+  group('Install', () {
+    test('Install checks always return true', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      expect(await device.isAppInstalled(null), true);
+      expect(await device.isLatestBuildInstalled(null), true);
+      expect(device.category, Category.desktop);
+    });
+
+    test('Install and uninstall are no-ops that report success', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      final MockAppplicationPackage package = MockAppplicationPackage();
+      expect(await device.uninstallApp(package), true);
+      expect(await device.isAppInstalled(package), true);
+      expect(await device.isLatestBuildInstalled(package), true);
+
+      expect(await device.installApp(package), true);
+      expect(await device.isAppInstalled(package), true);
+      expect(await device.isLatestBuildInstalled(package), true);
+      expect(device.category, Category.desktop);
+    });
+  });
+
+  group('Starting and stopping application', () {
+    final MockFileSystem mockFileSystem = MockFileSystem();
+    final MockProcessManager mockProcessManager = MockProcessManager();
+
+    // Configures mock environment so that startApp will be able to find and
+    // run an FakeDesktopDevice exectuable with for the given mode.
+    void setUpMockExecutable(FakeDesktopDevice device, BuildMode mode, {Future<int> exitFuture}) {
+      final String executableName = device.executablePathForDevice(null, mode);
+      final MockFile mockFile = MockFile();
+      when(mockFileSystem.file(executableName)).thenReturn(mockFile);
+      when(mockFile.existsSync()).thenReturn(true);
+      when(mockProcessManager.start(<String>[executableName])).thenAnswer((Invocation invocation) async {
+        return FakeProcess(
+          exitCode: Completer<int>().future,
+          stdout: Stream<List<int>>.fromIterable(<List<int>>[
+            utf8.encode('Observatory listening on http://127.0.0.1/0\n'),
+          ]),
+          stderr: const Stream<List<int>>.empty(),
+        );
+      });
+      when(mockProcessManager.run(any)).thenAnswer((Invocation invocation) async {
+        return ProcessResult(0, 1, '', '');
+      });
+    }
+
+    test('Stop without start is a successful no-op', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+    final MockAppplicationPackage package = MockAppplicationPackage();
+      expect(await device.stopApp(package), true);
+    });
+
+    testUsingContext('Can run from prebuilt application', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      final MockAppplicationPackage package = MockAppplicationPackage();
+      setUpMockExecutable(device, null);
+      final LaunchResult result = await device.startApp(package, prebuiltApplication: true);
+      expect(result.started, true);
+      expect(result.observatoryUri, Uri.parse('http://127.0.0.1/0'));
+    }, overrides: <Type, Generator>{
+      FileSystem: () => mockFileSystem,
+      ProcessManager: () => mockProcessManager,
+    });
+
+    testUsingContext('Null executable path fails gracefully', () async {
+      final NullExecutableDesktopDevice device = NullExecutableDesktopDevice();
+      final MockAppplicationPackage package = MockAppplicationPackage();
+      final LaunchResult result = await device.startApp(package, prebuiltApplication: true);
+      expect(result.started, false);
+      final BufferLogger logger = context.get<Logger>();
+      expect(logger.errorText, contains('Unable to find executable to run'));
+    });
+
+    testUsingContext('stopApp kills process started by startApp', () async {
+      final FakeDesktopDevice device = FakeDesktopDevice();
+      final MockAppplicationPackage package = MockAppplicationPackage();
+      setUpMockExecutable(device, null);
+      final LaunchResult result = await device.startApp(package, prebuiltApplication: true);
+      expect(result.started, true);
+      expect(await device.stopApp(package), true);
+    }, overrides: <Type, Generator>{
+      FileSystem: () => mockFileSystem,
+      ProcessManager: () => mockProcessManager,
+    });
+  });
+
+  test('Port forwarder is a no-op', () async {
+    final FakeDesktopDevice device = FakeDesktopDevice();
+    final DevicePortForwarder portForwarder = device.portForwarder;
+    final int result = await portForwarder.forward(2);
+    expect(result, 2);
+    expect(portForwarder.forwardedPorts.isEmpty, true);
+  });
+}

packages/flutter_tools/test/general.shard/linux/linux_device_test.dart

@@ -4,7 +4,6 @@
 
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
-import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/linux/application_package.dart';
@@ -12,7 +11,6 @@ import 'package:flutter_tools/src/linux/linux_device.dart';
 import 'package:flutter_tools/src/device.dart';
 import 'package:flutter_tools/src/project.dart';
 import 'package:mockito/mockito.dart';
-import 'package:process/process.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
@@ -21,22 +19,8 @@ void main() {
   group(LinuxDevice, () {
     final LinuxDevice device = LinuxDevice();
     final MockPlatform notLinux = MockPlatform();
-    final MockProcessManager mockProcessManager = MockProcessManager();
-    const String flutterToolCommand = 'flutter --someoption somevalue';
 
     when(notLinux.isLinux).thenReturn(false);
-    when(mockProcessManager.run(<String>[
-      'ps', 'aux',
-    ])).thenAnswer((Invocation invocation) async {
-      // The flutter tool process is returned as output to the ps aux command
-      final MockProcessResult result = MockProcessResult();
-      when(result.exitCode).thenReturn(0);
-      when<String>(result.stdout).thenReturn('username  $pid  $flutterToolCommand');
-      return result;
-    });
-    when(mockProcessManager.run(<String>[
-      'kill', '$pid',
-    ])).thenThrow(Exception('Flutter tool process has been killed'));
 
     testUsingContext('defaults', () async {
       final PrebuiltLinuxApp linuxApp = PrebuiltLinuxApp(executable: 'foo');
@@ -48,24 +32,6 @@ void main() {
       expect(await device.isAppInstalled(linuxApp), true);
       expect(await device.stopApp(linuxApp), true);
       expect(device.category, Category.desktop);
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
-    });
-
-    test('noop port forwarding', () async {
-      final LinuxDevice device = LinuxDevice();
-      final DevicePortForwarder portForwarder = device.portForwarder;
-      final int result = await portForwarder.forward(2);
-      expect(result, 2);
-      expect(portForwarder.forwardedPorts.isEmpty, true);
-    });
-
-    testUsingContext('The current running process is not killed when stopping the app', () async {
-      // The name of the executable is the same as a command line argument to the flutter tool
-      final PrebuiltLinuxApp linuxApp = PrebuiltLinuxApp(executable: 'somevalue');
-      expect(await device.stopApp(linuxApp), true);
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
     });
 
     testUsingContext('No devices listed if platform unsupported', () async {
@@ -95,16 +61,24 @@ void main() {
   }, overrides: <Type, Generator>{
     FileSystem: () => MemoryFileSystem(),
   });
+
+  testUsingContext('executablePathForDevice uses the correct package executable', () async {
+    final MockLinuxApp mockApp = MockLinuxApp();
+    const String debugPath = 'debug/executable';
+    const String profilePath = 'profile/executable';
+    const String releasePath = 'release/executable';
+    when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath);
+    when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath);
+    when(mockApp.executable(BuildMode.release)).thenReturn(releasePath);
+
+    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath);
+    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath);
+    expect(LinuxDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath);
+  }, overrides: <Type, Generator>{
+    FileSystem: () => MemoryFileSystem(),
+  });
 }
 
 class MockPlatform extends Mock implements Platform {}
 
-class MockFileSystem extends Mock implements FileSystem {}
-
-class MockFile extends Mock implements File {}
-
-class MockProcessManager extends Mock implements ProcessManager {}
-
-class MockProcess extends Mock implements Process {}
-
-class MockProcessResult extends Mock implements ProcessResult {}
+class MockLinuxApp extends Mock implements LinuxApp {}

packages/flutter_tools/test/general.shard/macos/macos_device_test.dart

@@ -2,9 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:async';
-import 'dart:convert';
-
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/project.dart';
@@ -21,7 +18,6 @@ import 'package:flutter_tools/src/macos/macos_device.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
-import '../../src/mocks.dart';
 
 void main() {
   group(MacOSDevice, () {
@@ -36,93 +32,13 @@ void main() {
 
     testUsingContext('defaults', () async {
       final MockMacOSApp mockMacOSApp = MockMacOSApp();
-      when(mockMacOSApp.executable(any)).thenReturn('foo');
       expect(await device.targetPlatform, TargetPlatform.darwin_x64);
       expect(device.name, 'macOS');
       expect(await device.installApp(mockMacOSApp), true);
       expect(await device.uninstallApp(mockMacOSApp), true);
       expect(await device.isLatestBuildInstalled(mockMacOSApp), true);
       expect(await device.isAppInstalled(mockMacOSApp), true);
-      expect(await device.stopApp(mockMacOSApp), false);
       expect(device.category, Category.desktop);
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
-    });
-
-    testUsingContext('stopApp', () async {
-      const String psOut = r'''
-tester    17193   0.0  0.2  4791128  37820   ??  S     2:27PM   0:00.09 /Applications/foo
-''';
-      final MockMacOSApp mockMacOSApp = MockMacOSApp();
-      when(mockMacOSApp.executable(any)).thenReturn('/Applications/foo');
-      when(mockProcessManager.run(<String>['ps', 'aux'])).thenAnswer((Invocation invocation) async {
-        return ProcessResult(1, 0, psOut, '');
-      });
-      when(mockProcessManager.run(<String>['kill', '17193'])).thenAnswer((Invocation invocation) async {
-        return ProcessResult(2, 0, '', '');
-      });
-      expect(await device.stopApp(mockMacOSApp), true);
-      verify(mockProcessManager.run(<String>['kill', '17193']));
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
-    });
-
-    group('startApp', () {
-      final MockMacOSApp macOSApp = MockMacOSApp();
-      final MockFileSystem mockFileSystem = MockFileSystem();
-      final MockProcessManager mockProcessManager = MockProcessManager();
-      final MockFile mockFile = MockFile();
-      when(macOSApp.executable(any)).thenReturn('test');
-      when(mockFileSystem.file('test')).thenReturn(mockFile);
-      when(mockFile.existsSync()).thenReturn(true);
-      when(mockProcessManager.start(<String>['test'])).thenAnswer((Invocation invocation) async {
-        return FakeProcess(
-          exitCode: Completer<int>().future,
-          stdout: Stream<List<int>>.fromIterable(<List<int>>[
-            utf8.encode('Observatory listening on http://127.0.0.1/0\n'),
-          ]),
-          stderr: const Stream<List<int>>.empty(),
-        );
-      });
-      when(mockProcessManager.run(any)).thenAnswer((Invocation invocation) async {
-        return ProcessResult(0, 1, '', '');
-      });
-
-      testUsingContext('Can run from prebuilt application', () async {
-        final LaunchResult result = await device.startApp(macOSApp, prebuiltApplication: true);
-        expect(result.started, true);
-        expect(result.observatoryUri, Uri.parse('http://127.0.0.1/0'));
-      }, overrides: <Type, Generator>{
-        FileSystem: () => mockFileSystem,
-        ProcessManager: () => mockProcessManager,
-      });
-
-      testUsingContext('The current running process is not killed when stopping the app', () async {
-        final String psOut = '''
-tester    $pid   0.0  0.2  4791128  37820   ??  S     2:27PM   0:00.09 flutter run --use-application-binary /Applications/foo
-''';
-        final MockMacOSApp mockMacOSApp = MockMacOSApp();
-        // The name of the executable is the same as a command line argument to the flutter tool
-        when(mockMacOSApp.executable(any)).thenReturn('/Applications/foo');
-        when(mockProcessManager.run(<String>['ps', 'aux'])).thenAnswer((Invocation invocation) async {
-          return ProcessResult(1, 0, psOut, '');
-        });
-        when(mockProcessManager.run(<String>[
-          'kill', '$pid',
-        ])).thenThrow(Exception('Flutter tool process has been killed'));
-
-        expect(await device.stopApp(mockMacOSApp), true);
-      }, overrides: <Type, Generator>{
-        ProcessManager: () => mockProcessManager,
-      });
-    });
-
-    test('noop port forwarding', () async {
-      final MacOSDevice device = MacOSDevice();
-      final DevicePortForwarder portForwarder = device.portForwarder;
-      final int result = await portForwarder.forward(2);
-      expect(result, 2);
-      expect(portForwarder.forwardedPorts.isEmpty, true);
     });
 
     testUsingContext('No devices listed if platform unsupported', () async {
@@ -151,6 +67,22 @@ tester    $pid   0.0  0.2  4791128  37820   ??  S     2:27PM   0:00.09 flutter r
     }, overrides: <Type, Generator>{
       FileSystem: () => MemoryFileSystem(),
     });
+
+    testUsingContext('executablePathForDevice uses the correct package executable', () async {
+      final MockMacOSApp mockApp = MockMacOSApp();
+      const String debugPath = 'debug/executable';
+      const String profilePath = 'profile/executable';
+      const String releasePath = 'release/executable';
+      when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath);
+      when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath);
+      when(mockApp.executable(BuildMode.release)).thenReturn(releasePath);
+
+      expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath);
+      expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath);
+      expect(MacOSDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath);
+    }, overrides: <Type, Generator>{
+      FileSystem: () => MemoryFileSystem(),
+    });
   });
 }
 
@@ -158,10 +90,4 @@ class MockPlatform extends Mock implements Platform {}
 
 class MockMacOSApp extends Mock implements MacOSApp {}
 
-class MockFileSystem extends Mock implements FileSystem {}
-
-class MockFile extends Mock implements File {}
-
 class MockProcessManager extends Mock implements ProcessManager {}
-
-class MockProcess extends Mock implements Process {}

packages/flutter_tools/test/general.shard/windows/windows_device_test.dart

@@ -4,7 +4,6 @@
 
 import 'package:file/memory.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
-import 'package:flutter_tools/src/base/io.dart';
 import 'package:flutter_tools/src/base/platform.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/project.dart';
@@ -12,7 +11,6 @@ import 'package:flutter_tools/src/windows/application_package.dart';
 import 'package:flutter_tools/src/windows/windows_device.dart';
 import 'package:flutter_tools/src/device.dart';
 import 'package:mockito/mockito.dart';
-import 'package:process/process.dart';
 
 import '../../src/common.dart';
 import '../../src/context.dart';
@@ -21,28 +19,9 @@ void main() {
   group(WindowsDevice, () {
     final WindowsDevice device = WindowsDevice();
     final MockPlatform notWindows = MockPlatform();
-    final MockProcessManager mockProcessManager = MockProcessManager();
-    const String flutterToolBinary = 'flutter.exe';
 
     when(notWindows.isWindows).thenReturn(false);
     when(notWindows.environment).thenReturn(const <String, String>{});
-    when(mockProcessManager.runSync(
-      <String>['powershell', '-script="Get-CimInstance Win32_Process"'],
-      workingDirectory: anyNamed('workingDirectory'),
-      environment: anyNamed('environment'),
-    )).thenAnswer((Invocation invocation) {
-      // The flutter tool process is returned as output to the powershell script
-      final MockProcessResult result = MockProcessResult();
-      when(result.exitCode).thenReturn(0);
-      when<String>(result.stdout).thenReturn('$pid  $flutterToolBinary');
-      when<String>(result.stderr).thenReturn('');
-      return result;
-    });
-    when(mockProcessManager.run(
-      <String>['Taskkill', '/PID', '$pid', '/F'],
-      workingDirectory: anyNamed('workingDirectory'),
-      environment: anyNamed('environment'),
-    )).thenThrow(Exception('Flutter tool process has been killed'));
 
     testUsingContext('defaults', () async {
       final PrebuiltWindowsApp windowsApp = PrebuiltWindowsApp(executable: 'foo');
@@ -52,18 +31,7 @@ void main() {
       expect(await device.uninstallApp(windowsApp), true);
       expect(await device.isLatestBuildInstalled(windowsApp), true);
       expect(await device.isAppInstalled(windowsApp), true);
-      expect(await device.stopApp(windowsApp), false);
       expect(device.category, Category.desktop);
-    }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
-    });
-
-    test('noop port forwarding', () async {
-      final WindowsDevice device = WindowsDevice();
-      final DevicePortForwarder portForwarder = device.portForwarder;
-      final int result = await portForwarder.forward(2);
-      expect(result, 2);
-      expect(portForwarder.forwardedPorts.isEmpty, true);
     });
 
     testUsingContext('No devices listed if platform unsupported', () async {
@@ -93,24 +61,24 @@ void main() {
       FileSystem: () => MemoryFileSystem(),
     });
 
-    testUsingContext('The current running process is not killed when stopping the app', () async {
-      // The name of the executable is the same as the flutter tool one
-      final PrebuiltWindowsApp windowsApp = PrebuiltWindowsApp(executable: flutterToolBinary);
-      expect(await device.stopApp(windowsApp), false);
+    testUsingContext('executablePathForDevice uses the correct package executable', () async {
+      final MockWindowsApp mockApp = MockWindowsApp();
+      const String debugPath = 'debug/executable';
+      const String profilePath = 'profile/executable';
+      const String releasePath = 'release/executable';
+      when(mockApp.executable(BuildMode.debug)).thenReturn(debugPath);
+      when(mockApp.executable(BuildMode.profile)).thenReturn(profilePath);
+      when(mockApp.executable(BuildMode.release)).thenReturn(releasePath);
+
+      expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.debug), debugPath);
+      expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.profile), profilePath);
+      expect(WindowsDevice().executablePathForDevice(mockApp, BuildMode.release), releasePath);
     }, overrides: <Type, Generator>{
-      ProcessManager: () => mockProcessManager,
+      FileSystem: () => MemoryFileSystem(),
     });
   });
 }
 
 class MockPlatform extends Mock implements Platform {}
 
-class MockFileSystem extends Mock implements FileSystem {}
-
-class MockFile extends Mock implements File {}
-
-class MockProcessManager extends Mock implements ProcessManager {}
-
-class MockProcess extends Mock implements Process {}
-
-class MockProcessResult extends Mock implements ProcessResult {}
+class MockWindowsApp extends Mock implements WindowsApp {}