Refactor DevFS for kernel code (#7529)

Refactor DevFS so that it's easier to add new types of content such as kernel code
* add tests for DevFS package scanning
* add tests for DevFS over VMService protocol
* which covers _DevFSHttpWriter and ServiceProtocolDevFSOperations
* replace AssetBundleEntry and DevFSEntry with DevFSContent
* refactor to cleanup common code and replace some fields with locals
* rework .package file generation refactor away DevFSOperations.writeSource
* only update .package file if it has changed
* only write/delete/evict assets that have been changed/removed

packages/flutter_tools/lib/src/asset.dart

@@ -12,45 +12,20 @@ import 'package:yaml/yaml.dart';
 import 'base/file_system.dart';
 import 'build_info.dart';
 import 'cache.dart';
+import 'devfs.dart';
 import 'dart/package_map.dart';
 import 'globals.dart';
 
-/// An entry in an asset bundle.
-class AssetBundleEntry {
-  /// An entry backed by a File.
-  AssetBundleEntry.fromFile(this.archivePath, this.file)
-      : _contents = null;
-
-  /// An entry backed by a String.
-  AssetBundleEntry.fromString(this.archivePath, this._contents)
-      : file = null;
-
-  /// The path within the bundle.
-  final String archivePath;
-
-  /// The payload.
-  List<int> contentsAsBytes() {
-    if (_contents != null) {
-      return UTF8.encode(_contents);
-    } else {
-      return file.readAsBytesSync();
-    }
-  }
-
-  bool get isStringEntry => _contents != null;
-  int get contentsLength => _contents.length;
-
-  final File file;
-  final String _contents;
-}
-
 /// A bundle of assets.
 class AssetBundle {
-  final Set<AssetBundleEntry> entries = new Set<AssetBundleEntry>();
+  final Map<String, DevFSContent> entries = <String, DevFSContent>{};
 
   static const String defaultManifestPath = 'flutter.yaml';
+  static const String _kAssetManifestJson = 'AssetManifest.json';
+  static const String _kFontManifestJson = 'FontManifest.json';
   static const String _kFontSetMaterial = 'material';
   static const String _kFontSetRoboto = 'roboto';
+  static const String _kLICENSE = 'LICENSE';
 
   bool _fixed = false;
   DateTime _lastBuildTimestamp;
@@ -73,8 +48,7 @@ class AssetBundle {
         continue;
       final String assetPath = path.join(projectRoot, asset);
       final String archivePath = asset;
-      entries.add(
-          new AssetBundleEntry.fromFile(archivePath, fs.file(assetPath)));
+      entries[archivePath] = new DevFSFileContent(fs.file(assetPath));
     }
   }
 
@@ -111,7 +85,7 @@ class AssetBundle {
     }
     if (manifest == null) {
       // No manifest file found for this application.
-      entries.add(new AssetBundleEntry.fromString('AssetManifest.json', '{}'));
+      entries[_kAssetManifestJson] = new DevFSStringContent('{}');
       return 0;
     }
     if (manifest != null) {
@@ -141,16 +115,11 @@ class AssetBundle {
         manifestDescriptor['uses-material-design'];
 
     for (_Asset asset in assetVariants.keys) {
-      AssetBundleEntry assetEntry = _createAssetEntry(asset);
-      if (assetEntry == null)
-        return 1;
-      entries.add(assetEntry);
-
+      assert(asset.assetFileExists);
+      entries[asset.assetEntry] = new DevFSFileContent(asset.assetFile);
       for (_Asset variant in assetVariants[asset]) {
-        AssetBundleEntry variantEntry = _createAssetEntry(variant);
-        if (variantEntry == null)
-          return 1;
-        entries.add(variantEntry);
+        assert(variant.assetFileExists);
+        entries[variant.assetEntry] = new DevFSFileContent(variant.assetFile);
       }
     }
 
@@ -161,29 +130,27 @@ class AssetBundle {
         materialAssets.addAll(_getMaterialAssets(_kFontSetRoboto));
     }
     for (_Asset asset in materialAssets) {
-      AssetBundleEntry assetEntry = _createAssetEntry(asset);
-      if (assetEntry == null)
-        return 1;
-      entries.add(assetEntry);
+      assert(asset.assetFileExists);
+      entries[asset.assetEntry] = new DevFSFileContent(asset.assetFile);
     }
 
-    entries.add(_createAssetManifest(assetVariants));
+    entries[_kAssetManifestJson] = _createAssetManifest(assetVariants);
 
-    AssetBundleEntry fontManifest =
+    DevFSContent fontManifest =
         _createFontManifest(manifestDescriptor, usesMaterialDesign, includeDefaultFonts, includeRobotoFonts);
     if (fontManifest != null)
-      entries.add(fontManifest);
+      entries[_kFontManifestJson] = fontManifest;
 
     // TODO(ianh): Only do the following line if we've changed packages or if our LICENSE file changed
-    entries.add(await _obtainLicenses(packageMap, assetBasePath, reportPackages: reportLicensedPackages));
+    entries[_kLICENSE] = await _obtainLicenses(packageMap, assetBasePath, reportPackages: reportLicensedPackages);
 
     return 0;
   }
 
   void dump() {
     printTrace('Dumping AssetBundle:');
-    for (AssetBundleEntry entry in entries) {
-      printTrace(entry.archivePath);
+    for (String archivePath in entries.keys.toList()..sort()) {
+      printTrace(archivePath);
     }
   }
 }
@@ -256,8 +223,8 @@ List<_Asset> _getMaterialAssets(String fontSet) {
 
 final String _licenseSeparator = '\n' + ('-' * 80) + '\n';
 
-/// Returns a AssetBundleEntry representing the license file.
-Future<AssetBundleEntry> _obtainLicenses(
+/// Returns a DevFSContent representing the license file.
+Future<DevFSContent> _obtainLicenses(
   PackageMap packageMap,
   String assetBase,
   { bool reportPackages }
@@ -323,17 +290,10 @@ Future<AssetBundleEntry> _obtainLicenses(
 
   final String combinedLicenses = combinedLicensesList.join(_licenseSeparator);
 
-  return new AssetBundleEntry.fromString('LICENSE', combinedLicenses);
-}
-
-
-/// Create a [AssetBundleEntry] from the given [_Asset]; the asset must exist.
-AssetBundleEntry _createAssetEntry(_Asset asset) {
-  assert(asset.assetFileExists);
-  return new AssetBundleEntry.fromFile(asset.assetEntry, asset.assetFile);
+  return new DevFSStringContent(combinedLicenses);
 }
 
-AssetBundleEntry _createAssetManifest(Map<_Asset, List<_Asset>> assetVariants) {
+DevFSContent _createAssetManifest(Map<_Asset, List<_Asset>> assetVariants) {
   Map<String, List<String>> json = <String, List<String>>{};
   for (_Asset main in assetVariants.keys) {
     List<String> variants = <String>[];
@@ -341,10 +301,10 @@ AssetBundleEntry _createAssetManifest(Map<_Asset, List<_Asset>> assetVariants) {
       variants.add(variant.relativePath);
     json[main.relativePath] = variants;
   }
-  return new AssetBundleEntry.fromString('AssetManifest.json', JSON.encode(json));
+  return new DevFSStringContent(JSON.encode(json));
 }
 
-AssetBundleEntry _createFontManifest(Map<String, dynamic> manifestDescriptor,
+DevFSContent _createFontManifest(Map<String, dynamic> manifestDescriptor,
                              bool usesMaterialDesign,
                              bool includeDefaultFonts,
                              bool includeRobotoFonts) {
@@ -358,7 +318,7 @@ AssetBundleEntry _createFontManifest(Map<String, dynamic> manifestDescriptor,
     fonts.addAll(manifestDescriptor['fonts']);
   if (fonts.isEmpty)
     return null;
-  return new AssetBundleEntry.fromString('FontManifest.json', JSON.encode(fonts));
+  return new DevFSStringContent(JSON.encode(fonts));
 }
 
 /// Given an assetBase location and a flutter.yaml manifest, return a map of

packages/flutter_tools/lib/src/flx.dart

@@ -11,6 +11,7 @@ import 'base/common.dart';
 import 'base/file_system.dart';
 import 'base/process.dart';
 import 'dart/package_map.dart';
+import 'devfs.dart';
 import 'build_info.dart';
 import 'globals.dart';
 import 'toolchain.dart';
@@ -156,12 +157,12 @@ Future<Null> assemble({
   zipBuilder.entries.addAll(assetBundle.entries);
 
   if (snapshotFile != null)
-    zipBuilder.addEntry(new AssetBundleEntry.fromFile(_kSnapshotKey, snapshotFile));
+    zipBuilder.entries[_kSnapshotKey] = new DevFSFileContent(snapshotFile);
 
   ensureDirectoryExists(outputPath);
 
   printTrace('Encoding zip file to $outputPath');
-  zipBuilder.createZip(fs.file(outputPath), fs.directory(workingDirPath));
+  await zipBuilder.createZip(fs.file(outputPath), fs.directory(workingDirPath));
 
   printTrace('Built $outputPath.');
 }

packages/flutter_tools/lib/src/hot.dart

@@ -273,7 +273,7 @@ class HotRunner extends ResidentRunner {
         return false;
     }
     Status devFSStatus = logger.startProgress('Syncing files to device...');
-    await _devFS.update(progressReporter: progressReporter,
+    int bytes = await _devFS.update(progressReporter: progressReporter,
                         bundle: assetBundle,
                         bundleDirty: rebuildBundle,
                         fileFilter: _dartDependencies);
@@ -282,18 +282,19 @@ class HotRunner extends ResidentRunner {
       // Clear the set after the sync so they are recomputed next time.
       _dartDependencies = null;
     }
-    printTrace('Synced ${getSizeAsMB(_devFS.bytes)}.');
+    printTrace('Synced ${getSizeAsMB(bytes)}.');
     return true;
   }
 
   Future<Null> _evictDirtyAssets() async {
-    if (_devFS.dirtyAssetEntries.length == 0)
+    if (_devFS.assetPathsToEvict.isEmpty)
       return;
     if (currentView.uiIsolate == null)
       throw 'Application isolate not found';
-    for (DevFSEntry entry in _devFS.dirtyAssetEntries) {
-      await currentView.uiIsolate.flutterEvictAsset(entry.assetPath);
+    for (String assetPath in _devFS.assetPathsToEvict) {
+      await currentView.uiIsolate.flutterEvictAsset(assetPath);
     }
+    _devFS.assetPathsToEvict.clear();
   }
 
   Future<Null> _cleanupDevFS() async {

packages/flutter_tools/lib/src/zip.dart

@@ -2,10 +2,12 @@
 // 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:archive/archive.dart';
 import 'package:path/path.dart' as path;
 
-import 'asset.dart';
+import 'devfs.dart';
 import 'base/file_system.dart';
 import 'base/process.dart';
 
@@ -20,27 +22,32 @@ abstract class ZipBuilder {
 
   ZipBuilder._();
 
-  List<AssetBundleEntry> entries = <AssetBundleEntry>[];
-
-  void addEntry(AssetBundleEntry entry) => entries.add(entry);
+  Map<String, DevFSContent> entries = <String, DevFSContent>{};
 
-  void createZip(File outFile, Directory zipBuildDir);
+  Future<Null> createZip(File outFile, Directory zipBuildDir);
 }
 
 class _ArchiveZipBuilder extends ZipBuilder {
   _ArchiveZipBuilder() : super._();
 
   @override
-  void createZip(File outFile, Directory zipBuildDir) {
+  Future<Null> createZip(File outFile, Directory zipBuildDir) async {
     Archive archive = new Archive();
 
-    for (AssetBundleEntry entry in entries) {
-      List<int> data = entry.contentsAsBytes();
-      archive.addFile(new ArchiveFile.noCompress(entry.archivePath, data.length, data));
-    }
+    final Completer<Null> finished = new Completer<Null>();
+    int count = entries.length;
+    entries.forEach((String archivePath, DevFSContent content) {
+      content.contentsAsBytes().then((List<int> data) {
+        archive.addFile(new ArchiveFile.noCompress(archivePath, data.length, data));
+        --count;
+        if (count == 0)
+          finished.complete();
+      });
+    });
+    await finished.future;
 
     List<int> zipData = new ZipEncoder().encode(archive);
-    outFile.writeAsBytesSync(zipData);
+    await outFile.writeAsBytes(zipData);
   }
 }
 
@@ -48,11 +55,11 @@ class _ZipToolBuilder extends ZipBuilder {
   _ZipToolBuilder() : super._();
 
   @override
-  void createZip(File outFile, Directory zipBuildDir) {
+  Future<Null> createZip(File outFile, Directory zipBuildDir) async {
     // If there are no assets, then create an empty zip file.
     if (entries.isEmpty) {
       List<int> zipData = new ZipEncoder().encode(new Archive());
-      outFile.writeAsBytesSync(zipData);
+      await outFile.writeAsBytes(zipData);
       return;
     }
 
@@ -63,23 +70,33 @@ class _ZipToolBuilder extends ZipBuilder {
       zipBuildDir.deleteSync(recursive: true);
     zipBuildDir.createSync(recursive: true);
 
-    for (AssetBundleEntry entry in entries) {
-      List<int> data = entry.contentsAsBytes();
-      File file = fs.file(path.join(zipBuildDir.path, entry.archivePath));
-      file.parent.createSync(recursive: true);
-      file.writeAsBytesSync(data);
-    }
-
-    if (_getCompressedNames().isNotEmpty) {
+    final Completer<Null> finished = new Completer<Null>();
+    int count = entries.length;
+    entries.forEach((String archivePath, DevFSContent content) {
+      content.contentsAsBytes().then((List<int> data) {
+        File file = fs.file(path.join(zipBuildDir.path, archivePath));
+        file.parent.createSync(recursive: true);
+        file.writeAsBytes(data).then((_) {
+          --count;
+          if (count == 0)
+            finished.complete();
+        });
+      });
+    });
+    await finished.future;
+
+    final Iterable<String> compressedNames = _getCompressedNames();
+    if (compressedNames.isNotEmpty) {
       runCheckedSync(
-        <String>['zip', '-q', outFile.absolute.path]..addAll(_getCompressedNames()),
+        <String>['zip', '-q', outFile.absolute.path]..addAll(compressedNames),
         workingDirectory: zipBuildDir.path
       );
     }
 
-    if (_getStoredNames().isNotEmpty) {
+    final Iterable<String> storedNames = _getStoredNames();
+    if (storedNames.isNotEmpty) {
       runCheckedSync(
-        <String>['zip', '-q', '-0', outFile.absolute.path]..addAll(_getStoredNames()),
+        <String>['zip', '-q', '-0', outFile.absolute.path]..addAll(storedNames),
         workingDirectory: zipBuildDir.path
       );
     }
@@ -87,21 +104,14 @@ class _ZipToolBuilder extends ZipBuilder {
 
   static const List<String> _kNoCompressFileExtensions = const <String>['.png', '.jpg'];
 
-  bool isAssetCompressed(AssetBundleEntry entry) {
+  bool isAssetCompressed(String archivePath) {
     return !_kNoCompressFileExtensions.any(
-        (String extension) => entry.archivePath.endsWith(extension)
+        (String extension) => archivePath.endsWith(extension)
     );
   }
 
-  Iterable<String> _getCompressedNames() {
-    return entries
-      .where(isAssetCompressed)
-      .map((AssetBundleEntry entry) => entry.archivePath);
-  }
+  Iterable<String> _getCompressedNames() => entries.keys.where(isAssetCompressed);
 
-  Iterable<String> _getStoredNames() {
-    return entries
-      .where((AssetBundleEntry entry) => !isAssetCompressed(entry))
-      .map((AssetBundleEntry entry) => entry.archivePath);
-  }
+  Iterable<String> _getStoredNames() => entries.keys
+      .where((String archivePath) => !isAssetCompressed(archivePath));
 }

packages/flutter_tools/test/asset_bundle_test.dart

@@ -5,6 +5,7 @@
 import 'dart:convert';
 import 'package:flutter_tools/src/asset.dart';
 import 'package:flutter_tools/src/base/file_system.dart';
+import 'package:flutter_tools/src/devfs.dart';
 import 'package:path/path.dart' as path;
 import 'package:test/test.dart';
 
@@ -37,29 +38,26 @@ void main()  {
       AssetBundle ab = new AssetBundle.fixed('', 'apple.txt');
       expect(ab.entries, isNotEmpty);
       expect(ab.entries.length, 1);
-      AssetBundleEntry entry = ab.entries.first;
-      expect(entry, isNotNull);
-      expect(entry.archivePath, 'apple.txt');
+      String archivePath = ab.entries.keys.first;
+      expect(archivePath, isNotNull);
+      expect(archivePath, 'apple.txt');
     });
     test('two entries', () async {
       AssetBundle ab = new AssetBundle.fixed('', 'apple.txt,packages/flutter_gallery_assets/shrine/products/heels.png');
       expect(ab.entries, isNotEmpty);
       expect(ab.entries.length, 2);
-      AssetBundleEntry firstEntry = ab.entries.first;
-      expect(firstEntry, isNotNull);
-      expect(firstEntry.archivePath, 'apple.txt');
-      AssetBundleEntry lastEntry = ab.entries.last;
-      expect(lastEntry, isNotNull);
-      expect(lastEntry.archivePath, 'packages/flutter_gallery_assets/shrine/products/heels.png');
+      List<String> archivePaths = ab.entries.keys.toList()..sort();
+      expect(archivePaths[0], 'apple.txt');
+      expect(archivePaths[1], 'packages/flutter_gallery_assets/shrine/products/heels.png');
     });
     test('file contents', () async {
       AssetBundle ab = new AssetBundle.fixed(projectRoot, assetPath);
       expect(ab.entries, isNotEmpty);
       expect(ab.entries.length, 1);
-      AssetBundleEntry entry = ab.entries.first;
-      expect(entry, isNotNull);
-      expect(entry.archivePath, assetPath);
-      expect(assetContents, UTF8.decode(entry.contentsAsBytes()));
+      String archivePath = ab.entries.keys.first;
+      DevFSContent content = ab.entries[archivePath];
+      expect(archivePath, assetPath);
+      expect(assetContents, UTF8.decode(await content.contentsAsBytes()));
     });
   });
 

packages/flutter_tools/test/src/mocks.dart

@@ -13,6 +13,7 @@ import 'package:flutter_tools/src/ios/devices.dart';
 import 'package:flutter_tools/src/ios/simulators.dart';
 import 'package:flutter_tools/src/runner/flutter_command.dart';
 import 'package:mockito/mockito.dart';
+import 'package:test/test.dart';
 
 class MockApplicationPackageStore extends ApplicationPackageStore {
   MockApplicationPackageStore() : super(
@@ -74,9 +75,16 @@ void applyMocksToCommand(FlutterCommand command) {
     ..commandValidator = () => true;
 }
 
-class MockDevFSOperations implements DevFSOperations {
+/// Common functionality for tracking mock interaction
+class BasicMock {
   final List<String> messages = new List<String>();
 
+  void expectMessages(List<String> expectedMessages) {
+    List<String> actualMessages = new List<String>.from(messages);
+    messages.clear();
+    expect(actualMessages, unorderedEquals(expectedMessages));
+  }
+
   bool contains(String match) {
     print('Checking for `$match` in:');
     print(messages);
@@ -84,7 +92,9 @@ class MockDevFSOperations implements DevFSOperations {
     messages.clear();
     return result;
   }
+}
 
+class MockDevFSOperations extends BasicMock implements DevFSOperations {
   @override
   Future<Uri> create(String fsName) async {
     messages.add('create $fsName');
@@ -97,19 +107,12 @@ class MockDevFSOperations implements DevFSOperations {
   }
 
   @override
-  Future<dynamic> writeFile(String fsName, DevFSEntry entry) async {
-    messages.add('writeFile $fsName ${entry.devicePath}');
-  }
-
-  @override
-  Future<dynamic> deleteFile(String fsName, DevFSEntry entry) async {
-    messages.add('deleteFile $fsName ${entry.devicePath}');
+  Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content) async {
+    messages.add('writeFile $fsName $devicePath');
   }
 
   @override
-  Future<dynamic> writeSource(String fsName,
-                              String devicePath,
-                              String contents) async {
-    messages.add('writeSource $fsName $devicePath');
+  Future<dynamic> deleteFile(String fsName, String devicePath) async {
+    messages.add('deleteFile $fsName $devicePath');
   }
 }