Bugfix in DOM bindings: canonicalize URLs even if no ApplicationLoader is available

DartApplicationLoader.cpp

Index: DartApplicationLoader.cpp
diff --git a/DartApplicationLoader.cpp b/DartApplicationLoader.cpp
index 04dba16d4f282c72d18308765484d4e1136c3d2b..84341b5abd459f602f1f908839feed6923d6ceb3 100644
--- a/DartApplicationLoader.cpp
+++ b/DartApplicationLoader.cpp
@@ -86,12 +86,6 @@ void DartApplicationLoader::reportDartError(Dart_Handle error)
 Dart_Handle DartApplicationLoader::libraryTagHandlerCallback(Dart_LibraryTag tag, Dart_Handle library, Dart_Handle urlHandle)
 {
     ASSERT(Dart_CurrentIsolate());
-    ASSERT(DartDOMData::current()->applicationLoader());
-    return DartDOMData::current()->applicationLoader()->libraryTagHandler(tag, library, urlHandle);

[kustermann, 2013/07/10 13:50:39]

(Sidenote: AFAIK the styleguide allows long lines, so I didn't shorten them.)

-}
-
-Dart_Handle DartApplicationLoader::libraryTagHandler(Dart_LibraryTag tag, Dart_Handle library, Dart_Handle urlHandle)
-{
     ASSERT(Dart_IsLibrary(library));
 
     const String url = DartUtilities::toString(urlHandle);
@@ -101,24 +95,38 @@ Dart_Handle DartApplicationLoader::libraryTagHandler(Dart_LibraryTag tag, Dart_H
     String libraryURL = DartUtilities::toString(libraryURLHandle);

[siva, 2013/07/10 18:10:26]

Why does this library to libraryURL translation have to happen eagerly. For the
case
url.startsWith("dart:") || .....
libraryURL is not used at all.

You could instead change the signature as:
static Dart_Handle CanonicalizeUrl(Dart_Handle urlHandle, String url,
Dart_Handle library);

and do the libraryURL translation inside CanonicalizeUrl if
url.startsWith("dart:") || url.startsWith("package:") is false.

[kustermann, 2013/07/11 12:34:11]

Done.

 
     if (tag == Dart_kCanonicalizeUrl) {
-        if (url.startsWith("dart:") || url.startsWith("package:"))
-            return urlHandle;
-
-        bool packageScheme = false;
-        if (libraryURL.startsWith("package:")) {
-            // KURL have problems concating package:foo/bar (without slashes right after colon)
-            // and relative urls. Therefore pretend to be a standard absolute URL.
-            packageScheme = true;
-            libraryURL = "http://" + libraryURL.substring(8);
-        }
+          // If a dart application calls spawnUri, the DartVM will call this
+          // libraryTagHandler to canonicalize the url.
+          // DartDOMData::current()->applicationLoader() may be NULL at this point.
+          return DartApplicationLoader::CanonicalizeUrl(urlHandle, url, libraryURL);
+    } else {
+          ASSERT(DartDOMData::current()->applicationLoader());
+          return DartDOMData::current()->applicationLoader()->libraryTagHandler(tag, urlHandle, url, libraryURL);

[siva, 2013/07/10 18:10:26]

Similarly change the signature of libraryTagHandler to
Dart_Handle libraryTagHandler(Dart_LibraryTag tag, Dart_Handle urlHandle, String
url, Dart_Handle library);

and inside libraryTagHandler do the translation only if
tag == Dart_kSourceTag is true.

[kustermann, 2013/07/11 12:34:11]

Done.

+    }
+}
 
-        const KURL canonical = KURL(KURL(KURL(), libraryURL), url);
-        String result = canonical.string();
-        if (packageScheme)
-            result = "package:" + result.substring(7);
-        return DartUtilities::stringToDartString(result);
+Dart_Handle DartApplicationLoader::CanonicalizeUrl(Dart_Handle urlHandle, String url, String libraryURL)
+{
+    if (url.startsWith("dart:") || url.startsWith("package:"))
+        return urlHandle;
+
+    bool packageScheme = false;
+    if (libraryURL.startsWith("package:")) {
+        // KURL have problems concating package:foo/bar (without slashes right after colon)

[siva, 2013/07/10 18:10:26]

KURL has problems......

[kustermann, 2013/07/11 12:34:11]

Done.

+        // and relative urls. Therefore pretend to be a standard absolute URL.
+        packageScheme = true;
+        libraryURL = "http://" + libraryURL.substring(8);

[siva, 2013/07/10 18:10:26]

These magic numbers 8, 7 etc. make me uncomfortable,
can we use named constants instead which are more descriptive
e.g:
const char* kPackagePrefix = "package:";
const int kPackagePrefixLength = strlen(kPackagePrefix);

const char* kHttpPrefix = "http://";
const int kHttpPrefixLength = strlen(kHttpPrefix);

[kustermann, 2013/07/11 12:34:11]

Done.

     }
 
+    const KURL canonical = KURL(KURL(KURL(), libraryURL), url);
+    String result = canonical.string();
+    if (packageScheme)
+        result = "package:" + result.substring(7);
+    return DartUtilities::stringToDartString(result);
+}
+
+Dart_Handle DartApplicationLoader::libraryTagHandler(Dart_LibraryTag tag, Dart_Handle urlHandle, String url, String libraryURL)
+{
     ASSERT(url != "dart:html");
 
     // Record the importer.