[devFS] Use URI to represent paths on device (#8446)

* [devFS] Use URI to represent paths on device

Previosuly, regular file paths in the format of the host platform were used to represent paths on device. That works when host and device share the same (POSIX) file path format. With a Windows host, this breaks. URIs are the solution as they are platform independent and the VM service on the device already interpreted the file paths as URIs anyways.

* review comments

* switch to file paths

* fix tests on Windows

* review comments

packages/flutter_tools/lib/src/devfs.dart

@@ -160,8 +160,8 @@ class DevFSStringContent extends DevFSByteContent {
 abstract class DevFSOperations {
   Future<Uri> create(String fsName);
   Future<dynamic> destroy(String fsName);
-  Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content);
-  Future<dynamic> deleteFile(String fsName, String devicePath);
+  Future<dynamic> writeFile(String fsName, Uri deviceUri, DevFSContent content);
+  Future<dynamic> deleteFile(String fsName, Uri deviceUri);
 }
 
 /// An implementation of [DevFSOperations] that speaks to the
@@ -186,7 +186,7 @@ class ServiceProtocolDevFSOperations implements DevFSOperations {
   }
 
   @override
-  Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content) async {
+  Future<dynamic> writeFile(String fsName, Uri deviceUri, DevFSContent content) async {
     List<int> bytes;
     try {
       bytes = await content.contentsAsBytes();
@@ -199,17 +199,18 @@ class ServiceProtocolDevFSOperations implements DevFSOperations {
         '_writeDevFSFile',
         params: <String, dynamic> {
           'fsName': fsName,
-          'path': devicePath,
+          // TODO(goderbauer): transfer real Uri (instead of file path) when remote end supports it
+          'path': deviceUri.toFilePath(windows: false),
           'fileContents': fileContents
         },
       );
     } catch (error) {
-      printTrace('DevFS: Failed to write $devicePath: $error');
+      printTrace('DevFS: Failed to write $deviceUri: $error');
     }
   }
 
   @override
-  Future<dynamic> deleteFile(String fsName, String devicePath) async {
+  Future<dynamic> deleteFile(String fsName, Uri deviceUri) async {
     // TODO(johnmccutchan): Add file deletion to the devFS protocol.
   }
 }
@@ -225,18 +226,18 @@ class _DevFSHttpWriter {
   static const int kMaxRetries = 3;
 
   int _inFlight = 0;
-  Map<String, DevFSContent> _outstanding;
+  Map<Uri, DevFSContent> _outstanding;
   Completer<Null> _completer;
   HttpClient _client;
   int _done;
   int _max;
 
-  Future<Null> write(Map<String, DevFSContent> entries,
+  Future<Null> write(Map<Uri, DevFSContent> entries,
                      {DevFSProgressReporter progressReporter}) async {
     _client = new HttpClient();
     _client.maxConnectionsPerHost = kMaxInFlight;
     _completer = new Completer<Null>();
-    _outstanding = new Map<String, DevFSContent>.from(entries);
+    _outstanding = new Map<Uri, DevFSContent>.from(entries);
     _done = 0;
     _max = _outstanding.length;
     _scheduleWrites(progressReporter);
@@ -250,15 +251,15 @@ class _DevFSHttpWriter {
         // Finished.
         break;
       }
-      String devicePath = _outstanding.keys.first;
-      DevFSContent content = _outstanding.remove(devicePath);
-      _scheduleWrite(devicePath, content, progressReporter);
+      Uri deviceUri = _outstanding.keys.first;
+      DevFSContent content = _outstanding.remove(deviceUri);
+      _scheduleWrite(deviceUri, content, progressReporter);
       _inFlight++;
     }
   }
 
   Future<Null> _scheduleWrite(
-    String devicePath,
+    Uri deviceUri,
     DevFSContent content,
     DevFSProgressReporter progressReporter, [
     int retry = 0,
@@ -267,19 +268,20 @@ class _DevFSHttpWriter {
       HttpClientRequest request = await _client.putUrl(httpAddress);
       request.headers.removeAll(HttpHeaders.ACCEPT_ENCODING);
       request.headers.add('dev_fs_name', fsName);
+      // TODO(goderbauer): transfer real Uri (instead of file path) when remote end supports it
       request.headers.add('dev_fs_path_b64',
-                          BASE64.encode(UTF8.encode(devicePath)));
+                          BASE64.encode(UTF8.encode(deviceUri.toFilePath(windows: false))));
       Stream<List<int>> contents = content.contentsAsCompressedStream();
       await request.addStream(contents);
       HttpClientResponse response = await request.close();
       await response.drain<Null>();
     } catch (e) {
       if (retry < kMaxRetries) {
-        printTrace('Retrying writing "$devicePath" to DevFS due to error: $e');
-        _scheduleWrite(devicePath, content, progressReporter, retry + 1);
+        printTrace('Retrying writing "$deviceUri" to DevFS due to error: $e');
+        _scheduleWrite(deviceUri, content, progressReporter, retry + 1);
         return;
       } else {
-        printError('Error writing "$devicePath" to DevFS: $e');
+        printError('Error writing "$deviceUri" to DevFS: $e');
       }
     }
     if (progressReporter != null) {
@@ -324,7 +326,7 @@ class DevFS {
   final String fsName;
   final Directory rootDirectory;
   String _packagesFilePath;
-  final Map<String, DevFSContent> _entries = <String, DevFSContent>{};
+  final Map<Uri, DevFSContent> _entries = <Uri, DevFSContent>{};
   final Set<String> assetPathsToEvict = new Set<String>();
 
   final List<Future<Map<String, dynamic>>> _pendingOperations =
@@ -373,17 +375,17 @@ class DevFS {
 
     // Handle deletions.
     printTrace('Scanning for deleted files');
-    String assetBuildDirPrefix = getAssetBuildDirectory() + fs.path.separator;
-    final List<String> toRemove = new List<String>();
-    _entries.forEach((String devicePath, DevFSContent content) {
+    String assetBuildDirPrefix = _asUriPath(getAssetBuildDirectory());
+    final List<Uri> toRemove = new List<Uri>();
+    _entries.forEach((Uri deviceUri, DevFSContent content) {
       if (!content._exists) {
         Future<Map<String, dynamic>> operation =
-            _operations.deleteFile(fsName, devicePath);
+            _operations.deleteFile(fsName, deviceUri);
         if (operation != null)
           _pendingOperations.add(operation);
-        toRemove.add(devicePath);
-        if (devicePath.startsWith(assetBuildDirPrefix)) {
-          String archivePath = devicePath.substring(assetBuildDirPrefix.length);
+        toRemove.add(deviceUri);
+        if (deviceUri.path.startsWith(assetBuildDirPrefix)) {
+          String archivePath = deviceUri.path.substring(assetBuildDirPrefix.length);
           assetPathsToEvict.add(archivePath);
         }
       }
@@ -397,13 +399,13 @@ class DevFS {
 
     // Update modified files
     int numBytes = 0;
-    Map<String, DevFSContent> dirtyEntries = <String, DevFSContent>{};
-    _entries.forEach((String devicePath, DevFSContent content) {
+    Map<Uri, DevFSContent> dirtyEntries = <Uri, DevFSContent>{};
+    _entries.forEach((Uri deviceUri, DevFSContent content) {
       String archivePath;
-      if (devicePath.startsWith(assetBuildDirPrefix))
-        archivePath = devicePath.substring(assetBuildDirPrefix.length);
+      if (deviceUri.path.startsWith(assetBuildDirPrefix))
+        archivePath = deviceUri.path.substring(assetBuildDirPrefix.length);
       if (content.isModified || (bundleDirty && archivePath != null)) {
-        dirtyEntries[devicePath] = content;
+        dirtyEntries[deviceUri] = content;
         numBytes += content.size;
         if (archivePath != null)
           assetPathsToEvict.add(archivePath);
@@ -420,9 +422,9 @@ class DevFS {
         }
       } else {
         // Make service protocol requests for each.
-        dirtyEntries.forEach((String devicePath, DevFSContent content) {
+        dirtyEntries.forEach((Uri deviceUri, DevFSContent content) {
           Future<Map<String, dynamic>> operation =
-              _operations.writeFile(fsName, devicePath, content);
+              _operations.writeFile(fsName, deviceUri, content);
           if (operation != null)
             _pendingOperations.add(operation);
         });
@@ -446,41 +448,44 @@ class DevFS {
     return numBytes;
   }
 
-  void _scanFile(String devicePath, FileSystemEntity file) {
-    DevFSContent content = _entries.putIfAbsent(devicePath, () => new DevFSFileContent(file));
+  void _scanFile(Uri deviceUri, FileSystemEntity file) {
+    DevFSContent content = _entries.putIfAbsent(deviceUri, () => new DevFSFileContent(file));
     content._exists = true;
   }
 
   void _scanBundleEntry(String archivePath, DevFSContent content, bool bundleDirty) {
     // We write the assets into the AssetBundle working dir so that they
     // are in the same location in DevFS and the iOS simulator.
-    final String devicePath = fs.path.join(getAssetBuildDirectory(), archivePath);
+    final Uri deviceUri = fs.path.toUri(fs.path.join(getAssetBuildDirectory(), archivePath));
 
-    _entries[devicePath] = content;
+    _entries[deviceUri] = content;
     content._exists = true;
   }
 
-  bool _shouldIgnore(String devicePath) {
-    List<String> ignoredPrefixes = <String>['android' + fs.path.separator,
-                                            getBuildDirectory(),
-                                            'ios' + fs.path.separator,
-                                            '.pub' + fs.path.separator];
-    for (String ignoredPrefix in ignoredPrefixes) {
-      if (devicePath.startsWith(ignoredPrefix))
+  bool _shouldIgnore(Uri deviceUri) {
+    List<String> ignoredUriPrefixes = <String>['android/',
+                                               _asUriPath(getBuildDirectory()),
+                                               'ios/',
+                                               '.pub/'];
+    for (String ignoredUriPrefix in ignoredUriPrefixes) {
+      if (deviceUri.path.startsWith(ignoredUriPrefix))
         return true;
     }
     return false;
   }
 
   Future<bool> _scanDirectory(Directory directory,
-                              {String directoryNameOnDevice,
+                              {Uri directoryUriOnDevice,
                                bool recursive: false,
                                bool ignoreDotFiles: true,
                                Set<String> fileFilter}) async {
-    if (directoryNameOnDevice == null) {
-      directoryNameOnDevice = fs.path.relative(directory.path, from: rootDirectory.path);
-      if (directoryNameOnDevice == '.')
-        directoryNameOnDevice = '';
+    if (directoryUriOnDevice == null) {
+      String relativeRootPath = fs.path.relative(directory.path, from: rootDirectory.path);
+      if (relativeRootPath == '.') {
+        directoryUriOnDevice = new Uri();
+      } else {
+        directoryUriOnDevice = fs.path.toUri(relativeRootPath);
+      }
     }
     try {
       Stream<FileSystemEntity> files =
@@ -506,17 +511,17 @@ class DevFS {
         }
         final String relativePath =
             fs.path.relative(file.path, from: directory.path);
-        final String devicePath = fs.path.join(directoryNameOnDevice, relativePath);
+        final Uri deviceUri = directoryUriOnDevice.resolveUri(fs.path.toUri(relativePath));
         if ((fileFilter != null) && !fileFilter.contains(file.absolute.path)) {
           // Skip files that are not included in the filter.
           continue;
         }
-        if (ignoreDotFiles && devicePath.startsWith('.')) {
+        if (ignoreDotFiles && deviceUri.path.startsWith('.')) {
           // Skip directories that start with a dot.
           continue;
         }
-        if (!_shouldIgnore(devicePath))
-          _scanFile(devicePath, file);
+        if (!_shouldIgnore(deviceUri))
+          _scanFile(deviceUri, file);
       }
     } catch (e) {
       // Ignore directory and error.
@@ -531,34 +536,38 @@ class DevFS {
 
     for (String packageName in packageMap.map.keys) {
       Uri packageUri = packageMap.map[packageName];
-      String packagePath = fs.path.fromUri(packageUri);
+      String packagePath = packageUri.toFilePath();
       Directory packageDirectory = fs.directory(packageUri);
-      String directoryNameOnDevice = fs.path.join('packages', packageName);
+      Uri directoryUriOnDevice = fs.path.toUri(fs.path.join('packages', packageName) + fs.path.separator);
       bool packageExists;
 
       if (fs.path.isWithin(rootDirectory.path, packagePath)) {
         // We already scanned everything under the root directory.
         packageExists = packageDirectory.existsSync();
-        directoryNameOnDevice = fs.path.relative(packagePath, from: rootDirectory.path);
+        directoryUriOnDevice = fs.path.toUri(
+            fs.path.relative(packagePath, from: rootDirectory.path) + fs.path.separator
+        );
       } else {
         packageExists =
             await _scanDirectory(packageDirectory,
-                                 directoryNameOnDevice: directoryNameOnDevice,
+                                 directoryUriOnDevice: directoryUriOnDevice,
                                  recursive: true,
                                  fileFilter: fileFilter);
       }
       if (packageExists) {
         sb ??= new StringBuffer();
-        sb.writeln('$packageName:$directoryNameOnDevice');
+        sb.writeln('$packageName:$directoryUriOnDevice');
       }
     }
     if (sb != null) {
-      DevFSContent content = _entries['.packages'];
+      DevFSContent content = _entries[fs.path.toUri('.packages')];
       if (content is DevFSStringContent && content.string == sb.toString()) {
         content._exists = true;
         return;
       }
-      _entries['.packages'] = new DevFSStringContent(sb.toString());
+      _entries[fs.path.toUri('.packages')] = new DevFSStringContent(sb.toString());
     }
   }
 }
+/// Converts a platform-specific file path to a platform-independent Uri path.
+String _asUriPath(String filePath) => fs.path.toUri(filePath).path + '/';

packages/flutter_tools/lib/src/run_hot.dart

@@ -434,17 +434,15 @@ class HotRunner extends ResidentRunner {
     String reloadMessage;
     try {
       String entryPath = fs.path.relative(mainPath, from: projectRootPath);
-      String deviceEntryPath =
-          _devFS.baseUri.resolve(entryPath).toFilePath();
-      String devicePackagesPath =
-          _devFS.baseUri.resolve('.packages').toFilePath();
+      Uri deviceEntryUri = _devFS.baseUri.resolveUri(fs.path.toUri(entryPath));
+      Uri devicePackagesUri = _devFS.baseUri.resolve('.packages');
       if (benchmarkMode)
         vmReloadTimer.start();
       Map<String, dynamic> reloadReport =
           await currentView.uiIsolate.reloadSources(
               pause: pause,
-              rootLibPath: deviceEntryPath,
-              packagesPath: devicePackagesPath);
+              rootLibUri: deviceEntryUri,
+              packagesUri: devicePackagesUri);
       if (!validateReloadReport(reloadReport)) {
         // Reload failed.
         flutterUsage.sendEvent('hot', 'reload-reject');

packages/flutter_tools/lib/src/vmservice.dart

@@ -838,17 +838,19 @@ class Isolate extends ServiceObjectOwner {
 
   Future<Map<String, dynamic>> reloadSources(
       { bool pause: false,
-        String rootLibPath,
-        String packagesPath}) async {
+        Uri rootLibUri,
+        Uri packagesUri}) async {
     try {
       Map<String, dynamic> arguments = <String, dynamic>{
         'pause': pause
       };
-      if (rootLibPath != null) {
-        arguments['rootLibUri'] = rootLibPath;
+      // TODO(goderbauer): Transfer Uri (instead of file path) when remote end supports it.
+      //     Note: Despite the name, `rootLibUri` and `packagesUri` expect file paths.
+      if (rootLibUri != null) {
+        arguments['rootLibUri'] = rootLibUri.toFilePath(windows: false);
       }
-      if (packagesPath != null) {
-        arguments['packagesUri'] = packagesPath;
+      if (packagesUri != null) {
+        arguments['packagesUri'] = packagesUri.toFilePath(windows: false);
       }
       Map<String, dynamic> response = await invokeRpcRaw('_reloadSources', params: arguments);
       return response;

packages/flutter_tools/test/devfs_test.dart

@@ -4,7 +4,6 @@
 
 import 'dart:async';
 import 'dart:convert';
-import 'dart:io' as io;
 
 import 'package:flutter_tools/src/asset.dart';
 import 'package:flutter_tools/src/base/io.dart';
@@ -70,7 +69,7 @@ void main() {
       File file = fs.file(fs.path.join(basePath, filePath));
       await file.parent.create(recursive: true);
       file.writeAsBytesSync(<int>[1, 2, 3]);
-      _packages['my_project'] = 'lib';
+      _packages['my_project'] = fs.path.toUri('lib');
 
       // simulate package
       await _createPackage('somepkg', 'somefile.txt');
@@ -82,20 +81,20 @@ void main() {
 
       int bytes = await devFS.update();
       devFSOperations.expectMessages(<String>[
-        'writeFile test ${fs.path.join('lib', 'foo.txt')}',
-        'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}',
         'writeFile test .packages',
+        'writeFile test lib/foo.txt',
+        'writeFile test packages/somepkg/somefile.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
 
       List<String> packageSpecOnDevice = LineSplitter.split(UTF8.decode(
-          await devFSOperations.devicePathToContent['.packages'].contentsAsBytes()
+          await devFSOperations.devicePathToContent[fs.path.toUri('.packages')].contentsAsBytes()
       )).toList();
       expect(packageSpecOnDevice,
-          unorderedEquals(<String>['my_project:lib', 'somepkg:packages/somepkg'])
+          unorderedEquals(<String>['my_project:lib/', 'somepkg:packages/somepkg/'])
       );
 
-      expect(bytes, 46);
+      expect(bytes, 48);
     });
     testUsingContext('add new file to local file system', () async {
       File file = fs.file(fs.path.join(basePath, filePath2));
@@ -103,7 +102,7 @@ void main() {
       file.writeAsBytesSync(<int>[1, 2, 3, 4, 5, 6, 7]);
       int bytes = await devFS.update();
       devFSOperations.expectMessages(<String>[
-        'writeFile test ${fs.path.join('foo', 'bar.txt')}',
+        'writeFile test foo/bar.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
       expect(bytes, 7);
@@ -120,7 +119,7 @@ void main() {
       await file.writeAsBytes(<int>[1, 2, 3, 4, 5, 6]);
       bytes = await devFS.update();
       devFSOperations.expectMessages(<String>[
-        'writeFile test ${fs.path.join('lib', 'foo.txt')}',
+        'writeFile test lib/foo.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
       expect(bytes, 6);
@@ -130,7 +129,7 @@ void main() {
       await file.delete();
       int bytes = await devFS.update();
       devFSOperations.expectMessages(<String>[
-        'deleteFile test ${fs.path.join('lib', 'foo.txt')}',
+        'deleteFile test lib/foo.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
       expect(bytes, 0);
@@ -140,10 +139,10 @@ void main() {
       int bytes = await devFS.update();
       devFSOperations.expectMessages(<String>[
         'writeFile test .packages',
-        'writeFile test ${fs.path.join('packages', 'newpkg', 'anotherfile.txt')}',
+        'writeFile test packages/newpkg/anotherfile.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
-      expect(bytes, 66);
+      expect(bytes, 69);
     });
     testUsingContext('add an asset bundle', () async {
       assetBundle.entries['a.txt'] = new DevFSStringContent('abc');
@@ -207,7 +206,7 @@ void main() {
       devFSOperations.expectMessages(<String>['destroy test']);
       expect(devFS.assetPathsToEvict, isEmpty);
     });
-  }, skip: io.Platform.isWindows);
+  });
 
   group('devfs remote', () {
     MockVMService vmService;
@@ -240,11 +239,11 @@ void main() {
       int bytes = await devFS.update();
       vmService.expectMessages(<String>[
         'writeFile test .packages',
-        'writeFile test ${fs.path.join('lib', 'foo.txt')}',
-        'writeFile test ${fs.path.join('packages', 'somepkg', 'somefile.txt')}',
+        'writeFile test lib/foo.txt',
+        'writeFile test packages/somepkg/somefile.txt',
       ]);
       expect(devFS.assetPathsToEvict, isEmpty);
-      expect(bytes, 46);
+      expect(bytes, 48);
     }, timeout: const Timeout(const Duration(seconds: 5)));
 
     testUsingContext('delete dev file system', () async {
@@ -252,7 +251,7 @@ void main() {
       vmService.expectMessages(<String>['_deleteDevFS {fsName: test}']);
       expect(devFS.assetPathsToEvict, isEmpty);
     });
-  }, skip: io.Platform.isWindows);
+  });
 }
 
 class MockVMService extends BasicMock implements VMService {
@@ -321,7 +320,7 @@ class MockVM implements VM {
 
 
 final List<Directory> _tempDirs = <Directory>[];
-final Map <String, String> _packages = <String, String>{};
+final Map <String, Uri> _packages = <String, Uri>{};
 
 Directory _newTempDir() {
   Directory tempDir = fs.systemTempDirectory.createTempSync('devfs${_tempDirs.length}');
@@ -340,12 +339,14 @@ Future<Null> _createPackage(String pkgName, String pkgFileName) async {
   File pkgFile = fs.file(fs.path.join(pkgTempDir.path, pkgName, 'lib', pkgFileName));
   await pkgFile.parent.create(recursive: true);
   pkgFile.writeAsBytesSync(<int>[11, 12, 13]);
-  _packages[pkgName] = pkgFile.parent.path;
+  _packages[pkgName] = fs.path.toUri(pkgFile.parent.path);
   StringBuffer sb = new StringBuffer();
-  _packages.forEach((String pkgName, String pkgPath) {
-    sb.writeln('$pkgName:$pkgPath');
+  _packages.forEach((String pkgName, Uri pkgUri) {
+    sb.writeln('$pkgName:$pkgUri');
   });
   fs.file(fs.path.join(_tempDirs[0].path, '.packages')).writeAsStringSync(sb.toString());
 }
 
-String _inAssetBuildDirectory(String filename) => fs.path.join(getAssetBuildDirectory(), filename);
+String _inAssetBuildDirectory(String filename) {
+  return '${fs.path.toUri(getAssetBuildDirectory()).path}/$filename';
+}

packages/flutter_tools/test/src/mocks.dart

@@ -95,7 +95,7 @@ class BasicMock {
 }
 
 class MockDevFSOperations extends BasicMock implements DevFSOperations {
-  Map<String, DevFSContent> devicePathToContent = <String, DevFSContent>{};
+  Map<Uri, DevFSContent> devicePathToContent = <Uri, DevFSContent>{};
 
   @override
   Future<Uri> create(String fsName) async {
@@ -109,14 +109,14 @@ class MockDevFSOperations extends BasicMock implements DevFSOperations {
   }
 
   @override
-  Future<dynamic> writeFile(String fsName, String devicePath, DevFSContent content) async {
-    messages.add('writeFile $fsName $devicePath');
-    devicePathToContent[devicePath] = content;
+  Future<dynamic> writeFile(String fsName, Uri deviceUri, DevFSContent content) async {
+    messages.add('writeFile $fsName $deviceUri');
+    devicePathToContent[deviceUri] = content;
   }
 
   @override
-  Future<dynamic> deleteFile(String fsName, String devicePath) async {
-    messages.add('deleteFile $fsName $devicePath');
-    devicePathToContent.remove(devicePath);
+  Future<dynamic> deleteFile(String fsName, Uri deviceUri) async {
+    messages.add('deleteFile $fsName $deviceUri');
+    devicePathToContent.remove(deviceUri);
   }
 }