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

extensions/browser/api/cast_channel/cast_channel_api.cc

 // Copyright 2014 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 "extensions/browser/api/cast_channel/cast_channel_api.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 276 matching lines @@
         *ip_endpoint_, ExtensionsBrowserClient::Get()->GetNetLog(),
         base::TimeDelta::FromMilliseconds(connect_info.timeout.get()
                                               ? *connect_info.timeout
                                               : kDefaultConnectTimeoutMillis),
         liveness_timeout_, ping_interval_, api_->GetLogger(),
         connect_info.capabilities.get() ? *connect_info.capabilities
                                         : CastDeviceCapability::NONE);
   }
   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) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   VLOG(1) << "Connect finished, OnOpen invoked.";
   // TODO: If we failed to open the CastSocket, we may want to clean up here,
   // rather than relying on the extension to call close(). This can be done by
   // calling RemoveSocket() and api_->GetLogger()->ClearLastError(channel_id).
   if (result != ChannelError::UNKNOWN) {
     CastSocket* socket = cast_socket_service_->GetSocket(new_channel_id_);
@@ skipping 110 matching lines @@
     SetResultFromSocket(*socket);
     // This will delete |socket|.
     cast_socket_service_->RemoveSocket(channel_id);
     api_->GetLogger()->ClearLastError(channel_id);
   }
   AsyncWorkCompleted();
 }
 
 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_);
 }
 
 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 =
       OnError::Create(channel_info, error_info);
   std::unique_ptr<Event> event(new Event(
       events::CAST_CHANNEL_ON_ERROR, OnError::kEventName, std::move(results)));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(ui_dispatch_cb_, base::Passed(std::move(event))));
 }
 
 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);
 
   std::unique_ptr<base::ListValue> results =
       OnMessage::Create(channel_info, message_info);
   std::unique_ptr<Event> event(new Event(events::CAST_CHANNEL_ON_MESSAGE,
                                          OnMessage::kEventName,
                                          std::move(results)));
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(ui_dispatch_cb_, base::Passed(std::move(event))));
 }
 
-void CastChannelOpenFunction::CastMessageHandler::Start() {
-}
-
 }  // namespace extensions