[Cast MRP] Fixed casting from Google Slides.

The native MRP was assuming mirroring sessions don't send messages
to the client, which is incorrect.  Fixing that assumption involved
moving quite a bit of functionality from CastActivityRecord to its
base class, ActivityRecord.

Bug: 1047834
Change-Id: Ie35b5d518af78a01451fc6e7b0f13be7024d115c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2064813Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: John Williams <jrw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748461}

chrome/browser/media/router/providers/cast/activity_record.cc

@@ -22,6 +22,34 @@ ActivityRecord::ActivityRecord(
 
 ActivityRecord::~ActivityRecord() = default;
 
+mojom::RoutePresentationConnectionPtr ActivityRecord::AddClient(
+    const CastMediaSource& source,
+    const url::Origin& origin,
+    int tab_id) {
+  const std::string& client_id = source.client_id();
+  DCHECK(!base::Contains(connected_clients_, client_id));
+  std::unique_ptr<CastSessionClient> client =
+      client_factory_for_test_
+          ? client_factory_for_test_->MakeClientForTest(client_id, origin,
+                                                        tab_id)
+          : std::make_unique<CastSessionClientImpl>(
+                client_id, origin, tab_id, source.auto_join_policy(), this);
+  auto presentation_connection = client->Init();
+  connected_clients_.emplace(client_id, std::move(client));
+
+  // Route is now local due to connected client.
+  route_.set_local(true);
+  return presentation_connection;
+}
+
+void ActivityRecord::RemoveClient(const std::string& client_id) {
+  // Don't erase by key here as the |client_id| may be referring to the
+  // client being deleted.
+  auto it = connected_clients_.find(client_id);
+  if (it != connected_clients_.end())
+    connected_clients_.erase(it);
+}
+
 CastSession* ActivityRecord::GetSession() const {
   DCHECK(session_id_);
   CastSession* session = session_tracker_->GetSessionById(*session_id_);
@@ -51,18 +79,91 @@ void ActivityRecord::SetOrUpdateSession(const CastSession& session,
 }
 
 void ActivityRecord::SendStopSessionMessageToClients(
-    const std::string& hash_token) {}
+    const std::string& hash_token) {
+  for (const auto& client : connected_clients_) {
+    client.second->SendMessageToClient(
+        CreateReceiverActionStopMessage(client.first, sink_, hash_token));
+  }
+}
 
 void ActivityRecord::SendMessageToClient(
     const std::string& client_id,
-    blink::mojom::PresentationConnectionMessagePtr message) {}
+    blink::mojom::PresentationConnectionMessagePtr message) {
+  auto it = connected_clients_.find(client_id);
+  if (it == connected_clients_.end()) {
+    DLOG(ERROR) << "Attempting to send message to nonexistent client: "
+                << client_id;
+    return;
+  }
+  it->second->SendMessageToClient(std::move(message));
+}
 
 void ActivityRecord::SendMediaStatusToClients(const base::Value& media_status,
-                                              base::Optional<int> request_id) {}
+                                              base::Optional<int> request_id) {
+  for (auto& client : connected_clients_)
+    client.second->SendMediaStatusToClient(media_status, request_id);
+}
 
 void ActivityRecord::ClosePresentationConnections(
-    blink::mojom::PresentationConnectionCloseReason close_reason) {}
+    blink::mojom::PresentationConnectionCloseReason close_reason) {
+  for (auto& client : connected_clients_)
+    client.second->CloseConnection(close_reason);
+}
+
+void ActivityRecord::TerminatePresentationConnections() {
+  for (auto& client : connected_clients_)
+    client.second->TerminateConnection();
+}
+
+base::Optional<int> ActivityRecord::SendMediaRequestToReceiver(
+    const CastInternalMessage& cast_message) {
+  NOTIMPLEMENTED();
+  return base::nullopt;
+}
+
+cast_channel::Result ActivityRecord::SendAppMessageToReceiver(
+    const CastInternalMessage& cast_message) {
+  NOTIMPLEMENTED();
+  return cast_channel::Result::kFailed;
+}
+
+void ActivityRecord::SendSetVolumeRequestToReceiver(
+    const CastInternalMessage& cast_message,
+    cast_channel::ResultCallback callback) {
+  NOTIMPLEMENTED();
+  std::move(callback).Run(cast_channel::Result::kFailed);
+}
+
+void ActivityRecord::StopSessionOnReceiver(
+    const std::string& client_id,
+    cast_channel::ResultCallback callback) {
+  if (!session_id_)
+    std::move(callback).Run(cast_channel::Result::kFailed);
+
+  message_handler_->StopSession(cast_channel_id(), *session_id_, client_id,
+                                std::move(callback));
+}
+
+void ActivityRecord::HandleLeaveSession(const std::string& client_id) {
+  auto client_it = connected_clients_.find(client_id);
+  CHECK(client_it != connected_clients_.end());
+  auto& client = *client_it->second;
+  std::vector<std::string> leaving_client_ids;
+  for (const auto& pair : connected_clients_) {
+    if (pair.second->MatchesAutoJoinPolicy(client.origin(), client.tab_id()))
+      leaving_client_ids.push_back(pair.first);
+  }
+
+  for (const auto& client_id : leaving_client_ids) {
+    auto leaving_client_it = connected_clients_.find(client_id);
+    CHECK(leaving_client_it != connected_clients_.end());
+    leaving_client_it->second->CloseConnection(
+        blink::mojom::PresentationConnectionCloseReason::CLOSED);
+    connected_clients_.erase(leaving_client_it);
+  }
+}
 
-void ActivityRecord::TerminatePresentationConnections() {}
+CastSessionClientFactoryForTest* ActivityRecord::client_factory_for_test_ =
+    nullptr;
 
 }  // namespace media_router

chrome/browser/media/router/providers/cast/activity_record.h

@@ -44,6 +44,16 @@ class ActivityRecord {
   base::Optional<int> mirroring_tab_id() const { return mirroring_tab_id_; }
   const MediaSinkInternal sink() const { return sink_; }
 
+  // Adds a new client |client_id| to this session and returns the handles of
+  // the two pipes to be held by Blink It is invalid to call this method if the
+  // client already exists.
+  virtual mojom::RoutePresentationConnectionPtr AddClient(
+      const CastMediaSource& source,
+      const url::Origin& origin,
+      int tab_id);
+
+  virtual void RemoveClient(const std::string& client_id);
+
   // On the first call, saves the ID of |session|.  On subsequent calls,
   // notifies all connected clients that the session has been updated.  In both
   // cases, the stored route description is updated to match the session
@@ -83,9 +93,53 @@ class ActivityRecord {
       mojo::PendingReceiver<mojom::MediaController> media_controller,
       mojo::PendingRemote<mojom::MediaStatusObserver> observer) = 0;
 
+  // Sends media command |cast_message|, which came from the SDK client, to the
+  // receiver hosting this session. Returns the locally-assigned request ID of
+  // the message sent to the receiver.
+  virtual base::Optional<int> SendMediaRequestToReceiver(
+      const CastInternalMessage& cast_message);
+
+  // Sends app message |cast_message|, which came from the SDK client, to the
+  // receiver hosting this session. Returns true if the message is sent
+  // successfully.
+  virtual cast_channel::Result SendAppMessageToReceiver(
+      const CastInternalMessage& cast_message);
+
+  // Sends a SET_VOLUME request to the receiver and calls |callback| when a
+  // response indicating whether the request succeeded is received.
+  virtual void SendSetVolumeRequestToReceiver(
+      const CastInternalMessage& cast_message,
+      cast_channel::ResultCallback callback);
+
+  // Stops the currently active session on the receiver, and invokes |callback|
+  // with the result. Called when a SDK client requests to stop the session.
+  virtual void StopSessionOnReceiver(const std::string& client_id,
+                                     cast_channel::ResultCallback callback);
+
+  // Called when the client given by |client_id| requests to leave the session.
+  // This will also cause all clients within the session with matching origin
+  // and/or tab ID to leave (i.e., their presentation connections will be
+  // closed).
+  virtual void HandleLeaveSession(const std::string& client_id);
+
+  static void SetClientFactoryForTest(
+      CastSessionClientFactoryForTest* factory) {
+    client_factory_for_test_ = factory;
+  }
+
  protected:
+  using ClientMap =
+      base::flat_map<std::string, std::unique_ptr<CastSessionClient>>;
+
   CastSession* GetSession() const;
 
+  CastSessionClient* GetClient(const std::string& client_id) {
+    auto it = connected_clients_.find(client_id);
+    return it == connected_clients_.end() ? nullptr : it->second.get();
+  }
+
+  int cast_channel_id() const { return sink_.cast_channel_id(); }
+
   MediaRoute route_;
   std::string app_id_;
   base::Optional<int> mirroring_tab_id_;
@@ -104,6 +158,12 @@ class ActivityRecord {
   base::Optional<std::string> session_id_;
 
   MediaSinkInternal sink_;
+  ClientMap connected_clients_;
+
+ private:
+  friend class CastActivityRecordTest;
+
+  static CastSessionClientFactoryForTest* client_factory_for_test_;
 };
 
 }  // namespace media_router

chrome/browser/media/router/providers/cast/cast_activity_manager.cc

@@ -9,7 +9,9 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/json/json_reader.h"
 #include "base/optional.h"
+#include "chrome/browser/media/router/data_decoder_util.h"
 #include "chrome/browser/media/router/providers/cast/cast_activity_record.h"
 #include "chrome/browser/media/router/providers/cast/cast_session_client.h"
 #include "chrome/browser/media/router/providers/cast/mirroring_activity_record.h"
@@ -133,12 +135,11 @@ void CastActivityManager::DoLaunchSession(DoLaunchSessionParams params) {
 
   mojom::RoutePresentationConnectionPtr presentation_connection;
 
-  if (cast_source.ContainsApp(kCastStreamingAppId)) {
-    // Lanch a mirroring session.
-    AddMirroringActivityRecord(route, app_id, params.tab_id, sink.cast_data());
-  } else {
-    // Launch a flinging session.
-    auto* activity_ptr = AddCastActivityRecord(route, app_id);
+  ActivityRecord* activity_ptr =
+      cast_source.ContainsStreamingApp()
+          ? AddMirroringActivityRecord(route, app_id, params.tab_id,
+                                       sink.cast_data())
+          : AddCastActivityRecord(route, app_id);
   const std::string& client_id = cast_source.client_id();
   if (!client_id.empty()) {
     presentation_connection =
@@ -147,7 +148,6 @@ void CastActivityManager::DoLaunchSession(DoLaunchSessionParams params) {
         client_id,
         CreateReceiverActionCastMessage(client_id, sink, hash_token_));
   }
-  }
 
   NotifyAllOnRoutesUpdated();
   base::TimeDelta launch_timeout = cast_source.launch_timeout();
@@ -538,6 +538,54 @@ cast_channel::ResultCallback CastActivityManager::MakeResultCallbackForRoute(
                         std::move(callback));
 }
 
+const MediaRoute* CastActivityManager::FindMirroringRouteForTab(
+    int32_t tab_id) {
+  for (const auto& entry : activities_) {
+    const std::string& route_id = entry.first;
+    const ActivityRecord& activity = *entry.second;
+    if (activity.mirroring_tab_id() == tab_id &&
+        !base::Contains(cast_activities_, route_id)) {
+      return &activity.route();
+    }
+  }
+  return nullptr;
+}
+
+void CastActivityManager::SendRouteMessage(const std::string& media_route_id,
+                                           const std::string& message) {
+  GetDataDecoder().ParseJson(
+      message, base::BindRepeating(&CastActivityManager::SendRouteJsonMessage,
+                                   weak_ptr_factory_.GetWeakPtr(),
+                                   media_route_id, message));
+}
+
+void CastActivityManager::SendRouteJsonMessage(
+    const std::string& media_route_id,
+    const std::string& message,
+    data_decoder::DataDecoder::ValueOrError result) {
+  if (result.error) {
+    DVLOG(1) << "Error parsing JSON data: " << *result.error;
+    return;
+  }
+
+  const std::string* client_id = result.value->FindStringKey("clientId");
+  if (!client_id) {
+    DVLOG(1) << "No client ID in message.";
+    return;
+  }
+
+  const auto it = activities_.find(media_route_id);
+  if (it == activities_.end()) {
+    DVLOG(1) << "No activity for " << media_route_id;
+    return;
+  }
+  ActivityRecord& activity = *it->second;
+
+  auto message_ptr =
+      blink::mojom::PresentationConnectionMessage::NewMessage(message);
+  activity.SendMessageToClient(*client_id, std::move(message_ptr));
+}
+
 void CastActivityManager::AddNonLocalActivityRecord(
     const MediaSinkInternal& sink,
     const CastSession& session) {

chrome/browser/media/router/providers/cast/cast_activity_manager.h

@@ -132,12 +132,21 @@ class CastActivityManager : public CastActivityManagerBase,
       const std::string& route_id,
       mojom::MediaRouteProvider::TerminateRouteCallback callback) override;
 
+  const MediaRoute* FindMirroringRouteForTab(int32_t tab_id);
+
+  void SendRouteMessage(const std::string& media_route_id,
+                        const std::string& message);
+
  private:
   friend class CastActivityManagerTest;
   using ActivityMap =
       base::flat_map<MediaRoute::Id, std::unique_ptr<ActivityRecord>>;
   using CastActivityMap = base::flat_map<MediaRoute::Id, CastActivityRecord*>;
 
+  void SendRouteJsonMessage(const std::string& media_route_id,
+                            const std::string& message,
+                            data_decoder::DataDecoder::ValueOrError result);
+
   // Bundle of parameters for DoLaunchSession().
   struct DoLaunchSessionParams {
     // Note: The compiler-generated constructors and destructor would be
@@ -182,8 +191,8 @@ class CastActivityManager : public CastActivityManagerBase,
 
   void RemoveActivityByRouteId(const std::string& route_id);
 
-  // Removes an activity, terminating any associated connections, then notifies
-  // the media router that routes have been updated.
+  // Removes an activity, terminating any associated connections, then
+  // notifies the media router that routes have been updated.
   void RemoveActivity(
       ActivityMap::iterator activity_it,
       blink::mojom::PresentationConnectionState state,
@@ -243,8 +252,8 @@ class CastActivityManager : public CastActivityManagerBase,
       int tab_id,
       const CastSinkExtraData& cast_data);
 
-  // Returns a sink used to convert a mirroring activity to a cast activity.  If
-  // no conversion should occur, returns base::nullopt.
+  // Returns a sink used to convert a mirroring activity to a cast activity.
+  // If no conversion should occur, returns base::nullopt.
   base::Optional<MediaSinkInternal> ConvertMirrorToCast(int tab_id);
 
   static CastActivityRecordFactoryForTest* activity_record_factory_;
@@ -255,8 +264,8 @@ class CastActivityManager : public CastActivityManagerBase,
   // activities.
   ActivityMap activities_;
 
-  // The values of this map are the subset of those in |activites_| where there
-  // is a CastActivityRecord.
+  // The values of this map are the subset of those in |activites_| where
+  // there is a CastActivityRecord.
   CastActivityMap cast_activities_;
 
   // The following raw pointer fields are assumed to outlive |this|.

chrome/browser/media/router/providers/cast/cast_activity_record.cc

@@ -29,34 +29,6 @@ CastActivityRecord::CastActivityRecord(
 
 CastActivityRecord::~CastActivityRecord() = default;
 
-mojom::RoutePresentationConnectionPtr CastActivityRecord::AddClient(
-    const CastMediaSource& source,
-    const url::Origin& origin,
-    int tab_id) {
-  const std::string& client_id = source.client_id();
-  DCHECK(!base::Contains(connected_clients_, client_id));
-  std::unique_ptr<CastSessionClient> client =
-      client_factory_for_test_
-          ? client_factory_for_test_->MakeClientForTest(client_id, origin,
-                                                        tab_id)
-          : std::make_unique<CastSessionClientImpl>(
-                client_id, origin, tab_id, source.auto_join_policy(), this);
-  auto presentation_connection = client->Init();
-  connected_clients_.emplace(client_id, std::move(client));
-
-  // Route is now local due to connected client.
-  route_.set_local(true);
-  return presentation_connection;
-}
-
-void CastActivityRecord::RemoveClient(const std::string& client_id) {
-  // Don't erase by key here as the |client_id| may be referring to the
-  // client being deleted.
-  auto it = connected_clients_.find(client_id);
-  if (it != connected_clients_.end())
-    connected_clients_.erase(it);
-}
-
 void CastActivityRecord::SetOrUpdateSession(const CastSession& session,
                                             const MediaSinkInternal& sink,
                                             const std::string& hash_token) {
@@ -121,76 +93,14 @@ void CastActivityRecord::SendSetVolumeRequestToReceiver(
       cast_message.client_id(), std::move(callback));
 }
 
-void CastActivityRecord::StopSessionOnReceiver(
-    const std::string& client_id,
-    cast_channel::ResultCallback callback) {
-  if (!session_id_)
-    std::move(callback).Run(cast_channel::Result::kFailed);
-
-  message_handler_->StopSession(cast_channel_id(), *session_id_, client_id,
-                                std::move(callback));
-}
-
-void CastActivityRecord::SendStopSessionMessageToClients(
-    const std::string& hash_token) {
-  // TODO(jrw): Add test for this method.
-  for (const auto& client : connected_clients_) {
-    client.second->SendMessageToClient(
-        CreateReceiverActionStopMessage(client.first, sink_, hash_token));
-  }
-}
-
-void CastActivityRecord::HandleLeaveSession(const std::string& client_id) {
-  auto client_it = connected_clients_.find(client_id);
-  CHECK(client_it != connected_clients_.end());
-  auto& client = *client_it->second;
-  std::vector<std::string> leaving_client_ids;
-  for (const auto& pair : connected_clients_) {
-    if (pair.second->MatchesAutoJoinPolicy(client.origin(), client.tab_id()))
-      leaving_client_ids.push_back(pair.first);
-  }
-
-  for (const auto& client_id : leaving_client_ids) {
-    auto leaving_client_it = connected_clients_.find(client_id);
-    CHECK(leaving_client_it != connected_clients_.end());
-    leaving_client_it->second->CloseConnection(
-        PresentationConnectionCloseReason::CLOSED);
-    connected_clients_.erase(leaving_client_it);
-  }
-}
-
-void CastActivityRecord::SendMessageToClient(
-    const std::string& client_id,
-    PresentationConnectionMessagePtr message) {
-  auto it = connected_clients_.find(client_id);
-  if (it == connected_clients_.end()) {
-    DLOG(ERROR) << "Attempting to send message to nonexistent client: "
-                << client_id;
-    return;
-  }
-  it->second->SendMessageToClient(std::move(message));
-}
-
 void CastActivityRecord::SendMediaStatusToClients(
     const base::Value& media_status,
     base::Optional<int> request_id) {
-  for (auto& client : connected_clients_)
-    client.second->SendMediaStatusToClient(media_status, request_id);
+  ActivityRecord::SendMediaStatusToClients(media_status, request_id);
   if (media_controller_)
     media_controller_->SetMediaStatus(media_status);
 }
 
-void CastActivityRecord::ClosePresentationConnections(
-    PresentationConnectionCloseReason close_reason) {
-  for (auto& client : connected_clients_)
-    client.second->CloseConnection(close_reason);
-}
-
-void CastActivityRecord::TerminatePresentationConnections() {
-  for (auto& client : connected_clients_)
-    client.second->TerminateConnection();
-}
-
 void CastActivityRecord::CreateMediaController(
     mojo::PendingReceiver<mojom::MediaController> media_controller,
     mojo::PendingRemote<mojom::MediaStatusObserver> observer) {
@@ -257,7 +167,4 @@ bool CastActivityRecord::HasJoinableClient(AutoJoinPolicy policy,
                      });
 }
 
-CastSessionClientFactoryForTest* CastActivityRecord::client_factory_for_test_ =
-    nullptr;
-
 }  // namespace media_router

chrome/browser/media/router/providers/cast/cast_activity_record.h

@@ -28,8 +28,6 @@ namespace media_router {
 class CastActivityRecord;
 class CastInternalMessage;
 class CastSession;
-class CastSessionClient;
-class CastSessionClientFactoryForTest;
 class CastSessionTracker;
 class MediaRoute;
 
@@ -42,9 +40,6 @@ class CastActivityRecordFactoryForTest {
 
 class CastActivityRecord : public ActivityRecord {
  public:
-  using ClientMap =
-      base::flat_map<std::string, std::unique_ptr<CastSessionClient>>;
-
   // Creates a new record owned by |owner|.
   CastActivityRecord(const MediaRoute& route,
                      const std::string& app_id,
@@ -53,88 +48,34 @@ class CastActivityRecord : public ActivityRecord {
   ~CastActivityRecord() override;
 
   // ActivityRecord implementation
-  void SendStopSessionMessageToClients(const std::string& hash_token) override;
   void SetOrUpdateSession(const CastSession& session,
                           const MediaSinkInternal& sink,
                           const std::string& hash_token) override;
-  void SendMessageToClient(
-      const std::string& client_id,
-      blink::mojom::PresentationConnectionMessagePtr message) override;
   void SendMediaStatusToClients(const base::Value& media_status,
                                 base::Optional<int> request_id) override;
-  void ClosePresentationConnections(
-      blink::mojom::PresentationConnectionCloseReason close_reason) override;
-  void TerminatePresentationConnections() override;
   void OnAppMessage(const cast::channel::CastMessage& message) override;
   void OnInternalMessage(const cast_channel::InternalMessage& message) override;
   void CreateMediaController(
       mojo::PendingReceiver<mojom::MediaController> media_controller,
       mojo::PendingRemote<mojom::MediaStatusObserver> observer) override;
-
-  // Sends media command |cast_message|, which came from the SDK client, to the
-  // receiver hosting this session. Returns the locally-assigned request ID of
-  // the message sent to the receiver.
-  virtual base::Optional<int> SendMediaRequestToReceiver(
-      const CastInternalMessage& cast_message);
-
-  // Sends app message |cast_message|, which came from the SDK client, to the
-  // receiver hosting this session. Returns true if the message is sent
-  // successfully.
-  virtual cast_channel::Result SendAppMessageToReceiver(
-      const CastInternalMessage& cast_message);
-
-  // Sends a SET_VOLUME request to the receiver and calls |callback| when a
-  // response indicating whether the request succeeded is received.
-  virtual void SendSetVolumeRequestToReceiver(
+  base::Optional<int> SendMediaRequestToReceiver(
+      const CastInternalMessage& cast_message) override;
+  cast_channel::Result SendAppMessageToReceiver(
+      const CastInternalMessage& cast_message) override;
+  void SendSetVolumeRequestToReceiver(
       const CastInternalMessage& cast_message,
-      cast_channel::ResultCallback callback);
-
-  // Stops the currently active session on the receiver, and invokes |callback|
-  // with the result. Called when a SDK client requests to stop the session.
-  virtual void StopSessionOnReceiver(const std::string& client_id,
-                                     cast_channel::ResultCallback callback);
-
-  // Called when the client given by |client_id| requests to leave the session.
-  // This will also cause all clients within the session with matching origin
-  // and/or tab ID to leave (i.e., their presentation connections will be
-  // closed).
-  virtual void HandleLeaveSession(const std::string& client_id);
-
-  // Adds a new client |client_id| to this session and returns the handles of
-  // the two pipes to be held by Blink It is invalid to call this method if the
-  // client already exists.
-  virtual mojom::RoutePresentationConnectionPtr AddClient(
-      const CastMediaSource& source,
-      const url::Origin& origin,
-      int tab_id);
-
-  virtual void RemoveClient(const std::string& client_id);
+      cast_channel::ResultCallback callback) override;
 
   bool CanJoinSession(const CastMediaSource& cast_source, bool incognito) const;
   bool HasJoinableClient(AutoJoinPolicy policy,
                          const url::Origin& origin,
                          int tab_id) const;
 
-  static void SetClientFactoryForTest(
-      CastSessionClientFactoryForTest* factory) {
-    client_factory_for_test_ = factory;
-  }
-
  private:
   friend class CastSessionClientImpl;
   friend class CastActivityManager;
   friend class CastActivityRecordTest;
 
-  static CastSessionClientFactoryForTest* client_factory_for_test_;
-
-  int cast_channel_id() const { return sink_.cast_channel_id(); }
-
-  CastSessionClient* GetClient(const std::string& client_id) {
-    auto it = connected_clients_.find(client_id);
-    return it == connected_clients_.end() ? nullptr : it->second.get();
-  }
-
-  ClientMap connected_clients_;
   std::unique_ptr<CastMediaController> media_controller_;
 };
 

chrome/browser/media/router/providers/cast/cast_media_route_provider.cc

@@ -98,11 +98,14 @@ void CastMediaRouteProvider::CreateRoute(const std::string& source_id,
                                          base::TimeDelta timeout,
                                          bool incognito,
                                          CreateRouteCallback callback) {
+  DVLOG(2) << "CreateRoute with origin: " << origin << " and tab ID " << tab_id;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
   // TODO(https://crbug.com/809249): Handle mirroring routes, including
   // mirror-to-Cast transitions.
   const MediaSinkInternal* sink = media_sink_service_->GetSinkById(sink_id);
   if (!sink) {
+    DVLOG(2) << "CreateRoute: sink not found";
     std::move(callback).Run(base::nullopt, nullptr,
                             std::string("Sink not found"),
                             RouteRequestResult::ResultCode::SINK_NOT_FOUND);
@@ -112,6 +115,7 @@ void CastMediaRouteProvider::CreateRoute(const std::string& source_id,
   std::unique_ptr<CastMediaSource> cast_source =
       CastMediaSource::FromMediaSourceId(source_id);
   if (!cast_source) {
+    DVLOG(2) << "CreateRoute: invalid source";
     std::move(callback).Run(
         base::nullopt, nullptr, std::string("Invalid source"),
         RouteRequestResult::ResultCode::NO_SUPPORTED_PROVIDER);
@@ -167,13 +171,13 @@ void CastMediaRouteProvider::TerminateRoute(const std::string& route_id,
 
 void CastMediaRouteProvider::SendRouteMessage(const std::string& media_route_id,
                                               const std::string& message) {
-  NOTIMPLEMENTED();
+  activity_manager_->SendRouteMessage(media_route_id, message);
 }
 
 void CastMediaRouteProvider::SendRouteBinaryMessage(
     const std::string& media_route_id,
     const std::vector<uint8_t>& data) {
-  NOTIMPLEMENTED();
+  NOTREACHED() << "Binary messages are not supported for Cast routes.";
 }
 
 void CastMediaRouteProvider::StartObservingMediaSinks(

chrome/browser/media/router/providers/cast/cast_session_client.cc

@@ -40,7 +40,7 @@ CastSessionClientImpl::CastSessionClientImpl(const std::string& client_id,
                                              const url::Origin& origin,
                                              int tab_id,
                                              AutoJoinPolicy auto_join_policy,
-                                             CastActivityRecord* activity)
+                                             ActivityRecord* activity)
     : CastSessionClient(client_id, origin, tab_id),
       auto_join_policy_(auto_join_policy),
       activity_(activity) {}
@@ -184,10 +184,14 @@ void CastSessionClientImpl::HandleParsedClientMessage(
       break;
 
     default:
-      // TODO(jrw): Log string value of type instead of int value.
-      DLOG(ERROR) << "Unhandled message type: "
+      auto opt_string = cast_util::EnumToString(cast_message->type());
+      if (opt_string) {
+        DLOG(ERROR) << "Unhandled message type: " << *opt_string;
+      } else {
+        DLOG(ERROR) << "Invalid message type: "
                     << static_cast<int>(cast_message->type());
       }
+  }
 }
 
 void CastSessionClientImpl::HandleV2ProtocolMessage(

chrome/browser/media/router/providers/cast/cast_session_client.h

@@ -25,7 +25,7 @@
 
 namespace media_router {
 
-class CastActivityRecord;
+class ActivityRecord;
 
 // Represents a Cast SDK client connection to a Cast session. This class
 // contains PresentationConnection Mojo pipes to send and receive messages
@@ -117,7 +117,7 @@ class CastSessionClientImpl : public CastSessionClient,
                         const url::Origin& origin,
                         int tab_id,
                         AutoJoinPolicy auto_join_policy,
-                        CastActivityRecord* activity);
+                        ActivityRecord* activity);
   ~CastSessionClientImpl() override;
 
   // CastSessionClient implementation
@@ -165,7 +165,7 @@ class CastSessionClientImpl : public CastSessionClient,
 
   const AutoJoinPolicy auto_join_policy_;
 
-  CastActivityRecord* const activity_;
+  ActivityRecord* const activity_;
 
   // The maximum number of pending media requests, used to prevent memory leaks.
   // Normally the number of pending requests should be fairly small, but each