[Chromoting] Add DataChannelManager to manage optional incoming data channels

remoting/protocol/fake_message_pipe.h

+// Copyright 2017 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_FAKE_MESSAGE_PIPE_H_
+#define REMOTING_PROTOCOL_FAKE_MESSAGE_PIPE_H_
+
+#include <memory>
+
+#include "remoting/protocol/message_pipe.h"
+
+namespace google {
+namespace protobuf {
+class MessageLite;
+}  // namespace protobuf
+}  // namespace google
+
+namespace remoting {
+namespace protocol {
+
+class FakeMessagePipeWrapper;
+
+class FakeMessagePipe final : public MessagePipe {
+ public:
+  // Sets |asynchronous| to true to make all operations asynchronizedly.

[joedow, 2017/05/30 16:24:18]

I don't think this comment is needed.  The param name is self-explanatory.

[Hzj_jie, 2017/05/31 00:11:53]

Done.

+  explicit FakeMessagePipe(bool asynchronous);
+  ~FakeMessagePipe() override;
+
+  // Creates an std::unique_ptr<FakeMessagePipeWrapper> instance to wrap |this|.
+  // All operations will be forwarded to |this|.
+  std::unique_ptr<FakeMessagePipeWrapper> Wrap();

[joedow, 2017/05/30 16:24:18]

Why is this needed?  Someone who has access to this instance already knows its
address and can use it as needed.  Also, returning a std_unique_ptr<> implies
transferring ownership, but this method can be called multiple times which means
double freeing is a concern.

[Hzj_jie, 2017/05/31 00:11:53]

It returns an FakeMessagePipeWrapper which forwards all the operations to the
original FakeMessagePipe instance. It's safe to call this function multiple
times because no freeing happens to this instance. The reason of
FakeMessagePipeWrapper is because DataChannelHandler takes ownership of
MessagePipe.

+
+  // MessagePipe implementation.
+  void Start(EventHandler* event_handler) override;
+  void Send(google::protobuf::MessageLite* message,
+            const base::Closure& done) override;
+
+  // Triggers EventHandler::OnMessageReceived().

[joedow, 2017/05/30 16:24:18]

I would remove the impl detail comments since they can change and it is easy to
forget to update the header comments since they are divorced from the impl
itself.  Instead write comments based on what the function's responsibilities
are.

[Hzj_jie, 2017/05/31 00:11:53]

Done.

+  void Receive(std::unique_ptr<CompoundBuffer> message);
+
+  // Triggers EventHandler::OnMessagePipeOpen().
+  void OpenPipe();
+
+  // Triggers EventHandler::OnMessagePipeClosed();
+  void ClosePipe();
+
+ private:
+  const bool asynchronous_;
+  bool pipe_opened_ = false;
+  EventHandler* event_handler_ = nullptr;
+};
+
+}  // namespace protocol
+}  // namespace remoting
+
+#endif  // REMOTING_PROTOCOL_FAKE_MESSAGE_PIPE_H_