Remove ChannelManager::AddChannel().

mojo/edk/system/channel_manager_unittest.cc

 // 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.
 
 #include "mojo/edk/system/channel_manager.h"
 
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/run_loop.h"
 #include "base/task_runner.h"
 #include "base/test/test_timeouts.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
 #include "base/time/time.h"
 #include "mojo/edk/embedder/platform_channel_pair.h"
 #include "mojo/edk/embedder/simple_platform_support.h"
 #include "mojo/edk/system/channel.h"
+#include "mojo/edk/system/channel_endpoint.h"
+#include "mojo/edk/system/message_pipe_dispatcher.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo {
 namespace system {
 namespace {
 
 class ChannelManagerTest : public testing::Test {
  public:
   ChannelManagerTest() : message_loop_(base::MessageLoop::TYPE_IO) {}
   ~ChannelManagerTest() override {}
 
  protected:
   embedder::SimplePlatformSupport* platform_support() {
     return &platform_support_;
   }
   base::MessageLoop* message_loop() { return &message_loop_; }
 
  private:
   embedder::SimplePlatformSupport platform_support_;
   base::MessageLoop message_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(ChannelManagerTest);
 };
 
 TEST_F(ChannelManagerTest, Basic) {
   ChannelManager cm(platform_support());
 
-  // Hang on to a ref to the |Channel|, so that we can check that the
-  // |ChannelManager| takes/releases refs to it.
-  scoped_refptr<Channel> ch(new Channel(platform_support()));
-  ASSERT_TRUE(ch->HasOneRef());
+  embedder::PlatformChannelPair channel_pair;
 
-  embedder::PlatformChannelPair channel_pair;
-  ch->Init(RawChannel::Create(channel_pair.PassServerHandle()));
+  scoped_refptr<ChannelEndpoint> cep;

[yzshen1, 2015/02/04 17:26:39]

nit: naming variables like this is probably disfavored by our style guide.

+  scoped_refptr<MessagePipeDispatcher> d =
+      MessagePipeDispatcher::CreateRemoteMessagePipe(&cep);
+  const ChannelId id = 1;
+  cm.CreateChannelOnIOThread(id, channel_pair.PassServerHandle(), cep);
+  cep = nullptr;
 
-  const ChannelId id = 1;
-  cm.AddChannel(id, ch, base::MessageLoopProxy::current());
-  // |ChannelManager| should take a ref.
+  scoped_refptr<Channel> ch = cm.GetChannel(id);
+  EXPECT_TRUE(ch);
+  // |ChannelManager| should have a ref.
   EXPECT_FALSE(ch->HasOneRef());
 
   cm.WillShutdownChannel(id);
   // |ChannelManager| should still have a ref.
   EXPECT_FALSE(ch->HasOneRef());
 
   cm.ShutdownChannel(id);
   // On the "I/O" thread, so shutdown should happen synchronously.
   // |ChannelManager| should have given up its ref.
   EXPECT_TRUE(ch->HasOneRef());
+
+  EXPECT_EQ(MOJO_RESULT_OK, d->Close());
 }
 
 TEST_F(ChannelManagerTest, TwoChannels) {
   ChannelManager cm(platform_support());
 
-  // Hang on to a ref to each |Channel|, so that we can check that the
-  // |ChannelManager| takes/releases refs to them.
-  scoped_refptr<Channel> ch1(new Channel(platform_support()));
-  ASSERT_TRUE(ch1->HasOneRef());
-  scoped_refptr<Channel> ch2(new Channel(platform_support()));
-  ASSERT_TRUE(ch2->HasOneRef());
+  embedder::PlatformChannelPair channel_pair;
 
-  embedder::PlatformChannelPair channel_pair;
-  ch1->Init(RawChannel::Create(channel_pair.PassServerHandle()));
-  ch2->Init(RawChannel::Create(channel_pair.PassClientHandle()));
+  scoped_refptr<ChannelEndpoint> cep1;
+  scoped_refptr<MessagePipeDispatcher> d1 =
+      MessagePipeDispatcher::CreateRemoteMessagePipe(&cep1);
+  const ChannelId id1 = 1;
+  cm.CreateChannelOnIOThread(id1, channel_pair.PassServerHandle(), cep1);
+  cep1 = nullptr;
 
-  const ChannelId id1 = 1;
-  cm.AddChannel(id1, ch1, base::MessageLoopProxy::current());
-  EXPECT_FALSE(ch1->HasOneRef());
+  scoped_refptr<ChannelEndpoint> cep2;
+  scoped_refptr<MessagePipeDispatcher> d2 =
+      MessagePipeDispatcher::CreateRemoteMessagePipe(&cep2);
+  const ChannelId id2 = 2;
+  cm.CreateChannelOnIOThread(id2, channel_pair.PassClientHandle(), cep2);
+  cep2 = nullptr;
 
-  const ChannelId id2 = 2;
-  cm.AddChannel(id2, ch2, base::MessageLoopProxy::current());
-  EXPECT_FALSE(ch2->HasOneRef());
+  scoped_refptr<Channel> ch1 = cm.GetChannel(id1);
+  EXPECT_TRUE(ch1);
+
+  scoped_refptr<Channel> ch2 = cm.GetChannel(id2);
+  EXPECT_TRUE(ch2);
 
   // Calling |WillShutdownChannel()| multiple times (on |id1|) is okay.
   cm.WillShutdownChannel(id1);
   cm.WillShutdownChannel(id1);
   EXPECT_FALSE(ch1->HasOneRef());
   // Not calling |WillShutdownChannel()| (on |id2|) is okay too.
 
   cm.ShutdownChannel(id1);
   EXPECT_TRUE(ch1->HasOneRef());
   cm.ShutdownChannel(id2);
   EXPECT_TRUE(ch2->HasOneRef());
+
+  EXPECT_EQ(MOJO_RESULT_OK, d1->Close());
+  EXPECT_EQ(MOJO_RESULT_OK, d2->Close());
 }
 
 class OtherThread : public base::SimpleThread {
  public:
-  // Note: We rely on the main thread keeping *exactly one* reference to
-  // |channel|.
+  // Note: There should be no other refs to the channel identified by
+  // |channel_id| outside the channel manager.
   OtherThread(scoped_refptr<base::TaskRunner> task_runner,
               ChannelManager* channel_manager,
-              Channel* channel,
+              ChannelId channel_id,
               base::Closure quit_closure)
       : base::SimpleThread("other_thread"),
         task_runner_(task_runner),
         channel_manager_(channel_manager),
-        channel_(channel),
+        channel_id_(channel_id),
         quit_closure_(quit_closure) {}
   ~OtherThread() override {}
 
  private:
   void Run() override {
-    // See comment above constructor.
-    ASSERT_TRUE(channel_->HasOneRef());
+    // TODO(vtl): Once we have a way of creating a channel from off the I/O
+    // thread, do that here instead.
 
     // You can use any unique, nonzero value as the ID.
-    const ChannelId id =
-        static_cast<ChannelId>(reinterpret_cast<uintptr_t>(channel_));
-    channel_manager_->AddChannel(id, make_scoped_refptr(channel_),
-                                 task_runner_);
-    // |ChannelManager| should take a ref.
-    EXPECT_FALSE(channel_->HasOneRef());
+    scoped_refptr<Channel> ch = channel_manager_->GetChannel(channel_id_);
+    // |ChannelManager| should have a ref.
+    EXPECT_FALSE(ch->HasOneRef());
 
-    channel_manager_->WillShutdownChannel(id);
+    channel_manager_->WillShutdownChannel(channel_id_);
     // |ChannelManager| should still have a ref.
-    EXPECT_FALSE(channel_->HasOneRef());
+    EXPECT_FALSE(ch->HasOneRef());
 
-    channel_manager_->ShutdownChannel(id);
+    channel_manager_->ShutdownChannel(channel_id_);
     // This doesn't happen synchronously, so we "wait" until it does.
-    // TODO(vtl): Possibly |Channel| should provide some notification of being
+    // TODO(vtl): Probably |Channel| should provide some notification of being
     // shut down.
     base::TimeTicks start_time(base::TimeTicks::Now());
     for (;;) {
-      if (channel_->HasOneRef())
+      if (ch->HasOneRef())
         break;
 
       // Check, instead of assert, since if things go wrong, dying is more
       // reliable than tearing down.
       CHECK_LT(base::TimeTicks::Now() - start_time,
                TestTimeouts::action_timeout());
       base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20));
     }
 
     CHECK(task_runner_->PostTask(FROM_HERE, quit_closure_));
   }
 
   scoped_refptr<base::TaskRunner> task_runner_;
   ChannelManager* channel_manager_;
-  Channel* channel_;
+  ChannelId channel_id_;
   base::Closure quit_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(OtherThread);
 };
 
 TEST_F(ChannelManagerTest, CallsFromOtherThread) {
   ChannelManager cm(platform_support());
 
-  // Hang on to a ref to the |Channel|, so that we can check that the
-  // |ChannelManager| takes/releases refs to it.
-  scoped_refptr<Channel> ch(new Channel(platform_support()));
-  ASSERT_TRUE(ch->HasOneRef());
+  embedder::PlatformChannelPair channel_pair;
 
-  embedder::PlatformChannelPair channel_pair;
-  ch->Init(RawChannel::Create(channel_pair.PassServerHandle()));
+  scoped_refptr<ChannelEndpoint> cep;
+  scoped_refptr<MessagePipeDispatcher> d =
+      MessagePipeDispatcher::CreateRemoteMessagePipe(&cep);
+  const ChannelId id = 1;
+  cm.CreateChannelOnIOThread(id, channel_pair.PassServerHandle(), cep);
+  cep = nullptr;
 
   base::RunLoop run_loop;
-  OtherThread thread(base::MessageLoopProxy::current(), &cm, ch.get(),
+  OtherThread thread(base::MessageLoopProxy::current(), &cm, id,
                      run_loop.QuitClosure());
   thread.Start();
   run_loop.Run();
   thread.Join();
+
+  EXPECT_EQ(MOJO_RESULT_OK, d->Close());
 }
 
 }  // namespace
 }  // namespace system
 }  // namespace mojo