[Chromoting] Add DataChannelManager to manage optional incoming data channels

remoting/protocol/fake_message_pipe.cc

+// 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.
+
+#include "remoting/protocol/fake_message_pipe.h"
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/memory/ptr_util.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "remoting/base/compound_buffer.h"
+#include "remoting/protocol/fake_message_pipe_wrapper.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace remoting {
+namespace protocol {
+
+FakeMessagePipe::FakeMessagePipe(bool asynchronous)
+    : asynchronous_(asynchronous) {}
+
+FakeMessagePipe::~FakeMessagePipe() = default;
+
+std::unique_ptr<FakeMessagePipeWrapper> FakeMessagePipe::Wrap() {
+  return base::MakeUnique<FakeMessagePipeWrapper>(this);
+}
+
+void FakeMessagePipe::Start(EventHandler* event_handler) {
+  ASSERT_TRUE(event_handler_ == nullptr);

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

You should avoid adding ASSERT statements in classes like this and opt for using
them inside the test itself.  Pushing asserts into the classes can make
debugging tests more difficult and logs less readable.

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

But this class itself is for testing purpose only.

+  ASSERT_TRUE(event_handler != nullptr);
+  event_handler_ = event_handler;
+}
+
+void FakeMessagePipe::Send(google::protobuf::MessageLite* message,
+                           const base::Closure& done) {
+  ASSERT_TRUE(pipe_opened_);
+  if (asynchronous_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, done);
+    return;
+  }
+  done.Run();
+}
+
+void FakeMessagePipe::Receive(std::unique_ptr<CompoundBuffer> message) {
+  ASSERT_TRUE(pipe_opened_);
+  ASSERT_TRUE(event_handler_ != nullptr);
+  if (asynchronous_) {

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

Can you move the duplicated logic (sync vs. async) into a separate method.  It
is very easy to update the logic for one and not the other given how this is
laid out (same with the other methods below).

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

Done.

+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::Bind(
+        [](FakeMessagePipe* me,
+          std::unique_ptr<CompoundBuffer> message) {
+          ASSERT_TRUE(me->pipe_opened_);
+          ASSERT_TRUE(me->event_handler_ != nullptr);
+          if (message) {
+            message->Lock();
+          }
+          me->event_handler_->OnMessageReceived(std::move(message));
+        },
+        base::Unretained(this),
+        base::Passed(std::move(message))));
+    return;
+  }
+
+  if (message) {
+    message->Lock();
+  }
+  event_handler_->OnMessageReceived(std::move(message));
+}
+
+void FakeMessagePipe::OpenPipe() {
+  ASSERT_FALSE(pipe_opened_);
+  ASSERT_TRUE(event_handler_ != nullptr);
+  if (asynchronous_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::Bind(
+        [](FakeMessagePipe* me) {
+          ASSERT_FALSE(me->pipe_opened_);
+          ASSERT_TRUE(me->event_handler_ != nullptr);
+          me->pipe_opened_ = true;
+          me->event_handler_->OnMessagePipeOpen();
+        },
+        base::Unretained(this)));
+    return;
+  }
+
+  pipe_opened_ = true;
+  event_handler_->OnMessagePipeOpen();
+}
+
+void FakeMessagePipe::ClosePipe() {
+  ASSERT_TRUE(pipe_opened_);
+  ASSERT_TRUE(event_handler_ != nullptr);
+  if (asynchronous_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::Bind(
+        [](FakeMessagePipe* me) {
+          ASSERT_TRUE(me->pipe_opened_);
+          ASSERT_TRUE(me->event_handler_ != nullptr);
+          me->pipe_opened_ = false;
+          me->event_handler_->OnMessagePipeClosed();
+        },
+        base::Unretained(this)));
+    return;
+  }
+
+  pipe_opened_ = false;
+  event_handler_->OnMessagePipeClosed();
+}
+
+}  // namespace protocol
+}  // namespace remoting