Simplify WebrtcDataStreamAdapter

remoting/protocol/webrtc_data_stream_adapter.h

 // Copyright 2015 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 REMOTING_PROTOCOL_WEBRTC_DATA_STREAM_ADAPTER_H_
 #define REMOTING_PROTOCOL_WEBRTC_DATA_STREAM_ADAPTER_H_
 
 #include <memory>
 #include <string>
 
 #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::DataChannelInterface> channel);
+  ~WebrtcDataStreamAdapter() override;
 
-  explicit WebrtcDataStreamAdapter(
-      rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_connection);
-  ~WebrtcDataStreamAdapter();
+  std::string name() { return channel_->label(); }
 
-  // Creates outgoing data channel.
-  std::unique_ptr<MessagePipe> CreateOutgoingChannel(const std::string& name);
-
-  // 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);
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
 #endif  // REMOTING_PROTOCOL_WEBRTC_DATA_STREAM_ADAPTER_H_