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

components/cast_channel/cast_socket.h

 // 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.
 
 #ifndef COMPONENTS_CAST_CHANNEL_CAST_SOCKET_H_
 #define COMPONENTS_CAST_CHANNEL_CAST_SOCKET_H_
 
 #include <stdint.h>
 
 #include <queue>
@@ skipping 37 matching lines @@
   VIDEO_OUT = 1 << 0,
   VIDEO_IN = 1 << 1,
   AUDIO_OUT = 1 << 2,
   AUDIO_IN = 1 << 3,
   DEV_MODE = 1 << 4
 };
 
 // Public interface of the CastSocket class.
 class CastSocket {
  public:
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Invoked when an error occurs on |socket|.
+    virtual void OnError(const CastSocket& socket,
+                         cast_channel::ChannelError error_state) = 0;
+
+    // Invoked when |socket| receives a message.
+    virtual void OnMessage(const CastSocket& socket,
+                           const cast_channel::CastMessage& message) = 0;
+  };
+
   virtual ~CastSocket() {}
 
   // Used by BrowserContextKeyedAPIFactory.
   static const char* service_name() { return "CastSocketImplManager"; }
 
   // Connects the channel to the peer. If successful, the channel will be in
   // READY_STATE_OPEN.  DO NOT delete the CastSocket object in |callback|.
   // Instead use Close().
   // |callback| will be invoked with any ChannelError that occurred, or
   // CHANNEL_ERROR_NONE if successful.
   // If the CastSocket is destroyed while the connection is pending, |callback|
   // will be invoked with CHANNEL_ERROR_UNKNOWN. In this case, invoking
   // |callback| must not result in any re-entrancy behavior.
-  // |delegate| receives message receipt and error events.
   // Ownership of |delegate| is transferred to this CastSocket.
-  virtual void Connect(std::unique_ptr<CastTransport::Delegate> delegate,
-                       base::Callback<void(ChannelError)> callback) = 0;
+  virtual void Connect(base::Callback<void(ChannelError)> callback) = 0;
 
   // Closes the channel if not already closed. On completion, the channel will
   // be in READY_STATE_CLOSED.
   //
   // It is fine to delete this object in |callback|.
   virtual void Close(const net::CompletionCallback& callback) = 0;
 
   // The IP endpoint for the destination of the channel.
   virtual const net::IPEndPoint& ip_endpoint() const = 0;
 
@@ skipping 20 matching lines @@
   // Marks a socket as invalid due to an error, and sends an OnError
   // event to |delegate_|.
   // The OnError event receipient is responsible for closing the socket in the
   // event of an error.
   // Setting the error state does not close the socket if it is open.
   virtual void SetErrorState(ChannelError error_state) = 0;
 
   // Returns a pointer to the socket's message transport layer. Can be used to
   // send and receive CastMessages over the socket.
   virtual CastTransport* transport() const = 0;
+
+  // Returns true if current socket has an observer with |id|.
+  virtual bool HasObserver(const std::string& id) = 0;

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

I'm not sure if we need to associate an id with an observer. This method, for
instance, can just take a Observer*.

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

Done.

+
+  // Register |observer| with current socket to receive message receipt
+  // and error events.
+  virtual void AddObserver(const std::string& id,
+                           std::unique_ptr<Observer> observer) = 0;

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

It's a bit unusual to take ownership of observers... There's no way to remove
observers, and CastSocket does not know when the observer is no longer valid,
resulting in wasted work (CastMessageHandler::*) and memory leaks
(CastMessageHandlers created in Profiles that no longer exist won't get deleted
until the CastSocket is deleted)

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

Make CastSocketService own observers.

 };
 
 // This class implements a channel between Chrome and a Cast device using a TCP
 // socket with SSL.  The channel may authenticate that the receiver is a genuine
 // Cast device.  All CastSocketImpl objects must be used only on the IO thread.
 //
 // NOTE: Not called "CastChannel" to reduce confusion with the generated API
 // code.
 class CastSocketImpl : public CastSocket {
  public:
@@ skipping 21 matching lines @@
                  base::TimeDelta liveness_timeout,
                  base::TimeDelta ping_interval,
                  const scoped_refptr<Logger>& logger,
                  uint64_t device_capabilities,
                  const AuthContext& auth_context);
 
   // Ensures that the socket is closed.
   ~CastSocketImpl() override;
 
   // CastSocket interface.
-  void Connect(std::unique_ptr<CastTransport::Delegate> delegate,
-               base::Callback<void(ChannelError)> callback) override;
+  void Connect(base::Callback<void(ChannelError)> callback) override;
   CastTransport* transport() const override;
   void Close(const net::CompletionCallback& callback) override;
   const net::IPEndPoint& ip_endpoint() const override;
   int id() const override;
   void set_id(int channel_id) override;
   ReadyState ready_state() const override;
   ChannelError error_state() const override;
   bool keep_alive() const override;
   bool audio_only() const override;
+  bool HasObserver(const std::string& id) override;
+  void AddObserver(const std::string& id,
+                   std::unique_ptr<Observer> observer) override;
 
  protected:
   // CastTransport::Delegate methods for receiving handshake messages.
   class AuthTransportDelegate : public CastTransport::Delegate {
    public:
     explicit AuthTransportDelegate(CastSocketImpl* socket);
 
     // Gets the error state of the channel.
     // Returns CHANNEL_ERROR_NONE if no errors are present.
     ChannelError error_state() const;
 
     // Gets recorded error details.
     LastError last_error() const;
 
     // CastTransport::Delegate interface.
     void OnError(ChannelError error_state) override;
     void OnMessage(const CastMessage& message) override;
     void Start() override;
 
    private:
     CastSocketImpl* socket_;
     ChannelError error_state_;
     LastError last_error_;
   };
 
+  // CastTransport::Delegate methods to receive normal messages and errors.
+  class CastSocketMessageDelegate : public CastTransport::Delegate {
+   public:
+    CastSocketMessageDelegate(cast_channel::CastSocketImpl* socket);
+    ~CastSocketMessageDelegate() override;
+
+    // CastTransport::Delegate implementation.
+    void OnError(cast_channel::ChannelError error_state) override;
+    void OnMessage(const cast_channel::CastMessage& message) override;
+    void Start() override;
+
+   private:
+    CastSocketImpl* const socket_;
+    DISALLOW_COPY_AND_ASSIGN(CastSocketMessageDelegate);
+  };
+
   // Replaces the internally-constructed transport object with one provided
   // by the caller (e.g. a mock).
   void SetTransportForTesting(std::unique_ptr<CastTransport> transport);
 
   // Verifies whether the socket complies with cast channel policy.
   // Audio only channel policy mandates that a device declaring a video out
   // capability must not have a certificate with audio only policy.
   bool VerifyChannelPolicy(const AuthResult& result);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestConnectAuthMessageCorrupted);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest,
                            TestConnectChallengeReplyReceiveError);
   FRIEND_TEST_ALL_PREFIXES(CastSocketTest,
                            TestConnectChallengeVerificationFails);
+  FRIEND_TEST_ALL_PREFIXES(CastSocketTest, TestObservers);
   friend class AuthTransportDelegate;
+  friend class CastSocketMessageDelegate;
   friend class CastSocketTest;
   friend class TestCastSocket;
 
   void SetErrorState(ChannelError error_state) override;
 
   // Frees resources and cancels pending callbacks.  |ready_state_| will be set
   // READY_STATE_CLOSED on completion.  A no-op if |ready_state_| is already
   // READY_STATE_CLOSED.
   void CloseInternal();
 
@@ skipping 152 matching lines @@
   // Cast message formatting and parsing layer.
   std::unique_ptr<CastTransport> transport_;
 
   // Caller's message read and error handling delegate.
   std::unique_ptr<CastTransport::Delegate> delegate_;
 
   // Raw pointer to the auth handshake delegate. Used to get detailed error
   // information.
   AuthTransportDelegate* auth_delegate_;
 
+  // Map of CastSocket::Observer keyed by observer id. For extension side
+  // observers, id is extension_id; For browser side observers, id is a hard
+  // coded string.
+  std::map<std::string, std::unique_ptr<Observer>> observer_map_;

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

Would a base::ObserverList work here?  I.e. can something keep track that
exactly one Observer is added for the browser, and one per extension?

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

Done.

+
   DISALLOW_COPY_AND_ASSIGN(CastSocketImpl);
 };
 }  // namespace cast_channel
 
 #endif  // COMPONENTS_CAST_CHANNEL_CAST_SOCKET_H_