[cast_channel] Make CastMessageHandler a CastSocket::Observer instead of CastTransport::Delegate

extensions/browser/api/cast_channel/cast_channel_api.cc

Index: extensions/browser/api/cast_channel/cast_channel_api.cc
diff --git a/extensions/browser/api/cast_channel/cast_channel_api.cc b/extensions/browser/api/cast_channel/cast_channel_api.cc
index bdbea3316955c9ad8b0f6ecfab1826e53c05c2f9..a984a8c82d7807cd6a0001e47ab3ddfbc0c08262 100644
--- a/extensions/browser/api/cast_channel/cast_channel_api.cc
+++ b/extensions/browser/api/cast_channel/cast_channel_api.cc
@@ -294,14 +294,15 @@ void CastChannelOpenFunction::AsyncWorkStart() {
   }
   new_channel_id_ = cast_socket_service_->AddSocket(base::WrapUnique(socket));
 
+  if (!socket->HasObserver(extension_->id())) {

[imcheng, 2017/06/16 23:39:25]

Since we are thinking about sharing CastSocket across profiles -- I don't think
this will work across profiles, since their MR extensions have the same
extension id.

We could try the following, but feel free to suggest other ideas:
- We could move CastMessageHandler into CastChannelAPI and have CastChannelAPI
manage CastMessageHandler. This means CastMessageHandler has an extension id and
is also tied to a Profile.
- However, because CastMessageHandler works in the IO thread, we may have to
make it thread-safe.
- Ideally, CastMessageHandler should be able to unregister itself from a
CastSocket when it is about to be destroyed. To do this, we need to guarantee
CastSocket outlives all observers, or we will need to make CastSocket support
WeakPtr.

[zhaobin, 2017/06/20 00:54:36]

Doing it a little differently.

1. Create one observer instance for each extension and pass ownership to
CastSocketService.
2. For each newly created socket, get observer ptr for |extension_id| and
register it with socket (does not pass ownership)

CastSocketService owns CastSockets and Observers. It can make sure CastSockets
are destroyed before Observers.
3. 

+    socket->AddObserver(extension_->id(),

[mark a. foltz, 2017/06/16 23:26:14]

I think this code should only run once per extension_id and |socket|?
Maybe you don't need to register the observer with an extension_id then, since
it will be bound in the callback.

[zhaobin, 2017/06/20 00:54:36]

Yes.

Done.

+                        base::MakeUnique<CastMessageHandler>(
+                            base::Bind(&CastChannelAPI::SendEvent,
+                                       api_->AsWeakPtr(), extension_->id()),
+                            api_->GetLogger()));
+  }
   // Construct read delegates.
-  std::unique_ptr<CastTransport::Delegate> delegate(
-      base::MakeUnique<CastMessageHandler>(
-          base::Bind(&CastChannelAPI::SendEvent, api_->AsWeakPtr(),
-                     extension_->id()),
-          socket, api_->GetLogger()));
-  socket->Connect(std::move(delegate),
-                  base::Bind(&CastChannelOpenFunction::OnOpen, this));
+  socket->Connect(base::Bind(&CastChannelOpenFunction::OnOpen, this));
 }
 
 void CastChannelOpenFunction::OnOpen(ChannelError result) {
@@ -432,10 +433,8 @@ void CastChannelCloseFunction::OnClose(int result) {
 
 CastChannelOpenFunction::CastMessageHandler::CastMessageHandler(
     const EventDispatchCallback& ui_dispatch_cb,
-    CastSocket* socket,
     scoped_refptr<Logger> logger)
-    : ui_dispatch_cb_(ui_dispatch_cb), socket_(socket), logger_(logger) {
-  DCHECK(socket_);
+    : ui_dispatch_cb_(ui_dispatch_cb), logger_(logger) {
   DCHECK(logger_);
 }
 
@@ -443,14 +442,15 @@ CastChannelOpenFunction::CastMessageHandler::~CastMessageHandler() {
 }
 
 void CastChannelOpenFunction::CastMessageHandler::OnError(
+    const cast_channel::CastSocket& socket,
     ChannelError error_state) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   ChannelInfo channel_info;
-  FillChannelInfo(*socket_, &channel_info);
+  FillChannelInfo(socket, &channel_info);
   channel_info.error_state = api::cast_channel::ToChannelError(error_state);
   ErrorInfo error_info;
-  FillErrorInfo(channel_info.error_state, logger_->GetLastError(socket_->id()),
+  FillErrorInfo(channel_info.error_state, logger_->GetLastError(socket.id()),
                 &error_info);
 
   std::unique_ptr<base::ListValue> results =
@@ -463,13 +463,14 @@ void CastChannelOpenFunction::CastMessageHandler::OnError(
 }
 
 void CastChannelOpenFunction::CastMessageHandler::OnMessage(
+    const cast_channel::CastSocket& socket,
     const CastMessage& message) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   MessageInfo message_info;
   CastMessageToMessageInfo(message, &message_info);
   ChannelInfo channel_info;
-  FillChannelInfo(*socket_, &channel_info);
+  FillChannelInfo(socket, &channel_info);
   VLOG(1) << "Received message " << ParamToString(message_info)
           << " on channel " << ParamToString(channel_info);
 
@@ -483,7 +484,4 @@ void CastChannelOpenFunction::CastMessageHandler::OnMessage(
       base::Bind(ui_dispatch_cb_, base::Passed(std::move(event))));
 }
 
-void CastChannelOpenFunction::CastMessageHandler::Start() {
-}
-
 }  // namespace extensions