Merge pull request #2574 from johnmccutchan/fix_canonicalization

Fix uri canonicalization in DartLibraryProviderFiles

sky/shell/dart/dart_library_provider_files.cc

@@ -15,9 +15,31 @@ namespace sky {
 namespace shell {
 namespace {
 
-void CopyComplete(base::FilePath file, bool success) {
+void CopyComplete(base::FilePath file, std::string uri, bool success) {
   if (!success)
-    LOG(FATAL) << "Failed to load " << file.AsUTF8Unsafe();
+    LOG(FATAL) << "Failed to load "
+               << file.AsUTF8Unsafe()
+               << " (" << uri << ")";
+}
+
+// Extract the scheme prefix ('package:' or 'file:' from )
+static std::string ExtractSchemePrefix(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true)) {
+    return "package:";
+  } else if (base::StartsWithASCII(url, "file:", true)) {
+    return "file:";
+  }
+  return "";
+}
+
+// Extract the path from a package: or file: url.
+static std::string ExtractPath(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&url, 0, "package:", "");
+  } else if (base::StartsWithASCII(url, "file:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&url, 0, "file:", "");
+  }
+  return url;
 }
 
 base::FilePath SimplifyPath(const base::FilePath& path) {
@@ -66,15 +88,47 @@ void DartLibraryProviderFiles::GetLibraryAsStream(
     blink::DataPipeConsumerCallback callback) {
   mojo::DataPipe pipe;
   callback.Run(pipe.consumer_handle.Pass());
-
-  base::FilePath source(name);
+  std::string path = GetFilePathForURL(name);
+  base::FilePath source(path);
   scoped_refptr<base::TaskRunner> runner =
       base::WorkerPool::GetTaskRunner(true);
   mojo::common::CopyFromFile(source, pipe.producer_handle.Pass(), 0,
-                             runner.get(), base::Bind(&CopyComplete, source));
+                             runner.get(),
+                             base::Bind(&CopyComplete, source, name));
 }
 
-std::string DartLibraryProviderFiles::CanonicalizePackageURL(std::string url) {
+Dart_Handle DartLibraryProviderFiles::CanonicalizeURL(Dart_Handle library,
+                                                      Dart_Handle url) {
+  std::string string = blink::StdStringFromDart(url);
+  if (base::StartsWithASCII(string, "dart:", true))
+    return url;
+  if (base::StartsWithASCII(string, "package:", true))
+    return url;
+  if (base::StartsWithASCII(string, "file:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&string, 0, "file:", "");
+    return blink::StdStringToDart(string);;
+  }
+
+  std::string library_url = blink::StdStringFromDart(Dart_LibraryUrl(library));
+  std::string prefix = ExtractSchemePrefix(library_url);
+  std::string path = ExtractPath(library_url);
+  base::FilePath base_path(path);
+  base::FilePath resolved_path = base_path.DirName().Append(string);
+  base::FilePath normalized_path = SimplifyPath(resolved_path);
+  return blink::StdStringToDart(prefix + normalized_path.AsUTF8Unsafe());
+}
+
+std::string DartLibraryProviderFiles::GetFilePathForURL(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true))
+    return GetFilePathForPackageURL(url);
+  if (base::StartsWithASCII(url, "file:", true))
+    return GetFilePathForFileURL(url);
+
+  return url;
+}
+
+std::string DartLibraryProviderFiles::GetFilePathForPackageURL(
+    std::string url) {
   DCHECK(base::StartsWithASCII(url, "package:", true));
   base::ReplaceFirstSubstringAfterOffset(&url, 0, "package:", "");
   size_t slash = url.find('/');
@@ -93,26 +147,11 @@ std::string DartLibraryProviderFiles::CanonicalizePackageURL(std::string url) {
   return SimplifyPath(path).AsUTF8Unsafe();
 }
 
-std::string DartLibraryProviderFiles::CanonicalizeFileURL(std::string url) {
-  DCHECK(base::StartsWithASCII(url, "file:", true));
-  base::ReplaceFirstSubstringAfterOffset(&url, 0, "file:", "");
+std::string DartLibraryProviderFiles::GetFilePathForFileURL(std::string url) {
+  DCHECK(base::StartsWithASCII(url, "file://", true));
+  base::ReplaceFirstSubstringAfterOffset(&url, 0, "file://", "");
   return url;
 }
 
-Dart_Handle DartLibraryProviderFiles::CanonicalizeURL(Dart_Handle library,
-                                                      Dart_Handle url) {
-  std::string string = blink::StdStringFromDart(url);
-  if (base::StartsWithASCII(string, "dart:", true))
-    return url;
-  if (base::StartsWithASCII(string, "package:", true))
-    return blink::StdStringToDart(CanonicalizePackageURL(string));
-  if (base::StartsWithASCII(string, "file:", true))
-    return blink::StdStringToDart(CanonicalizeFileURL(string));
-  base::FilePath base_path(blink::StdStringFromDart(Dart_LibraryUrl(library)));
-  base::FilePath resolved_path = base_path.DirName().Append(string);
-  base::FilePath normalized_path = SimplifyPath(resolved_path);
-  return blink::StdStringToDart(normalized_path.AsUTF8Unsafe());
-}
-
 }  // namespace shell
 }  // namespace sky

sky/shell/dart/dart_library_provider_files.h

@@ -26,8 +26,9 @@ class DartLibraryProviderFiles : public blink::DartLibraryProvider {
   Dart_Handle CanonicalizeURL(Dart_Handle library, Dart_Handle url) override;
 
  private:
-  std::string CanonicalizePackageURL(std::string url);
-  std::string CanonicalizeFileURL(std::string url);
+  std::string GetFilePathForURL(std::string url);
+  std::string GetFilePathForPackageURL(std::string url);
+  std::string GetFilePathForFileURL(std::string url);
 
   base::FilePath packages_;
   tonic::PackagesMap packages_map_;

sky/tools/sky_snapshot/loader.cc

@@ -18,6 +18,26 @@
 
 namespace {
 
+// Extract the scheme prefix ('package:' or 'file:' from )
+static std::string ExtractSchemePrefix(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true)) {
+    return "package:";
+  } else if (base::StartsWithASCII(url, "file:", true)) {
+    return "file:";
+  }
+  return "";
+}
+
+// Extract the path from a package: or file: url.
+static std::string ExtractPath(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&url, 0, "package:", "");
+  } else if (base::StartsWithASCII(url, "file:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&url, 0, "file:", "");
+  }
+  return url;
+}
+
 base::FilePath SimplifyPath(const base::FilePath& path) {
   std::vector<base::FilePath::StringType> components;
   path.GetComponents(&components);
@@ -43,8 +63,10 @@ class Loader {
 
   const std::set<std::string>& dependencies() const { return dependencies_; }
 
-  std::string CanonicalizePackageURL(std::string url);
   Dart_Handle CanonicalizeURL(Dart_Handle library, Dart_Handle url);
+  std::string GetFilePathForURL(std::string url);
+  std::string GetFilePathForPackageURL(std::string url);
+  std::string GetFilePathForFileURL(std::string url);
   std::string Fetch(const std::string& url);
   Dart_Handle Import(Dart_Handle url);
   Dart_Handle Source(Dart_Handle library, Dart_Handle url);
@@ -83,39 +105,59 @@ void Loader::LoadPackagesMap(const base::FilePath& packages) {
   }
 }
 
-std::string Loader::CanonicalizePackageURL(std::string url) {
-  DCHECK(base::StartsWithASCII(url, "package:", true));
-  base::ReplaceFirstSubstringAfterOffset(&url, 0, "package:", "");
-  if (packages_map_) {
-    size_t slash = url.find('/');
-    if (slash == std::string::npos)
-      return std::string();
-    std::string package = url.substr(0, slash);
-    std::string library_path = url.substr(slash + 1);
-    std::string package_path = packages_map_->Resolve(package);
-    if (package_path.empty())
-      return std::string();
-    if (base::StartsWithASCII(package_path, "file://", true)) {
-      base::ReplaceFirstSubstringAfterOffset(&package_path, 0, "file://", "");
-      return package_path + library_path;
-    } else {
-      auto path = packages_.DirName().Append(package_path).Append(library_path);
-      return SimplifyPath(path).AsUTF8Unsafe();
-    }
-  }
-  return package_root_.Append(url).AsUTF8Unsafe();
-}
-
 Dart_Handle Loader::CanonicalizeURL(Dart_Handle library, Dart_Handle url) {
   std::string string = StringFromDart(url);
   if (base::StartsWithASCII(string, "dart:", true))
     return url;
   if (base::StartsWithASCII(string, "package:", true))
-    return StringToDart(CanonicalizePackageURL(string));
-  base::FilePath base_path(StringFromDart(Dart_LibraryUrl(library)));
+    return url;
+  if (base::StartsWithASCII(string, "file:", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&string, 0, "file:", "");
+    return StringToDart(string);;
+  }
+
+  std::string library_url = StringFromDart(Dart_LibraryUrl(library));
+  std::string prefix = ExtractSchemePrefix(library_url);
+  std::string path = ExtractPath(library_url);
+  base::FilePath base_path(path);
   base::FilePath resolved_path = base_path.DirName().Append(string);
   base::FilePath normalized_path = SimplifyPath(resolved_path);
-  return StringToDart(normalized_path.AsUTF8Unsafe());
+  return StringToDart(prefix + normalized_path.AsUTF8Unsafe());
+}
+
+std::string Loader::GetFilePathForURL(std::string url) {
+  if (base::StartsWithASCII(url, "package:", true))
+    return GetFilePathForPackageURL(url);
+  if (base::StartsWithASCII(url, "file:", true))
+    return GetFilePathForFileURL(url);
+
+  return url;
+}
+
+std::string Loader::GetFilePathForPackageURL(
+    std::string url) {
+  DCHECK(base::StartsWithASCII(url, "package:", true));
+  base::ReplaceFirstSubstringAfterOffset(&url, 0, "package:", "");
+  size_t slash = url.find('/');
+  if (slash == std::string::npos)
+    return std::string();
+  std::string package = url.substr(0, slash);
+  std::string library_path = url.substr(slash + 1);
+  std::string package_path = packages_map_->Resolve(package);
+  if (package_path.empty())
+    return std::string();
+  if (base::StartsWithASCII(package_path, "file://", true)) {
+    base::ReplaceFirstSubstringAfterOffset(&package_path, 0, "file://", "");
+    return package_path + library_path;
+  }
+  auto path = packages_.DirName().Append(package_path).Append(library_path);
+  return SimplifyPath(path).AsUTF8Unsafe();
+}
+
+std::string Loader::GetFilePathForFileURL(std::string url) {
+  DCHECK(base::StartsWithASCII(url, "file://", true));
+  base::ReplaceFirstSubstringAfterOffset(&url, 0, "file://", "");
+  return url;
 }
 
 std::string Loader::Fetch(const std::string& url) {