[flutter_tools] Make ApplicationPackageFactory inject dependencies for Android Builds (#67827)

* [flutter_tools] simplication of application store logic

* maybe return null

* fix install tests

* Clean up imports

* revert windows test

* remove unused import

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

@@ -575,7 +575,8 @@ class AndroidDevice extends Device {
       );
       // Package has been built, so we can get the updated application ID and
       // activity name from the .apk.
-      package = await AndroidApk.fromAndroidProject(project.android);
+      package = await ApplicationPackageFactory.instance
+        .getPackageForPlatform(devicePlatform, buildInfo: debuggingOptions.buildInfo) as AndroidApk;
     }
     // There was a failure parsing the android project information.
     if (package == null) {

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

@@ -323,6 +323,7 @@ class AndroidSdk {
         sdkLevel: platformVersion,
         platformName: platformName,
         buildToolsVersion: buildToolsVersion,
+        fileSystem: globals.fs,
       );
     }).where((AndroidSdkVersion version) => version != null).toList();
 
@@ -436,19 +437,29 @@ class AndroidSdkVersion implements Comparable<AndroidSdkVersion> {
     @required this.sdkLevel,
     @required this.platformName,
     @required this.buildToolsVersion,
+    @required FileSystem fileSystem,
   }) : assert(sdkLevel != null),
        assert(platformName != null),
-       assert(buildToolsVersion != null);
+       assert(buildToolsVersion != null),
+       _fileSystem = fileSystem;
 
   final AndroidSdk sdk;
   final int sdkLevel;
   final String platformName;
   final Version buildToolsVersion;
 
+  final FileSystem _fileSystem;
+
   String get buildToolsVersionName => buildToolsVersion.toString();
 
   String get androidJarPath => getPlatformsPath('android.jar');
 
+  /// Return the path to the android application package tool.
+  ///
+  /// This is used to dump the xml in order to launch built android applications.
+  ///
+  /// See also:
+  ///   * [AndroidApk.fromApk], which depends on this to determine application identifiers.
   String get aaptPath => getBuildToolsPath('aapt');
 
   List<String> validateSdkWellFormed() {
@@ -464,11 +475,11 @@ class AndroidSdkVersion implements Comparable<AndroidSdkVersion> {
   }
 
   String getPlatformsPath(String itemName) {
-    return globals.fs.path.join(sdk.directory, 'platforms', platformName, itemName);
+    return _fileSystem.path.join(sdk.directory, 'platforms', platformName, itemName);
   }
 
   String getBuildToolsPath(String binaryName) {
-    return globals.fs.path.join(sdk.directory, 'build-tools', buildToolsVersionName, binaryName);
+    return _fileSystem.path.join(sdk.directory, 'build-tools', buildToolsVersionName, binaryName);
   }
 
   @override
@@ -478,7 +489,7 @@ class AndroidSdkVersion implements Comparable<AndroidSdkVersion> {
   String toString() => '[${sdk.directory}, SDK version $sdkLevel, build-tools $buildToolsVersionName]';
 
   String _exists(String path) {
-    if (!globals.fs.isFileSync(path)) {
+    if (!_fileSystem.isFileSync(path)) {
       return 'Android SDK file not found: $path.';
     }
     return null;

packages/flutter_tools/lib/src/application_package.dart

@@ -5,13 +5,16 @@
 import 'dart:collection';
 
 import 'package:meta/meta.dart';
+import 'package:process/process.dart';
 import 'package:xml/xml.dart';
 
+import 'android/android_sdk.dart';
 import 'android/gradle.dart';
 import 'base/common.dart';
 import 'base/context.dart';
 import 'base/file_system.dart';
 import 'base/io.dart';
+import 'base/logger.dart';
 import 'base/process.dart';
 import 'base/user_messages.dart';
 import 'build_info.dart';
@@ -26,8 +29,28 @@ import 'web/web_device.dart';
 import 'windows/application_package.dart';
 
 class ApplicationPackageFactory {
+  ApplicationPackageFactory({
+    @required AndroidSdk androidSdk,
+    @required ProcessManager processManager,
+    @required Logger logger,
+    @required UserMessages userMessages,
+    @required FileSystem fileSystem,
+  }) : _androidSdk = androidSdk,
+       _processManager = processManager,
+       _logger = logger,
+       _userMessages = userMessages,
+       _fileSystem = fileSystem,
+       _processUtils = ProcessUtils(logger: logger, processManager: processManager);
+
   static ApplicationPackageFactory get instance => context.get<ApplicationPackageFactory>();
 
+  final AndroidSdk _androidSdk;
+  final ProcessManager _processManager;
+  final Logger _logger;
+  final ProcessUtils _processUtils;
+  final UserMessages _userMessages;
+  final FileSystem _fileSystem;
+
   Future<ApplicationPackage> getPackageForPlatform(
     TargetPlatform platform, {
     BuildInfo buildInfo,
@@ -39,12 +62,27 @@ class ApplicationPackageFactory {
       case TargetPlatform.android_arm64:
       case TargetPlatform.android_x64:
       case TargetPlatform.android_x86:
-        if (globals.androidSdk?.licensesAvailable == true  && globals.androidSdk?.latestVersion == null) {
+        if (_androidSdk?.licensesAvailable == true && _androidSdk?.latestVersion == null) {
           await checkGradleDependencies();
         }
-        return applicationBinary == null
-            ? await AndroidApk.fromAndroidProject(FlutterProject.current().android)
-            : AndroidApk.fromApk(applicationBinary);
+        if (applicationBinary == null) {
+          return await AndroidApk.fromAndroidProject(
+            FlutterProject.current().android,
+            processManager: _processManager,
+            processUtils: _processUtils,
+            logger: _logger,
+            androidSdk: _androidSdk,
+            userMessages: _userMessages,
+            fileSystem: _fileSystem,
+          );
+        }
+        return AndroidApk.fromApk(
+          applicationBinary,
+          processManager: _processManager,
+          logger: _logger,
+          androidSdk: _androidSdk,
+          userMessages: _userMessages,
+        );
       case TargetPlatform.ios:
         return applicationBinary == null
             ? await IOSApp.fromIosProject(FlutterProject.current().ios, buildInfo)
@@ -96,6 +134,7 @@ abstract class ApplicationPackage {
   String toString() => displayName ?? id;
 }
 
+/// An application package created from an already built Android APK.
 class AndroidApk extends ApplicationPackage {
   AndroidApk({
     String id,
@@ -107,10 +146,17 @@ class AndroidApk extends ApplicationPackage {
        super(id: id);
 
   /// Creates a new AndroidApk from an existing APK.
-  factory AndroidApk.fromApk(File apk) {
-    final String aaptPath = globals.androidSdk?.latestVersion?.aaptPath;
-    if (aaptPath == null) {
-      globals.printError(userMessages.aaptNotFound);
+  ///
+  /// Returns `null` if the APK was invalid or any required tooling was missing.
+  factory AndroidApk.fromApk(File apk, {
+    @required AndroidSdk androidSdk,
+    @required ProcessManager processManager,
+    @required UserMessages userMessages,
+    @required Logger logger,
+  }) {
+    final String aaptPath = androidSdk?.latestVersion?.aaptPath;
+    if (aaptPath == null || !processManager.canRun(aaptPath)) {
+      logger.printError(userMessages.aaptNotFound);
       return null;
     }
 
@@ -131,15 +177,15 @@ class AndroidApk extends ApplicationPackage {
       return null;
     }
 
-    final ApkManifestData data = ApkManifestData.parseFromXmlDump(apptStdout);
+    final ApkManifestData data = ApkManifestData.parseFromXmlDump(apptStdout, logger);
 
     if (data == null) {
-      globals.printError('Unable to read manifest info from ${apk.path}.');
+      logger.printError('Unable to read manifest info from ${apk.path}.');
       return null;
     }
 
     if (data.packageName == null || data.launchableActivityName == null) {
-      globals.printError('Unable to read manifest info from ${apk.path}.');
+      logger.printError('Unable to read manifest info from ${apk.path}.');
       return null;
     }
 
@@ -161,7 +207,14 @@ class AndroidApk extends ApplicationPackage {
   final int versionCode;
 
   /// Creates a new AndroidApk based on the information in the Android manifest.
-  static Future<AndroidApk> fromAndroidProject(AndroidProject androidProject) async {
+  static Future<AndroidApk> fromAndroidProject(AndroidProject androidProject, {
+    @required AndroidSdk androidSdk,
+    @required ProcessManager processManager,
+    @required UserMessages userMessages,
+    @required ProcessUtils processUtils,
+    @required Logger logger,
+    @required FileSystem fileSystem,
+  }) async {
     File apkFile;
 
     if (androidProject.isUsingGradle) {
@@ -169,20 +222,26 @@ class AndroidApk extends ApplicationPackage {
       if (apkFile.existsSync()) {
         // Grab information from the .apk. The gradle build script might alter
         // the application Id, so we need to look at what was actually built.
-        return AndroidApk.fromApk(apkFile);
+        return AndroidApk.fromApk(
+          apkFile,
+          androidSdk: androidSdk,
+          processManager: processManager,
+          logger: logger,
+          userMessages: userMessages,
+        );
       }
       // The .apk hasn't been built yet, so we work with what we have. The run
       // command will grab a new AndroidApk after building, to get the updated
       // IDs.
     } else {
-      apkFile = globals.fs.file(globals.fs.path.join(getAndroidBuildDirectory(), 'app.apk'));
+      apkFile = fileSystem.file(fileSystem.path.join(getAndroidBuildDirectory(), 'app.apk'));
     }
 
     final File manifest = androidProject.appManifestFile;
 
     if (!manifest.existsSync()) {
-      globals.printError('AndroidManifest.xml could not be found.');
-      globals.printError('Please check ${manifest.path} for errors.');
+      logger.printError('AndroidManifest.xml could not be found.');
+      logger.printError('Please check ${manifest.path} for errors.');
       return null;
     }
 
@@ -193,19 +252,19 @@ class AndroidApk extends ApplicationPackage {
     } on XmlParserException catch (exception) {
       String manifestLocation;
       if (androidProject.isUsingGradle) {
-        manifestLocation = globals.fs.path.join(androidProject.hostAppGradleRoot.path, 'app', 'src', 'main', 'AndroidManifest.xml');
+        manifestLocation = fileSystem.path.join(androidProject.hostAppGradleRoot.path, 'app', 'src', 'main', 'AndroidManifest.xml');
       } else {
-        manifestLocation = globals.fs.path.join(androidProject.hostAppGradleRoot.path, 'AndroidManifest.xml');
+        manifestLocation = fileSystem.path.join(androidProject.hostAppGradleRoot.path, 'AndroidManifest.xml');
       }
-      globals.printError('AndroidManifest.xml is not a valid XML document.');
-      globals.printError('Please check $manifestLocation for errors.');
+      logger.printError('AndroidManifest.xml is not a valid XML document.');
+      logger.printError('Please check $manifestLocation for errors.');
       throwToolExit('XML Parser error message: ${exception.toString()}');
     }
 
     final Iterable<XmlElement> manifests = document.findElements('manifest');
     if (manifests.isEmpty) {
-      globals.printError('AndroidManifest.xml has no manifest element.');
-      globals.printError('Please check ${manifest.path} for errors.');
+      logger.printError('AndroidManifest.xml has no manifest element.');
+      logger.printError('Please check ${manifest.path} for errors.');
       return null;
     }
     final String packageId = manifests.first.getAttribute('package');
@@ -241,8 +300,8 @@ class AndroidApk extends ApplicationPackage {
     }
 
     if (packageId == null || launchActivity == null) {
-      globals.printError('package identifier or launch activity not found.');
-      globals.printError('Please check ${manifest.path} for errors.');
+      logger.printError('package identifier or launch activity not found.');
+      logger.printError('Please check ${manifest.path} for errors.');
       return null;
     }
 
@@ -495,7 +554,7 @@ class ApkManifestData {
     return false;
   }
 
-  static ApkManifestData parseFromXmlDump(String data) {
+  static ApkManifestData parseFromXmlDump(String data, Logger logger) {
     if (data == null || data.trim().isEmpty) {
       return null;
     }
@@ -572,7 +631,7 @@ class ApkManifestData {
     final String packageName = package.value.substring(1, package.value.indexOf('" '));
 
     if (launchActivity == null) {
-      globals.printError('Error running $packageName. Default activity not found');
+      logger.printError('Error running $packageName. Default activity not found');
       return null;
     }
 
@@ -584,16 +643,16 @@ class ApkManifestData {
     // Example format: (type 0x10)0x1
     final _Attribute versionCodeAttr = manifest.firstAttribute('android:versionCode');
     if (versionCodeAttr == null) {
-      globals.printError('Error running $packageName. Manifest versionCode not found');
+      logger.printError('Error running $packageName. Manifest versionCode not found');
       return null;
     }
     if (!versionCodeAttr.value.startsWith('(type 0x10)')) {
-      globals.printError('Error running $packageName. Manifest versionCode invalid');
+      logger.printError('Error running $packageName. Manifest versionCode invalid');
       return null;
     }
     final int versionCode = int.tryParse(versionCodeAttr.value.substring(11));
     if (versionCode == null) {
-      globals.printError('Error running $packageName. Manifest versionCode invalid');
+      logger.printError('Error running $packageName. Manifest versionCode invalid');
       return null;
     }
 

packages/flutter_tools/lib/src/context_runner.dart

@@ -88,7 +88,13 @@ Future<T> runInContext<T>(
         androidSdk: globals.androidSdk,
         featureFlags: featureFlags,
       ),
-      ApplicationPackageFactory: () => ApplicationPackageFactory(),
+      ApplicationPackageFactory: () => ApplicationPackageFactory(
+        userMessages: globals.userMessages,
+        processManager: globals.processManager,
+        logger: globals.logger,
+        fileSystem: globals.fs,
+        androidSdk: globals.androidSdk,
+      ),
       Artifacts: () => CachedArtifacts(
         fileSystem: globals.fs,
         cache: globals.cache,

packages/flutter_tools/test/commands.shard/hermetic/install_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:file/file.dart';
 import 'package:flutter_tools/src/application_package.dart';
+import 'package:flutter_tools/src/base/file_system.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/commands/install.dart';

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

@@ -10,8 +10,10 @@ import 'package:flutter_tools/src/android/android_sdk.dart';
 import 'package:flutter_tools/src/application_package.dart';
 import 'package:flutter_tools/src/base/context.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/base/logger.dart';
 import 'package:flutter_tools/src/base/os.dart';
 import 'package:flutter_tools/src/base/platform.dart';
+import 'package:flutter_tools/src/base/user_messages.dart';
 import 'package:flutter_tools/src/build_info.dart';
 import 'package:flutter_tools/src/cache.dart';
 import 'package:flutter_tools/src/fuchsia/application_package.dart';
@@ -130,67 +132,114 @@ void main() {
       expect(fakeProcessManager.hasRemainingExpectations, isFalse);
     }, overrides: overrides);
 
-    testUsingContext('returns null when failed to extract manifest', () async {
+    testWithoutContext('returns null when failed to extract manifest', () async {
       final AndroidSdkVersion sdkVersion = MockitoAndroidSdkVersion();
       when(sdk.latestVersion).thenReturn(sdkVersion);
+      final AndroidApk androidApk = AndroidApk.fromApk(
+        null,
+        processManager: fakeProcessManager,
+        logger: BufferLogger.test(),
+        userMessages: UserMessages(),
+        androidSdk: sdk,
+      );
 
-      expect(AndroidApk.fromApk(null), isNull);
+      expect(androidApk, isNull);
       expect(fakeProcessManager.hasRemainingExpectations, isFalse);
-    }, overrides: overrides);
+    });
   });
 
   group('ApkManifestData', () {
-    testUsingContext('Parses manifest with an Activity that has enabled set to true, action set to android.intent.action.MAIN and category set to android.intent.category.LAUNCHER', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithExplicitEnabledAndMainLauncherActivity);
+    testWithoutContext('Parses manifest with an Activity that has enabled set to true, action set to android.intent.action.MAIN and category set to android.intent.category.LAUNCHER', () {
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithExplicitEnabledAndMainLauncherActivity,
+        BufferLogger.test(),
+      );
+
       expect(data, isNotNull);
       expect(data.packageName, 'io.flutter.examples.hello_world');
       expect(data.launchableActivityName, 'io.flutter.examples.hello_world.MainActivity2');
-    }, overrides: noColorTerminalOverride);
+    });
+
+    testWithoutContext('Parses manifest with an Activity that has no value for its enabled field, action set to android.intent.action.MAIN and category set to android.intent.category.LAUNCHER', () {
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithDefaultEnabledAndMainLauncherActivity,
+        BufferLogger.test(),
+      );
 
-    testUsingContext('Parses manifest with an Activity that has no value for its enabled field, action set to android.intent.action.MAIN and category set to android.intent.category.LAUNCHER', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithDefaultEnabledAndMainLauncherActivity);
       expect(data, isNotNull);
       expect(data.packageName, 'io.flutter.examples.hello_world');
       expect(data.launchableActivityName, 'io.flutter.examples.hello_world.MainActivity2');
-    }, overrides: noColorTerminalOverride);
+    });
+
+    testWithoutContext('Parses manifest with a dist namespace', () {
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithDistNamespace,
+        BufferLogger.test(),
+      );
 
-    testUsingContext('Parses manifest with a dist namespace', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithDistNamespace);
       expect(data, isNotNull);
       expect(data.packageName, 'io.flutter.examples.hello_world');
       expect(data.launchableActivityName, 'io.flutter.examples.hello_world.MainActivity');
-    }, overrides: noColorTerminalOverride);
+    });
+
+    testWithoutContext('Error when parsing manifest with no Activity that has enabled set to true nor has no value for its enabled field', () {
+      final BufferLogger logger = BufferLogger.test();
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithNoEnabledActivity,
+        logger,
+      );
 
-    testUsingContext('Error when parsing manifest with no Activity that has enabled set to true nor has no value for its enabled field', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithNoEnabledActivity);
       expect(data, isNull);
       expect(
-          testLogger.errorText, 'Error running io.flutter.examples.hello_world. Default activity not found\n');
-    }, overrides: noColorTerminalOverride);
+        logger.errorText,
+        'Error running io.flutter.examples.hello_world. Default activity not found\n',
+      );
+    });
+
+    testWithoutContext('Error when parsing manifest with no Activity that has action set to android.intent.action.MAIN', () {
+      final BufferLogger logger = BufferLogger.test();
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithNoMainActivity,
+        logger,
+      );
 
-    testUsingContext('Error when parsing manifest with no Activity that has action set to android.intent.action.MAIN', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithNoMainActivity);
       expect(data, isNull);
       expect(
-          testLogger.errorText, 'Error running io.flutter.examples.hello_world. Default activity not found\n');
-    }, overrides: noColorTerminalOverride);
+        logger.errorText,
+        'Error running io.flutter.examples.hello_world. Default activity not found\n',
+      );
+    });
+
+    testWithoutContext('Error when parsing manifest with no Activity that has category set to android.intent.category.LAUNCHER', () {
+      final BufferLogger logger = BufferLogger.test();
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithNoLauncherActivity,
+        logger,
+      );
 
-    testUsingContext('Error when parsing manifest with no Activity that has category set to android.intent.category.LAUNCHER', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithNoLauncherActivity);
       expect(data, isNull);
       expect(
-          testLogger.errorText, 'Error running io.flutter.examples.hello_world. Default activity not found\n');
-    }, overrides: noColorTerminalOverride);
+        logger.errorText,
+        'Error running io.flutter.examples.hello_world. Default activity not found\n',
+      );
+    });
+
+    testWithoutContext('Parsing manifest with Activity that has multiple category, android.intent.category.LAUNCHER and android.intent.category.DEFAULT', () {
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithLauncherAndDefaultActivity,
+        BufferLogger.test(),
+      );
 
-    testUsingContext('Parsing manifest with Activity that has multiple category, android.intent.category.LAUNCHER and android.intent.category.DEFAULT', () {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithLauncherAndDefaultActivity);
       expect(data, isNotNull);
       expect(data.packageName, 'io.flutter.examples.hello_world');
       expect(data.launchableActivityName, 'io.flutter.examples.hello_world.MainActivity');
-    }, overrides: noColorTerminalOverride);
+    });
 
-    testUsingContext('Parses manifest with missing application tag', () async {
-      final ApkManifestData data = ApkManifestData.parseFromXmlDump(_aaptDataWithoutApplication);
+    testWithoutContext('Parses manifest with missing application tag', () async {
+      final ApkManifestData data = ApkManifestData.parseFromXmlDump(
+        _aaptDataWithoutApplication,
+        BufferLogger.test(),
+      );
 
       expect(data, isNull);
     });