[MediaRouter] Replace RouteMessage with PresentationConnectionMessage

chrome/browser/media/cast_remoting_connector.cc

 // Copyright 2016 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/cast_remoting_connector.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/media/cast_remoting_connector_messaging.h"
 #include "chrome/browser/media/cast_remoting_sender.h"
 #include "chrome/browser/media/router/media_router.h"
 #include "chrome/browser/media/router/media_router_factory.h"
 #include "chrome/browser/media/router/route_message_observer.h"
 #include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/media_router/media_source_helper.h"
-#include "chrome/common/media_router/route_message.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 
 using content::BrowserThread;
 using media::mojom::RemotingSinkCapabilities;
 using media::mojom::RemotingStartFailReason;
 using media::mojom::RemotingStopReason;
@@ skipping 84 matching lines @@
     : public media_router::RouteMessageObserver {
  public:
   MessageObserver(media_router::MediaRouter* router,
                   const media_router::MediaRoute::Id& route_id,
                   CastRemotingConnector* connector)
       : RouteMessageObserver(router, route_id), connector_(connector) {}
   ~MessageObserver() final {}
 
  private:
   void OnMessagesReceived(
-      const std::vector<media_router::RouteMessage>& messages) final {
+      const std::vector<content::PresentationConnectionMessage>& messages)
+      final {
     connector_->ProcessMessagesFromRoute(messages);
   }
 
   CastRemotingConnector* const connector_;
 };
 
 // static
 const void* const CastRemotingConnector::kUserDataKey = &kUserDataKey;
 
 // static
@@ skipping 221 matching lines @@
     struct Helper {
       static void IgnoreSendMessageResult(bool ignored) {}
     };
     media_router::MediaRoutesObserver::router()->SendRouteMessage(
         message_observer_->route_id(), message,
         base::Bind(&Helper::IgnoreSendMessageResult));
   }
 }
 
 void CastRemotingConnector::ProcessMessagesFromRoute(
-    const std::vector<media_router::RouteMessage>& messages) {
+    const std::vector<content::PresentationConnectionMessage>& messages) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // Note: If any calls to message parsing functions are added/changed here,
   // please update cast_remoting_connector_fuzzertest.cc as well!
 
-  for (const media_router::RouteMessage& message : messages) {
-    switch (message.type) {
-      case media_router::RouteMessage::TEXT:
-        // This is a notification message from the Cast Provider, about the
-        // execution state of the media remoting session between Chrome and the
-        // remote device.
-        DCHECK(message.text);
+  for (const auto& message : messages) {
+    if (message.data) {

[mark a. foltz, 2017/06/28 07:29:54]

message.is_binary()

[imcheng, 2017/06/29 08:21:08]

Done.

+      // All binary messages are passed through to the source during an active
+      // remoting session.
+      if (active_bridge_)
+        active_bridge_->OnMessageFromSink(*message.data);
 
-        // If this is a "start streams" acknowledgement message, the
-        // CastRemotingSenders should now be available to begin consuming from
-        // the data pipes.
-        if (active_bridge_ &&
-            Messaging::IsMessageForSession(
-                *message.text,
-                Messaging::kStartedStreamsMessageFormatPartial,
-                session_counter_)) {
-          if (pending_audio_sender_request_.is_pending()) {
-            cast::CastRemotingSender::FindAndBind(
-                Messaging::GetStreamIdFromStartedMessage(
-                    *message.text,
-                    Messaging::kStartedStreamsMessageAudioIdSpecifier),
-                std::move(pending_audio_pipe_),
-                std::move(pending_audio_sender_request_),
-                base::Bind(&CastRemotingConnector::OnDataSendFailed,
-                           weak_factory_.GetWeakPtr()));
-          }
-          if (pending_video_sender_request_.is_pending()) {
-            cast::CastRemotingSender::FindAndBind(
-                Messaging::GetStreamIdFromStartedMessage(
-                    *message.text,
-                    Messaging::kStartedStreamsMessageVideoIdSpecifier),
-                std::move(pending_video_pipe_),
-                std::move(pending_video_sender_request_),
-                base::Bind(&CastRemotingConnector::OnDataSendFailed,
-                           weak_factory_.GetWeakPtr()));
-          }
-          break;
-        }
+      continue;
+    }
 
-        // If this is a failure message, call StopRemoting().
-        if (active_bridge_ &&
-            Messaging::IsMessageForSession(*message.text,
-                                           Messaging::kFailedMessageFormat,
-                                           session_counter_)) {
-          StopRemoting(active_bridge_, RemotingStopReason::UNEXPECTED_FAILURE);
-          break;
-        }
+    DCHECK(message.message);

[mark a. foltz, 2017/06/28 07:29:53]

If is_binary && !active_bridge_ then this DCHECK will fail.  Intentional?

[imcheng, 2017/06/29 08:21:08]

If is_binary is true then we won't reach here due to the continue; .

+    // This is a notification message from the Cast Provider, about the
+    // execution state of the media remoting session between Chrome and the
+    // remote device.
 
-        // If this is a stop acknowledgement message, indicating that the last
-        // session was stopped, notify all sources that the sink is once again
-        // available.
-        if (Messaging::IsMessageForSession(*message.text,
-                                           Messaging::kStoppedMessageFormat,
-                                           session_counter_)) {
-          if (active_bridge_) {
-            // Hmm...The Cast Provider was in a state that disagrees with this
-            // connector. Attempt to resolve this by shutting everything down to
-            // effectively reset to a known state.
-            LOG(WARNING) << "BUG: Cast Provider sent 'stopped' message during "
-                            "an active remoting session.";
-            StopRemoting(active_bridge_,
-                         RemotingStopReason::UNEXPECTED_FAILURE);
-          }
-          for (RemotingBridge* notifyee : bridges_)
-            notifyee->OnSinkAvailable(enabled_features_);
-          break;
-        }
-
-        LOG(WARNING) << "BUG: Unexpected message from Cast Provider: "
-                     << *message.text;
-        break;
-
-      case media_router::RouteMessage::BINARY:  // This is for the source.
-        DCHECK(message.binary);
-
-        // All binary messages are passed through to the source during an active
-        // remoting session.
-        if (active_bridge_)
-          active_bridge_->OnMessageFromSink(*message.binary);
-        break;
+    // If this is a "start streams" acknowledgement message, the
+    // CastRemotingSenders should now be available to begin consuming from
+    // the data pipes.
+    if (active_bridge_ &&

[mark a. foltz, 2017/06/28 07:29:54]

This might be easier to follow if there was an early return at the beginning for
!active_bridge_.  Optional to fix

[imcheng, 2017/06/29 08:21:08]

That might be a bit challenging to extract since there is some logic for
!active_bridge_ that's dependent on other conditions (L436-437). I will leave it
to xjz@ / miu@ to determine what's the best way to reorganize the logic here if
needed.

+        Messaging::IsMessageForSession(
+            *message.message, Messaging::kStartedStreamsMessageFormatPartial,
+            session_counter_)) {
+      if (pending_audio_sender_request_.is_pending()) {
+        cast::CastRemotingSender::FindAndBind(
+            Messaging::GetStreamIdFromStartedMessage(
+                *message.message,
+                Messaging::kStartedStreamsMessageAudioIdSpecifier),
+            std::move(pending_audio_pipe_),
+            std::move(pending_audio_sender_request_),
+            base::Bind(&CastRemotingConnector::OnDataSendFailed,
+                       weak_factory_.GetWeakPtr()));
+      }
+      if (pending_video_sender_request_.is_pending()) {
+        cast::CastRemotingSender::FindAndBind(
+            Messaging::GetStreamIdFromStartedMessage(
+                *message.message,
+                Messaging::kStartedStreamsMessageVideoIdSpecifier),
+            std::move(pending_video_pipe_),
+            std::move(pending_video_sender_request_),
+            base::Bind(&CastRemotingConnector::OnDataSendFailed,
+                       weak_factory_.GetWeakPtr()));
+      }
+    } else if (active_bridge_ &&
+               Messaging::IsMessageForSession(*message.message,
+                                              Messaging::kFailedMessageFormat,
+                                              session_counter_)) {
+      // If this is a failure message, call StopRemoting().
+      StopRemoting(active_bridge_, RemotingStopReason::UNEXPECTED_FAILURE);
+    } else if (Messaging::IsMessageForSession(*message.message,
+                                              Messaging::kStoppedMessageFormat,
+                                              session_counter_)) {
+      // If this is a stop acknowledgement message, indicating that the last
+      // session was stopped, notify all sources that the sink is once again
+      // available.
+      if (active_bridge_) {
+        // Hmm...The Cast Provider was in a state that disagrees with this
+        // connector. Attempt to resolve this by shutting everything down to
+        // effectively reset to a known state.
+        LOG(WARNING) << "BUG: Cast Provider sent 'stopped' message during "
+                        "an active remoting session.";
+        StopRemoting(active_bridge_, RemotingStopReason::UNEXPECTED_FAILURE);
+      }
+      for (RemotingBridge* notifyee : bridges_)
+        notifyee->OnSinkAvailable(enabled_features_);
+    } else {
+      LOG(WARNING) << "BUG: Unexpected message from Cast Provider: "
+                   << *message.message;
     }
   }
 }
 
 void CastRemotingConnector::HandleSendMessageResult(bool success) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // A single message send failure is treated as fatal to an active remoting
   // session.
   if (!success && active_bridge_)
     StopRemoting(active_bridge_, RemotingStopReason::MESSAGE_SEND_FAILED);
@@ skipping 41 matching lines @@
         media_router::MediaRoutesObserver::router(), route.media_route_id(),
         this));
     // TODO(miu): In the future, scan the route ID for sink capabilities
     // properties and pass these to the source in the OnSinkAvailable()
     // notification.
     for (RemotingBridge* notifyee : bridges_)
       notifyee->OnSinkAvailable(enabled_features_);
     break;
   }
 }