Fix package relative imports

Source/bindings/dart/DartUtilities.cpp

Index: Source/bindings/dart/DartUtilities.cpp
diff --git a/Source/bindings/dart/DartUtilities.cpp b/Source/bindings/dart/DartUtilities.cpp
index 910a4b3672f30e866131a8b0c913b54df64c8292..1a6946e3cd360ccf2ef10e78cb06618df339e763 100644
--- a/Source/bindings/dart/DartUtilities.cpp
+++ b/Source/bindings/dart/DartUtilities.cpp
@@ -1094,32 +1094,34 @@ const uint8_t* DartUtilities::fullSnapshot(LocalFrame* frame)
 
 Dart_Handle DartUtilities::canonicalizeUrl(Dart_Handle library, Dart_Handle urlHandle, String url)
 {
-    if (url.startsWith("dart:") || url.startsWith("package:"))
+    DEFINE_STATIC_LOCAL(const char*, dartPrefix, ("dart:"));
+    DEFINE_STATIC_LOCAL(const char*, packagePrefix, ("package:"));
+
+    if (url.startsWith(dartPrefix) || url.startsWith(packagePrefix))
+        // Not a relative URL.
         return urlHandle;
 
     Dart_Handle libraryURLHandle = Dart_LibraryUrl(library);
     ASSERT(!Dart_IsError(libraryURLHandle));
     String libraryURL = DartUtilities::toString(libraryURLHandle);
 
-    bool packageScheme = false;
-
-    const char* kPackagePrefix = "package:";
-    const int kPackagePrefixLength = strlen(kPackagePrefix);
-
-    const char* kHttpPrefix = "http://";
-    const int kHttpPrefixLength = strlen(kHttpPrefix);
-
-    if (libraryURL.startsWith(kPackagePrefix)) {
-        // KURL has problems concating package:foo/bar (without slashes right after colon)
-        // and relative urls. Therefore pretend to be a standard absolute URL.
-        packageScheme = true;
-        libraryURL = kHttpPrefix + libraryURL.substring(kPackagePrefixLength);
+    String result;
+    if (libraryURL.startsWith(packagePrefix) && !libraryURL.startsWith("package://")) {
+        // KURL expects a URL with an authority. If the library URL is
+        // "package:<path>", we convert to "package://_/<path>" for resolution and

[Jacob, 2014/06/23 16:42:27]

nit: keep commend at 80 chars.

[vsm, 2014/06/24 07:16:28]

Done.

+        // convert back.

[ahe, 2014/06/23 16:05:49]

Interesting approach. As far as I can tell, Chrome doesn't try to rewrite URIs
like http://_//fisk.txt and file://_//fisk.txt.  However, http:////fisk.txt and
file:////fisk.txt lead to unexpected results where slashes are dropped.

In other words, it seems like the "_" ensures that this trick works for both:

package:/foo...

and

package:foo...

So that's good :-)

+        DEFINE_STATIC_LOCAL(const int, packagePrefixLength, (strlen(packagePrefix)));
+        DEFINE_STATIC_LOCAL(const char*, resolvePrefix, ("package://_/"));
+        DEFINE_STATIC_LOCAL(const int, resolvePrefixLength, (strlen(resolvePrefix)));

[Jacob, 2014/06/23 16:42:27]

instead of defining static locals for the string and the length 
consider
DEFINE_STATIC_LOCAL(String, resolvePrefix, ("package://_/"))

getting the length from the string object should be O(1) as well and the code
will be a bit more terse.

[vsm, 2014/06/24 07:16:28]

Done.

+
+        libraryURL = resolvePrefix + libraryURL.substring(packagePrefixLength);
+        result = KURL(KURL(KURL(), libraryURL), url).string();
+        if (result.startsWith(resolvePrefix))
+            result = packagePrefix + result.substring(resolvePrefixLength);
+    } else {
+        result = KURL(KURL(KURL(), libraryURL), url).string();
     }
 
-    const KURL canonical = KURL(KURL(KURL(), libraryURL), url);
-    String result = canonical.string();
-    if (packageScheme)
-        result = kPackagePrefix + result.substring(kHttpPrefixLength);
     return DartUtilities::stringToDartString(result);
 }