Convert MediaRouter mojom apis to intake url::Origin objects instead of strings

chrome/browser/media/android/router/media_router_android.cc

Index: chrome/browser/media/android/router/media_router_android.cc
diff --git a/chrome/browser/media/android/router/media_router_android.cc b/chrome/browser/media/android/router/media_router_android.cc
index 9fa7aa64f3a124c44ee96637a4077b5033f4ddf9..7ca05b48f64efe5af9329675028dbfaa0e97bc90 100644
--- a/chrome/browser/media/android/router/media_router_android.cc
+++ b/chrome/browser/media/android/router/media_router_android.cc
@@ -75,20 +75,12 @@ const MediaRoute* MediaRouterAndroid::FindRouteBySource(
 void MediaRouterAndroid::CreateRoute(
     const MediaSource::Id& source_id,
     const MediaSink::Id& sink_id,
-    const GURL& origin,
+    const url::Origin& origin,
     content::WebContents* web_contents,
     const std::vector<MediaRouteResponseCallback>& callbacks,
     base::TimeDelta timeout,
     bool incognito) {
   // TODO(avayvod): Implement timeouts (crbug.com/583036).
-  if (!origin.is_valid()) {
-    std::unique_ptr<RouteRequestResult> result = RouteRequestResult::FromError(
-        "Invalid origin", RouteRequestResult::INVALID_ORIGIN);
-    for (const MediaRouteResponseCallback& callback : callbacks)
-      callback.Run(*result);
-    return;
-  }
-
   std::string presentation_id = MediaRouterBase::CreatePresentationId();
 
   int tab_id = -1;
@@ -112,7 +104,7 @@ void MediaRouterAndroid::CreateRoute(
   ScopedJavaLocalRef<jstring> jpresentation_id =
           base::android::ConvertUTF8ToJavaString(env, presentation_id);
   ScopedJavaLocalRef<jstring> jorigin =
-          base::android::ConvertUTF8ToJavaString(env, origin.spec());
+      base::android::ConvertUTF8ToJavaString(env, origin.GetURL().spec());

[mark a. foltz, 2017/01/24 04:04:17]

Is it necessary to convert origin to a GURL, or can you just call .serialize()
here?
If I read the spec correctly, they should be equivalent.

https://w3c.github.io/webappsec-suborigins/#serializing

[steimel, 2017/01/25 19:01:10]

Tested a few scenarios, and looks like they're not quite equivalent:
1) for a unique origin, Serialize() returns "null", but GetURL().spec() gives ""
2) for a file origin, Serialize() returns "file://", but GetURL().spec() gives
"file:///"
3) for an origin such as "http://google.com", Serialize() returns
"http://google.com", but GetURL().spec() returns "http://google.com/"

I doubt 2 and 3 matter, but I could see 1 being an issue. Thoughts?

[mark a. foltz, 2017/01/25 21:13:04]

If case #1 returns the string "null" then things should just work.  However it
would be safer to keep the current code and fix the real issue described below
separately.

I think the current code is actually a bug in the Cast MRP implementation, since
unique origins are all mapped to "" and end up being considered "same-origin"
for the purposes of the SDK.

Filed crbug.com/685358; can you add a TODO(crbug.com/XXXX) here?

[steimel, 2017/01/25 22:25:26]

Done.

 
   Java_ChromeMediaRouter_createRoute(env, java_media_router_, jsource_id,
                                      jsink_id, jpresentation_id, jorigin,
@@ -122,7 +114,7 @@ void MediaRouterAndroid::CreateRoute(
 void MediaRouterAndroid::ConnectRouteByRouteId(
     const MediaSource::Id& source,
     const MediaRoute::Id& route_id,
-    const GURL& origin,
+    const url::Origin& origin,
     content::WebContents* web_contents,
     const std::vector<MediaRouteResponseCallback>& callbacks,
     base::TimeDelta timeout,
@@ -133,20 +125,12 @@ void MediaRouterAndroid::ConnectRouteByRouteId(
 void MediaRouterAndroid::JoinRoute(
     const MediaSource::Id& source_id,
     const std::string& presentation_id,
-    const GURL& origin,
+    const url::Origin& origin,
     content::WebContents* web_contents,
     const std::vector<MediaRouteResponseCallback>& callbacks,
     base::TimeDelta timeout,
     bool incognito) {
   // TODO(avayvod): Implement timeouts (crbug.com/583036).
-  if (!origin.is_valid()) {
-    std::unique_ptr<RouteRequestResult> result = RouteRequestResult::FromError(
-        "Invalid origin", RouteRequestResult::INVALID_ORIGIN);
-    for (const MediaRouteResponseCallback& callback : callbacks)
-      callback.Run(*result);
-    return;
-  }
-
   int tab_id = -1;
   TabAndroid* tab = web_contents
       ? TabAndroid::FromWebContents(web_contents) : nullptr;
@@ -154,7 +138,7 @@ void MediaRouterAndroid::JoinRoute(
     tab_id = tab->GetAndroidId();
 
   DVLOG(2) << "JoinRoute: " << source_id << ", " << presentation_id << ", "
-           << origin.spec() << ", " << tab_id;
+           << origin.GetURL().spec() << ", " << tab_id;
 
   int request_id = route_requests_.Add(base::MakeUnique<MediaRouteRequest>(
       MediaSource(source_id), presentation_id, callbacks));
@@ -165,7 +149,7 @@ void MediaRouterAndroid::JoinRoute(
   ScopedJavaLocalRef<jstring> jpresentation_id =
           base::android::ConvertUTF8ToJavaString(env, presentation_id);
   ScopedJavaLocalRef<jstring> jorigin =
-          base::android::ConvertUTF8ToJavaString(env, origin.spec());
+      base::android::ConvertUTF8ToJavaString(env, origin.GetURL().spec());
 
   Java_ChromeMediaRouter_joinRoute(env, java_media_router_, jsource_id,
                                    jpresentation_id, jorigin, tab_id,
@@ -348,7 +332,7 @@ void MediaRouterAndroid::OnSinksReceived(
   if (it != sinks_observers_.end()) {
     // TODO(imcheng): Pass origins to OnSinksUpdated (crbug.com/594858).
     for (auto& observer : *it->second)
-      observer.OnSinksUpdated(sinks_converted, std::vector<GURL>());
+      observer.OnSinksUpdated(sinks_converted, std::vector<url::Origin>());
   }
 }