Gets presentation ID from route ID upon route creation, and overrides previous presentation ID with this final ID.

This is a quick fix to get presentation ID to web app before we overhaul route/sink/source URN format.

With https://codereview.chromium.org/1199933006/, https://codereview.chromium.org/1209473003/ and this CL, send/receive message works end to end.

Review URL: https://codereview.chromium.org/1202963004

Cr-Commit-Position: refs/heads/master@{#336326}

10 files changed, 103 insertions, 29 deletions

diff --git a/chrome/browser/media/router/create_session_request.cc b/chrome/browser/media/router/create_session_request.cc
index faed210..c9516cd 100644
--- a/chrome/browser/media/router/create_session_request.cc
+++ b/chrome/browser/media/router/create_session_request.cc
@@ -31,7 +31,11 @@ CreateSessionRequest::~CreateSessionRequest() {
 void CreateSessionRequest::MaybeInvokeSuccessCallback(
     const MediaRoute::Id& route_id) {
   if (!cb_invoked_) {
-    success_cb_.Run(presentation_info_, route_id);
+    // Overwrite presentation ID.
+    success_cb_.Run(content::PresentationSessionInfo(
+                        presentation_info_.presentation_url,
+                        GetPresentationIdAndUrl(route_id).first),
+                    route_id);
     cb_invoked_ = true;
   }
 }
diff --git a/chrome/browser/media/router/create_session_request_unittest.cc b/chrome/browser/media/router/create_session_request_unittest.cc
index 8e8590b..322beac 100644
--- a/chrome/browser/media/router/create_session_request_unittest.cc
+++ b/chrome/browser/media/router/create_session_request_unittest.cc
@@ -12,8 +12,10 @@ namespace media_router {
 
 namespace {
 
-const char kPresentationUrl[] = "http://fooUrl";
+const char kPresentationUrl[] = "http://foo.com";
 const char kPresentationId[] = "presentationId";
+const char kRouteId[] =
+    "urn:x-org.chromium:media:route:presentationId/cast-sink1/http://foo.com";
 
 }  // namespace
 
@@ -79,7 +81,7 @@ TEST_F(CreateSessionRequestTest, SuccessCallback) {
                  session_info),
       base::Bind(&CreateSessionRequestTest::FailOnError,
                  base::Unretained(this)));
-  context.MaybeInvokeSuccessCallback("routeid");
+  context.MaybeInvokeSuccessCallback(kRouteId);
   // No-op since success callback is already invoked.
   context.MaybeInvokeErrorCallback(content::PresentationError(
       content::PRESENTATION_ERROR_NO_AVAILABLE_SCREENS, "Error message"));
@@ -101,7 +103,7 @@ TEST_F(CreateSessionRequestTest, ErrorCallback) {
                  error));
   context.MaybeInvokeErrorCallback(error);
   // No-op since error callback is already invoked.
-  context.MaybeInvokeSuccessCallback("routeid");
+  context.MaybeInvokeSuccessCallback(kRouteId);
   EXPECT_TRUE(cb_invoked_);
 }
 
diff --git a/chrome/browser/media/router/media_route.cc b/chrome/browser/media/router/media_route.cc
index 0aa126f..3defca4 100644
--- a/chrome/browser/media/router/media_route.cc
+++ b/chrome/browser/media/router/media_route.cc
@@ -7,6 +7,10 @@
 #include "base/logging.h"
 #include "chrome/browser/media/router/media_source.h"
 
+namespace {
+const char kRouteUrnPrefix[] = "urn:x-org.chromium:media:route:";
+}
+
 namespace media_router {
 
 MediaRoute::MediaRoute(const MediaRoute::Id& media_route_id,
@@ -29,4 +33,37 @@ bool MediaRoute::Equals(const MediaRoute& other) const {
   return media_route_id_ == other.media_route_id_;
 }
 
+// <route-id> =
+//  urn:x-org.chromium:media:route:<presentation-id>/<sink>/<source>
+// <source> = <url>|<capture-source>
+// <sink> = <provider-name>-<sink-id>
+std::pair<std::string, std::string> GetPresentationIdAndUrl(
+    const MediaRoute::Id& id) {
+  if (id.find(kRouteUrnPrefix) != 0) {
+    LOG(ERROR) << "Invalid media route ID. Expecting prefix "
+               << kRouteUrnPrefix;
+    return std::make_pair(std::string(), std::string());
+  }
+  size_t prefix_len = arraysize(kRouteUrnPrefix) - 1;
+  size_t first_delim = id.find("/");
+  if (first_delim == std::string::npos || first_delim == prefix_len) {
+    LOG(ERROR) << "Invalid media route ID. Expecting presentation ID.";
+    return std::make_pair(std::string(), std::string());
+  }
+
+  std::string presentation_id = id.substr(prefix_len, first_delim - prefix_len);
+  size_t second_delim = id.find("/", first_delim + 1);
+  if (second_delim == std::string::npos || second_delim == first_delim + 1) {
+    LOG(ERROR) << "Invalid media route ID. Expecting sink.";
+    return std::make_pair(std::string(), std::string());
+  }
+
+  if (second_delim == id.size() - 1) {
+    LOG(ERROR) << "Invalid media route ID. Expecting source.";
+    return std::make_pair(std::string(), std::string());
+  }
+  std::string source = id.substr(second_delim + 1);
+  return std::make_pair(presentation_id, source);
+}
+
 }  // namespace media_router
diff --git a/chrome/browser/media/router/media_route.h b/chrome/browser/media/router/media_route.h
index e8d270a..8e47ee3 100644
--- a/chrome/browser/media/router/media_route.h
+++ b/chrome/browser/media/router/media_route.h
@@ -79,6 +79,11 @@ class MediaRoute {
   MediaRouteState state_;
 };
 
+// Return a pair of Presentation ID and URL. If the input route ID is invalid,
+// a pair of empty strings is returned.
+std::pair<std::string, std::string> GetPresentationIdAndUrl(
+    const MediaRoute::Id& id);
+
 }  // namespace media_router
 
 #endif  // CHROME_BROWSER_MEDIA_ROUTER_MEDIA_ROUTE_H_
diff --git a/chrome/browser/media/router/media_route_unittest.cc b/chrome/browser/media/router/media_route_unittest.cc
index e52b075..31f221d 100644
--- a/chrome/browser/media/router/media_route_unittest.cc
+++ b/chrome/browser/media/router/media_route_unittest.cc
@@ -7,34 +7,56 @@
 #include "chrome/browser/media/router/media_source_helper.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+namespace {
+const char kRouteId1[] =
+    "urn:x-org.chromium:media:route:1/cast-sink1/http://foo.com";
+const char kRouteId2[] =
+    "urn:x-org.chromium:media:route:2/cast-sink2/http://foo.com";
+}  // namespace
+
 namespace media_router {
 
 // Tests the == operator to ensure that only route ID equality is being checked.
 TEST(MediaRouteTest, Equals) {
-  MediaRoute route1("routeId1", MediaSourceForCastApp("DialApp"),
+  MediaRoute route1(kRouteId1, MediaSourceForCastApp("DialApp"),
                     MediaSink("sinkId", "sinkName"), "Description", false);
 
   // Same as route1 with different sink ID.
-  MediaRoute route2("routeId1", MediaSourceForCastApp("DialApp"),
+  MediaRoute route2(kRouteId1, MediaSourceForCastApp("DialApp"),
                     MediaSink("differentSinkId", "different sink"),
                     "Description", false);
   EXPECT_TRUE(route1.Equals(route2));
 
   // Same as route1 with different description.
-  MediaRoute route3("routeId1", MediaSourceForCastApp("DialApp"),
+  MediaRoute route3(kRouteId1, MediaSourceForCastApp("DialApp"),
                     MediaSink("sinkId", "sinkName"), "differentDescription",
                     false);
   EXPECT_TRUE(route1.Equals(route3));
 
   // Same as route1 with different is_local.
-  MediaRoute route4("routeId1", MediaSourceForCastApp("DialApp"),
+  MediaRoute route4(kRouteId1, MediaSourceForCastApp("DialApp"),
                     MediaSink("sinkId", "sinkName"), "Description", true);
   EXPECT_TRUE(route1.Equals(route4));
 
   // The ID is different from route1's.
-  MediaRoute route5("routeId2", MediaSourceForCastApp("DialApp"),
+  MediaRoute route5(kRouteId2, MediaSourceForCastApp("DialApp"),
                     MediaSink("sinkId", "sinkName"), "Description", false);
   EXPECT_FALSE(route1.Equals(route5));
 }
 
+TEST(MediaRouteTest, ParseId) {
+  EXPECT_EQ("1", GetPresentationIdAndUrl(kRouteId1).first);
+  EXPECT_EQ("http://foo.com", GetPresentationIdAndUrl(kRouteId1).second);
+  auto invalid = std::make_pair(std::string(), std::string());
+  EXPECT_EQ(invalid, GetPresentationIdAndUrl("invalid"));
+  EXPECT_EQ(invalid, GetPresentationIdAndUrl(
+                         "urn:x-org.chromium:media:route:2"));
+  EXPECT_EQ(invalid, GetPresentationIdAndUrl(
+                         "urn:x-org.chromium:media:route:2/"));
+  EXPECT_EQ(invalid, GetPresentationIdAndUrl(
+                         "urn:x-org.chromium:media:route:2/cast-sink2/"));
+  EXPECT_EQ(invalid, GetPresentationIdAndUrl(
+                         "urn:x-org.chromium:media:route:2//http://foo.com"));
+}
+
 }  // namespace media_router
diff --git a/chrome/browser/media/router/media_router_mojo_impl.cc b/chrome/browser/media/router/media_router_mojo_impl.cc
index ffadab0..b1b2e05 100644
--- a/chrome/browser/media/router/media_router_mojo_impl.cc
+++ b/chrome/browser/media/router/media_router_mojo_impl.cc
@@ -49,9 +49,9 @@ void EventPageWakeComplete(bool success) {
 scoped_ptr<content::PresentationSessionMessage>
 ConvertToPresentationSessionMessage(interfaces::RouteMessagePtr input) {
   DCHECK(!input.is_null());
-  // TODO(haibinlu): get presentation_url&id from route_id
-  std::string presentation_url;
-  std::string presentation_id;
+  const auto& id_and_url = GetPresentationIdAndUrl(input->route_id);
+  const std::string& presentation_id = id_and_url.first;
+  const std::string& presentation_url = id_and_url.second;
   scoped_ptr<content::PresentationSessionMessage> output;
   switch (input->type) {
     case interfaces::RouteMessage::Type::TYPE_TEXT: {
diff --git a/chrome/browser/media/router/presentation_service_delegate_impl.cc b/chrome/browser/media/router/presentation_service_delegate_impl.cc
index 46649ae..d49c6f1 100644
--- a/chrome/browser/media/router/presentation_service_delegate_impl.cc
+++ b/chrome/browser/media/router/presentation_service_delegate_impl.cc
@@ -666,9 +666,8 @@ void PresentationServiceDelegateImpl::OnRouteCreated(const MediaRoute& route) {
   if (!main_frame)
     return;
   RenderFrameHostId render_frame_host_id(GetRenderFrameHostId(main_frame));
-  // TODO(imcheng): Pass in valid default presentation ID once it is
-  // available from MediaRoute URN. BUG=493365
-  std::string presentation_id;
+  std::string presentation_id =
+      GetPresentationIdAndUrl(route.media_route_id()).first;
   frame_manager_->OnPresentationSessionStarted(
       render_frame_host_id, true,
       content::PresentationSessionInfo(PresentationUrlFromMediaSource(source),
diff --git a/content/browser/presentation/presentation_service_impl.cc b/content/browser/presentation/presentation_service_impl.cc
index f387097..6546c60 100644
--- a/content/browser/presentation/presentation_service_impl.cc
+++ b/content/browser/presentation/presentation_service_impl.cc
@@ -457,18 +457,23 @@ void PresentationServiceImpl::ListenForSessionMessages(
 
 void PresentationServiceImpl::OnSessionMessages(
     scoped_ptr<ScopedVector<PresentationSessionMessage>> messages) {
-  DCHECK(messages.get() && !messages->empty());
   if (!on_session_messages_callback_.get()) {
     // The Reset method of this class was invoked.
     return;
   }
 
-  mojo::Array<presentation::SessionMessagePtr> mojoMessages(messages->size());
-  for (size_t i = 0; i < messages->size(); ++i) {
-    mojoMessages[i] = ToMojoSessionMessage((*messages)[i]);
+  if (!messages.get() || messages->empty()) {
+    // Error handling. Session is either closed or in error state.
+    on_session_messages_callback_->Run(
+        mojo::Array<presentation::SessionMessagePtr>());
+  } else {
+    mojo::Array<presentation::SessionMessagePtr> mojoMessages(messages->size());
+    for (size_t i = 0; i < messages->size(); ++i) {
+      mojoMessages[i] = ToMojoSessionMessage((*messages)[i]);
+    }
+    on_session_messages_callback_->Run(mojoMessages.Pass());
   }
 
-  on_session_messages_callback_->Run(mojoMessages.Pass());
   on_session_messages_callback_.reset();
 }
 
diff --git a/content/renderer/presentation/presentation_dispatcher.cc b/content/renderer/presentation/presentation_dispatcher.cc
index 8100c03..2390967 100644
--- a/content/renderer/presentation/presentation_dispatcher.cc
+++ b/content/renderer/presentation/presentation_dispatcher.cc
@@ -344,6 +344,9 @@ void PresentationDispatcher::OnSessionCreated(
 
   DCHECK(!session_info.is_null());
   callback->onSuccess(new PresentationSessionClient(session_info.Pass()));
+  presentation_service_->ListenForSessionMessages(
+      base::Bind(&PresentationDispatcher::OnSessionMessagesReceived,
+                 base::Unretained(this)));
 }
 
 void PresentationDispatcher::OnSessionStateChange(
@@ -404,9 +407,6 @@ void PresentationDispatcher::ConnectToPresentationServiceIfNeeded() {
   presentation_service_->ListenForSessionStateChange(base::Bind(
       &PresentationDispatcher::OnSessionStateChange,
       base::Unretained(this)));
-  presentation_service_->ListenForSessionMessages(
-      base::Bind(&PresentationDispatcher::OnSessionMessagesReceived,
-      base::Unretained(this)));
   */
 }
 
diff --git a/extensions/renderer/resources/media_router_bindings.js b/extensions/renderer/resources/media_router_bindings.js
index ade2ee8..eb0d355 100644
--- a/extensions/renderer/resources/media_router_bindings.js
+++ b/extensions/renderer/resources/media_router_bindings.js
@@ -63,13 +63,13 @@ define('media_router_bindings', [
     if ("string" == typeof message.message) {
       return new mediaRouterMojom.RouteMessage({
         'route_id': message.routeId,
-        'type': RouteMessage.Type.TEXT,
+        'type': mediaRouterMojom.RouteMessage.Type.TEXT,
         'message': message.message,
       });
     } else {
       return new mediaRouterMojom.RouteMessage({
         'route_id': message.routeId,
-        'type': RouteMessage.Type.BINARY,
+        'type': mediaRouterMojom.RouteMessage.Type.BINARY,
         'data': message.message,
       });
     }
@@ -470,7 +470,7 @@ define('media_router_bindings', [
    */
   MediaRouteProvider.prototype.sendRouteMessage = function(
       routeId, message) {
-    this.handlers_.sendRouteMessage(routeId, message)
+    return this.handlers_.sendRouteMessage(routeId, message)
         .then(function() {
           return true;
         }, function() {
@@ -485,11 +485,11 @@ define('media_router_bindings', [
    *    an empty list if an error occurred.
    */
   MediaRouteProvider.prototype.listenForRouteMessages = function(routeIds) {
-    this.handlers_.listenForRouteMessages(routeIds)
+    return this.handlers_.listenForRouteMessages(routeIds)
         .then(function(messages) {
-          return messages.map(messageToMojo_);
+          return {'messages': messages.map(messageToMojo_)};
         }, function() {
-          return [];
+          return {'messages': []};
         });
   };