Custom handlers should throw SecurityError exception if the URL's origin differs from the document'…

Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp

Index: Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp
diff --git a/Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp b/Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp
index 1b86cf25ed48951fe845f704cc3f9ef075c74bcf..d83078fcdbdf29d9cfc5124dba84de8e09ceb600 100644
--- a/Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp
+++ b/Source/modules/navigatorcontentutils/NavigatorContentUtils.cpp
@@ -68,7 +68,7 @@ static void initCustomSchemeHandlerWhitelist()
         schemeWhitelist->add(schemes[i]);
 }
 
-static bool verifyCustomHandlerURL(const KURL& baseURL, const String& url, ExceptionState& exceptionState)
+static bool verifyCustomHandlerURL(const Document& document, const String& url, ExceptionState& exceptionState)
 {
     // The specification requires that it is a SyntaxError if the "%s" token is
     // not present.
@@ -84,13 +84,20 @@ static bool verifyCustomHandlerURL(const KURL& baseURL, const String& url, Excep
     String newURL = url;
     newURL.remove(index, WTF_ARRAY_LENGTH(token) - 1);
 
-    KURL kurl(baseURL, newURL);
+    KURL baseURL = document.baseURL();

[tkent, 2014/09/22 09:53:14]

This variable looks unnecessary.
The following |if| block uses |baseURL|, but we can replace it with
document.baseURL().

[pals, 2014/09/22 11:37:53]

Done.

+    KURL kurl = document.completeURL(url);
 
     if (kurl.isEmpty() || !kurl.isValid()) {
         exceptionState.throwDOMException(SyntaxError, "The custom handler URL created by removing '%s' and prepending '" + baseURL.string() + "' is invalid.");
         return false;
     }
 
+    // The specification says that the API throws SecurityError exception if the URL's origin differs from the document's origin.

[tkent, 2014/09/22 09:53:14]

nit: I recommend to wrap code comments in 80 columns because of readability in
codereview.chromium.org.

[pals, 2014/09/22 11:37:53]

Done.

+    if (!document.securityOrigin()->canRequest(kurl)) {
+        exceptionState.throwSecurityError("Can only register custom handler in the document's origin.");
+        return false;
+    }
+
     return true;
 }
 
@@ -149,17 +156,18 @@ void NavigatorContentUtils::registerProtocolHandler(Navigator& navigator, const
     if (!navigator.frame())
         return;
 
-    ASSERT(navigator.frame()->document());
-    KURL baseURL = navigator.frame()->document()->baseURL();
+    Document* document = navigator.frame()->document();
+    ASSERT(document);
 
-    if (!verifyCustomHandlerURL(baseURL, url, exceptionState))
+    if (!verifyCustomHandlerURL(*document, url, exceptionState))
         return;
 
     if (!verifyCustomHandlerScheme(scheme, exceptionState))
         return;
 
     ASSERT(navigator.frame()->page());
-    NavigatorContentUtils::from(*navigator.frame()->page())->client()->registerProtocolHandler(scheme, baseURL, KURL(ParsedURLString, url), title);
+    KURL baseURL = document->baseURL();

[tkent, 2014/09/22 09:53:14]

This variable is not used.

[pals, 2014/09/22 11:37:53]

Done.

+    NavigatorContentUtils::from(*navigator.frame()->page())->client()->registerProtocolHandler(scheme, document->completeURL(url), title);
 }
 
 static String customHandlersStateString(const NavigatorContentUtilsClient::CustomHandlersState state)
@@ -193,16 +201,15 @@ String NavigatorContentUtils::isProtocolHandlerRegistered(Navigator& navigator,
     if (document->activeDOMObjectsAreStopped())
         return declined;
 
-    KURL baseURL = document->baseURL();
-
-    if (!verifyCustomHandlerURL(baseURL, url, exceptionState))
+    if (!verifyCustomHandlerURL(*document, url, exceptionState))
         return declined;
 
     if (!verifyCustomHandlerScheme(scheme, exceptionState))
         return declined;
 
     ASSERT(navigator.frame()->page());
-    return customHandlersStateString(NavigatorContentUtils::from(*navigator.frame()->page())->client()->isProtocolHandlerRegistered(scheme, baseURL, KURL(ParsedURLString, url)));
+    KURL baseURL = document->baseURL();

[tkent, 2014/09/22 09:53:14]

This variable is not used.

[pals, 2014/09/22 11:37:53]

Done.

+    return customHandlersStateString(NavigatorContentUtils::from(*navigator.frame()->page())->client()->isProtocolHandlerRegistered(scheme, document->completeURL(url)));
 }
 
 void NavigatorContentUtils::unregisterProtocolHandler(Navigator& navigator, const String& scheme, const String& url, ExceptionState& exceptionState)
@@ -210,17 +217,18 @@ void NavigatorContentUtils::unregisterProtocolHandler(Navigator& navigator, cons
     if (!navigator.frame())
         return;
 
-    ASSERT(navigator.frame()->document());
-    KURL baseURL = navigator.frame()->document()->baseURL();
+    Document* document = navigator.frame()->document();
+    ASSERT(document);
 
-    if (!verifyCustomHandlerURL(baseURL, url, exceptionState))
+    if (!verifyCustomHandlerURL(*document, url, exceptionState))
         return;
 
     if (!verifyCustomHandlerScheme(scheme, exceptionState))
         return;
 
     ASSERT(navigator.frame()->page());
-    NavigatorContentUtils::from(*navigator.frame()->page())->client()->unregisterProtocolHandler(scheme, baseURL, KURL(ParsedURLString, url));
+    KURL baseURL = document->baseURL();

[tkent, 2014/09/22 09:53:14]

Ditto.

[pals, 2014/09/22 11:37:53]

Done.

+    NavigatorContentUtils::from(*navigator.frame()->page())->client()->unregisterProtocolHandler(scheme, document->completeURL(url));
 }
 
 const char* NavigatorContentUtils::supplementName()