Simplify WebrtcDataStreamAdapter

remoting/protocol/webrtc_data_stream_adapter.h

Index: remoting/protocol/webrtc_data_stream_adapter.h
diff --git a/remoting/protocol/webrtc_data_stream_adapter.h b/remoting/protocol/webrtc_data_stream_adapter.h
index ad1c1b4fbf2434bc15099d690ee9215f1540410b..bb2a2278208d6f8ecc4d7635b74395c7b915b7af 100644
--- a/remoting/protocol/webrtc_data_stream_adapter.h
+++ b/remoting/protocol/webrtc_data_stream_adapter.h
@@ -10,36 +10,44 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "remoting/protocol/errors.h"
-#include "remoting/protocol/message_channel_factory.h"
+#include "remoting/protocol/message_pipe.h"
 #include "third_party/webrtc/api/peerconnectioninterface.h"
 #include "third_party/webrtc/base/refcount.h"
 
-namespace rtc {
-class PeerConnectionInterface;
-}  // namespace rtc
-
 namespace remoting {
 namespace protocol {
 
-// WebrtcDataStreamAdapter wraps MessagePipe for WebRTC data channels.
-class WebrtcDataStreamAdapter {
+// WebrtcDataStreamAdapter implements MessagePipe for WebRTC data channels.
+class WebrtcDataStreamAdapter : public MessagePipe,
+                                public webrtc::DataChannelObserver {
  public:
-  typedef base::Callback<void(ErrorCode)> ErrorCallback;
-
   explicit WebrtcDataStreamAdapter(
-      rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection);
-  ~WebrtcDataStreamAdapter();
+      rtc::scoped_refptr<webrtc::DataChannelInterface> channel);
+  ~WebrtcDataStreamAdapter() override;
 
-  // Creates outgoing data channel.
-  std::unique_ptr<MessagePipe> CreateOutgoingChannel(const std::string& name);
+  std::string name() { return channel_->label(); }
 
-  // Creates incoming data channel.
-  std::unique_ptr<MessagePipe> WrapIncomingDataChannel(
-      rtc::scoped_refptr<webrtc::DataChannelInterface> data_channel);
+  // MessagePipe interface.
+  void Start(EventHandler* event_handler) override;
+  void Send(google::protobuf::MessageLite* message,
+            const base::Closure& done) override;
 
  private:
-  rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection_;
+  enum class State { CONNECTING, OPEN, CLOSED };
+
+  // webrtc::DataChannelObserver interface.
+  void OnStateChange() override;
+  void OnMessage(const webrtc::DataBuffer& buffer) override;

[Jamie, 2016/07/25 18:25:59]

These are public in the DataChannelObserver interface, so I'm not sure it makes
much sense to declare them private here. The fact that you don't want these to
be public might indicate that inheritance is not the right model here, and you'd
be better off with a private observer as a class member; I suspect that's
overkill in this situation, but something to consider (I'd suggest private
inheritance, but I think that's disallowed).

[Sergey Ulanov, 2016/07/25 18:48:19]

Putting this interface in private section makes it clear that it's an internal
interface implementation and is not supposed to be called directly. This is
allowed by style guide and the same style is used in many other places in
chromium (and remoting). Adding a private observer class would make this code
more complicated without any benefits.
There was a thread in chromium-dev about it:
https://groups.google.com/a/chromium.org/d/topic/chromium-dev/CwzjcWd9cYM/dis...

+
+  void OnConnected();
+
+  void OnClosed();
+
+  rtc::scoped_refptr<webrtc::DataChannelInterface> channel_;
+
+  EventHandler* event_handler_ = nullptr;
+
+  State state_ = State::CONNECTING;
 
   DISALLOW_COPY_AND_ASSIGN(WebrtcDataStreamAdapter);
 };