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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/media/android/router/media_router_android.h"
 
 #include <string>
 #include <utility>
 #include <vector>
 
@@ skipping 57 matching lines @@
   for (const auto& route : active_routes_) {
     if (route.media_source().id() == source_id)
       return &route;
   }
   return nullptr;
 }
 
 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;
   TabAndroid* tab = web_contents
       ? TabAndroid::FromWebContents(web_contents) : nullptr;
   if (tab)
     tab_id = tab->GetAndroidId();
 
   bool is_incognito = web_contents
       && web_contents->GetBrowserContext()->IsOffTheRecord();
 
   int route_request_id =
       route_requests_.Add(base::MakeUnique<MediaRouteRequest>(
           MediaSource(source_id), presentation_id, callbacks));
 
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jsource_id =
           base::android::ConvertUTF8ToJavaString(env, source_id);
   ScopedJavaLocalRef<jstring> jsink_id =
           base::android::ConvertUTF8ToJavaString(env, sink_id);
   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,
                                      tab_id, is_incognito, route_request_id);
 }
 
 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,
     bool incognito) {
   NOTIMPLEMENTED();
 }
 
 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;
   if (tab)
     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));
 
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jsource_id =
           base::android::ConvertUTF8ToJavaString(env, source_id);
   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,
                                    request_id);
 }
 
 void MediaRouterAndroid::TerminateRoute(const MediaRoute::Id& route_id) {
   JNIEnv* env = base::android::AttachCurrentThread();
   ScopedJavaLocalRef<jstring> jroute_id =
           base::android::ConvertUTF8ToJavaString(env, route_id);
@@ skipping 162 matching lines @@
         MediaSink(ConvertJavaStringToUTF8(env, jsink_urn.obj()),
         ConvertJavaStringToUTF8(env, jsink_name.obj()),
         MediaSink::GENERIC));
   }
 
   std::string source_urn = ConvertJavaStringToUTF8(env, jsource_urn);
   auto it = sinks_observers_.find(source_urn);
   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>());
   }
 }
 
 void MediaRouterAndroid::OnRouteCreated(
     JNIEnv* env,
     const JavaParamRef<jobject>& obj,
     const JavaParamRef<jstring>& jmedia_route_id,
     const JavaParamRef<jstring>& jsink_id,
     jint jroute_request_id,
     jboolean jis_local) {
@@ skipping 90 matching lines @@
     return;
 
   std::vector<RouteMessage> messages(1);
   messages.front().type = RouteMessage::TEXT;
   messages.front().text = ConvertJavaStringToUTF8(env, jmessage);
   for (auto& observer : *it->second.get())
     observer.OnMessagesReceived(messages);
 }
 
 }  // namespace media_router