Improve AOT snapshot input verification + cleanup (#17136)

Bugfix: Moves AOT snapshot input verification past where the last input
is added to the inputs list.

Cleanup:
* Extracts _isValidAotPlatform method.
* Moves non-platform-specific logic to the top.
* Moves variable declaration closer to first use, and inlines to a
  narrower scope where possible.

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

@@ -264,36 +264,16 @@ class Snapshotter {
     @required bool preferSharedLibrary,
     List<String> extraGenSnapshotOptions: const <String>[],
   }) async {
-    if (!(platform == TargetPlatform.android_arm ||
-          platform == TargetPlatform.android_arm64 ||
-          platform == TargetPlatform.ios)) {
+    if (!_isValidAotPlatform(platform)) {
       printError('${getNameForTargetPlatform(platform)} does not support AOT compilation.');
       return -2;
     }
 
-    final Directory outputDir = fs.directory(outputPath);
-    outputDir.createSync(recursive: true);
-
-    final String vmSnapshotData = fs.path.join(outputDir.path, 'vm_snapshot_data');
-    final String isolateSnapshotData = fs.path.join(outputDir.path, 'isolate_snapshot_data');
-    final String depfilePath = fs.path.join(outputDir.path, 'snapshot.d');
-    final String assembly = fs.path.join(outputDir.path, 'snapshot_assembly.S');
-    final String assemblyO = fs.path.join(outputDir.path, 'snapshot_assembly.o');
-    final String assemblySo = fs.path.join(outputDir.path, 'app.so');
-    final bool compileToSharedLibrary =
-        preferSharedLibrary && androidSdk.ndkCompiler != null;
-
+    final bool compileToSharedLibrary = preferSharedLibrary && androidSdk.ndkCompiler != null;
     if (preferSharedLibrary && !compileToSharedLibrary) {
-      printStatus(
-          'Could not find NDK compiler. Not building in shared library mode');
+      printStatus('Could not find NDK compiler. Not building in shared library mode.');
     }
 
-    final String vmEntryPoints = artifacts.getArtifactPath(Artifact.dartVmEntryPointsTxt, platform, buildMode);
-    assert(vmEntryPoints != null);
-
-    final String ioEntryPoints = artifacts.getArtifactPath(Artifact.dartIoEntriesTxt, platform, buildMode);
-    assert(ioEntryPoints != null);
-
     final PackageMap packageMap = new PackageMap(packagesPath);
     final String packageMapError = packageMap.checkValid();
     if (packageMapError != null) {
@@ -301,19 +281,19 @@ class Snapshotter {
       return -3;
     }
 
+    final Directory outputDir = fs.directory(outputPath);
+    outputDir.createSync(recursive: true);
+
     final String skyEnginePkg = _getPackagePath(packageMap, 'sky_engine');
     final String uiPath = fs.path.join(skyEnginePkg, 'lib', 'ui', 'ui.dart');
     final String vmServicePath = fs.path.join(skyEnginePkg, 'sdk_ext', 'vmservice_io.dart');
 
-    final List<String> inputPaths = <String>[vmEntryPoints, ioEntryPoints, uiPath, vmServicePath, mainPath];
+    final List<String> inputPaths = <String>[uiPath, vmServicePath, mainPath];
     final Set<String> outputPaths = new Set<String>();
 
-    final Iterable<String> missingInputs = inputPaths.where((String p) => !fs.isFileSync(p));
-    if (missingInputs.isNotEmpty) {
-      printError('Missing input files: $missingInputs');
-      return -4;
-    }
-
+    final String vmSnapshotData = fs.path.join(outputDir.path, 'vm_snapshot_data');
+    final String isolateSnapshotData = fs.path.join(outputDir.path, 'isolate_snapshot_data');
+    final String depfilePath = fs.path.join(outputDir.path, 'snapshot.d');
     final List<String> genSnapshotArgs = <String>[
       '--vm_snapshot_data=$vmSnapshotData',
       '--isolate_snapshot_data=$isolateSnapshotData',
@@ -321,42 +301,56 @@ class Snapshotter {
       '--url_mapping=dart:vmservice_io,$vmServicePath',
       '--dependencies=$depfilePath',
     ];
-
-    if ((extraGenSnapshotOptions != null) && extraGenSnapshotOptions.isNotEmpty) {
-      printTrace('Extra gen-snapshot options: $extraGenSnapshotOptions');
+    if (previewDart2) {
+      genSnapshotArgs.addAll(<String>[
+        '--reify-generic-functions',
+        '--strong',
+      ]);
+    }
+    if (buildMode != BuildMode.release) {
+      genSnapshotArgs.addAll(<String>[
+        '--no-checked',
+        '--conditional_directives',
+      ]);
+    }
+    if (extraGenSnapshotOptions != null && extraGenSnapshotOptions.isNotEmpty) {
+      printTrace('Extra gen_snapshot options: $extraGenSnapshotOptions');
       genSnapshotArgs.addAll(extraGenSnapshotOptions);
     }
 
     final bool interpreter = _isInterpreted(platform, buildMode);
     if (!interpreter) {
+      final String vmEntryPoints = artifacts.getArtifactPath(Artifact.dartVmEntryPointsTxt, platform, buildMode);
+      final String ioEntryPoints = artifacts.getArtifactPath(Artifact.dartIoEntriesTxt, platform, buildMode);
       genSnapshotArgs.add('--embedder_entry_points_manifest=$vmEntryPoints');
       genSnapshotArgs.add('--embedder_entry_points_manifest=$ioEntryPoints');
+      inputPaths..addAll(<String>[vmEntryPoints, ioEntryPoints]);
     }
 
-    // iOS symbols used to load snapshot data in the engine.
+    // iOS snapshot generated files, compiled object files.
     const String kVmSnapshotData = 'kDartVmSnapshotData';
     const String kIsolateSnapshotData = 'kDartIsolateSnapshotData';
-
-    // iOS snapshot generated files, compiled object files.
     final String kVmSnapshotDataO = fs.path.join(outputDir.path, '$kVmSnapshotData.o');
     final String kIsolateSnapshotDataO = fs.path.join(outputDir.path, '$kIsolateSnapshotData.o');
 
+    // Compile-to-assembly outputs.
+    final String assembly = fs.path.join(outputDir.path, 'snapshot_assembly.S');
+    final String assemblyO = fs.path.join(outputDir.path, 'snapshot_assembly.o');
+    final String assemblySo = fs.path.join(outputDir.path, 'app.so');
+
     switch (platform) {
       case TargetPlatform.android_arm:
       case TargetPlatform.android_arm64:
       case TargetPlatform.android_x64:
       case TargetPlatform.android_x86:
-        final String vmSnapshotInstructions = fs.path.join(outputDir.path, 'vm_snapshot_instr');
-        final String isolateSnapshotInstructions = fs.path.join(outputDir.path, 'isolate_snapshot_instr');
         if (compileToSharedLibrary) {
           outputPaths.add(assemblySo);
           genSnapshotArgs.add('--snapshot_kind=app-aot-assembly');
           genSnapshotArgs.add('--assembly=$assembly');
         } else {
-          outputPaths.addAll(<String>[
-            vmSnapshotData,
-            isolateSnapshotData,
-          ]);
+          final String vmSnapshotInstructions = fs.path.join(outputDir.path, 'vm_snapshot_instr');
+          final String isolateSnapshotInstructions = fs.path.join(outputDir.path, 'isolate_snapshot_instr');
+          outputPaths.addAll(<String>[vmSnapshotData, isolateSnapshotData]);
           genSnapshotArgs.addAll(<String>[
             '--snapshot_kind=app-aot-blobs',
             '--vm_snapshot_instructions=$vmSnapshotInstructions',
@@ -394,13 +388,14 @@ class Snapshotter {
         assert(false);
     }
 
-    if (buildMode != BuildMode.release) {
-      genSnapshotArgs.addAll(<String>[
-        '--no-checked',
-        '--conditional_directives',
-      ]);
+    // Verify that all required inputs exist.
+    final Iterable<String> missingInputs = inputPaths.where((String p) => !fs.isFileSync(p));
+    if (missingInputs.isNotEmpty) {
+      printError('Missing input files: $missingInputs from $inputPaths');
+      return -4;
     }
 
+    // If inputs and outputs have not changed since last run, skip the build.
     final String fingerprintPath = '$depfilePath.fingerprint';
     final SnapshotType snapshotType = new SnapshotType(platform, buildMode);
     if (!await _isBuildRequired(snapshotType, outputPaths, depfilePath, mainPath, fingerprintPath)) {
@@ -408,12 +403,6 @@ class Snapshotter {
       return 0;
     }
 
-    if (previewDart2) {
-      genSnapshotArgs.addAll(<String>[
-        '--reify-generic-functions',
-        '--strong',
-      ]);
-    }
     genSnapshotArgs.add(mainPath);
 
     final int genSnapshotExitCode = await genSnapshot.run(
@@ -436,12 +425,10 @@ class Snapshotter {
     if (platform == TargetPlatform.ios) {
       printStatus('Building App.framework...');
 
-      final String kVmSnapshotDataC = fs.path.join(outputDir.path, '$kVmSnapshotData.c');
-      final String kIsolateSnapshotDataC = fs.path.join(outputDir.path, '$kIsolateSnapshotData.c');
-
       const List<String> commonBuildOptions = const <String>['-arch', 'arm64', '-miphoneos-version-min=8.0'];
-
       if (interpreter) {
+        final String kVmSnapshotDataC = fs.path.join(outputDir.path, '$kVmSnapshotData.c');
+        final String kIsolateSnapshotDataC = fs.path.join(outputDir.path, '$kIsolateSnapshotData.c');
         await fs.file(vmSnapshotData).rename(fs.path.join(outputDir.path, kVmSnapshotData));
         await fs.file(isolateSnapshotData).rename(fs.path.join(outputDir.path, kIsolateSnapshotData));
 
@@ -497,6 +484,14 @@ class Snapshotter {
     return 0;
   }
 
+  bool _isValidAotPlatform(TargetPlatform platform) {
+    return const <TargetPlatform>[
+      TargetPlatform.android_arm,
+      TargetPlatform.android_arm64,
+      TargetPlatform.ios,
+    ].contains(platform);
+  }
+
   /// Returns true if the specified platform and build mode require running in interpreted mode.
   bool _isInterpreted(TargetPlatform platform, BuildMode buildMode) {
     return platform == TargetPlatform.ios && buildMode == BuildMode.debug;

packages/flutter_tools/test/base/build_test.dart

@@ -662,12 +662,12 @@ void main() {
         '--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
         '--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
         '--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
-        '--snapshot_kind=core',
-        'snapshot.dart',
-        '--no-checked',
-        '--conditional_directives',
         '--reify-generic-functions',
         '--strong',
+        '--no-checked',
+        '--conditional_directives',
+        '--snapshot_kind=core',
+        'snapshot.dart',
         'main.dill',
       ]);
     }, overrides: contextOverrides);
@@ -704,14 +704,14 @@ void main() {
         '--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
         '--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
         '--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
+        '--reify-generic-functions',
+        '--strong',
+        '--no-checked',
+        '--conditional_directives',
         '--embedder_entry_points_manifest=$kVmEntrypoints',
         '--embedder_entry_points_manifest=$kIoEntries',
         '--snapshot_kind=app-aot-assembly',
         '--assembly=${fs.path.join(outputPath, 'snapshot_assembly.S')}',
-        '--no-checked',
-        '--conditional_directives',
-        '--reify-generic-functions',
-        '--strong',
         'main.dill',
       ]);
     }, overrides: contextOverrides);
@@ -748,12 +748,12 @@ void main() {
         '--url_mapping=dart:ui,${fs.path.join(skyEnginePath, 'lib', 'ui', 'ui.dart')}',
         '--url_mapping=dart:vmservice_io,${fs.path.join(skyEnginePath, 'sdk_ext', 'vmservice_io.dart')}',
         '--dependencies=${fs.path.join(outputPath, 'snapshot.d')}',
+        '--reify-generic-functions',
+        '--strong',
         '--embedder_entry_points_manifest=$kVmEntrypoints',
         '--embedder_entry_points_manifest=$kIoEntries',
         '--snapshot_kind=app-aot-assembly',
         '--assembly=${fs.path.join(outputPath, 'snapshot_assembly.S')}',
-        '--reify-generic-functions',
-        '--strong',
         'main.dill',
       ]);
     }, overrides: contextOverrides);