Use ScopedMessagePipeHande in Connector.

This is the first step in moving the bindings entirely to
ScopedMessagePipeHandle

BUG=None
R=viettrungluu@chromium.org, viettrungluu

Review URL: https://codereview.chromium.org/89003003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@237577 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 36 insertions, 32 deletions

diff --git a/mojo/public/bindings/lib/connector.cc b/mojo/public/bindings/lib/connector.cc
index ca0cede..221dbea 100644
--- a/mojo/public/bindings/lib/connector.cc
+++ b/mojo/public/bindings/lib/connector.cc
@@ -13,8 +13,8 @@ namespace mojo {
 
 // ----------------------------------------------------------------------------
 
-Connector::Connector(const MessagePipeHandle& message_pipe)
-    : message_pipe_(message_pipe),
+Connector::Connector(ScopedMessagePipeHandle message_pipe)
+    : message_pipe_(message_pipe.Pass()),
       incoming_receiver_(NULL),
       error_(false) {
 }
diff --git a/mojo/public/bindings/lib/connector.h b/mojo/public/bindings/lib/connector.h
index b78b55c..1fbf1d8 100644
--- a/mojo/public/bindings/lib/connector.h
+++ b/mojo/public/bindings/lib/connector.h
@@ -21,9 +21,8 @@ namespace mojo {
 //
 class Connector : public MessageReceiver {
  public:
-  // The Connector does not take ownership of |message_pipe|.
-  // TODO(darin): Perhaps it should take ownership.
-  explicit Connector(const MessagePipeHandle& message_pipe);
+  // The Connector takes ownership of |message_pipe|.
+  explicit Connector(ScopedMessagePipeHandle message_pipe);
   virtual ~Connector();
 
   // Sets the receiver to handle messages read from the message pipe.  The
@@ -62,7 +61,7 @@ class Connector : public MessageReceiver {
   void WriteMore();
   void WriteOne(Message* message, bool* wait_to_write);
 
-  MessagePipeHandle message_pipe_;
+  ScopedMessagePipeHandle message_pipe_;
   MessageReceiver* incoming_receiver_;
   MessageQueue write_queue_;
 
diff --git a/mojo/public/bindings/lib/message.cc b/mojo/public/bindings/lib/message.cc
index 8b94832..c9eb38d 100644
--- a/mojo/public/bindings/lib/message.cc
+++ b/mojo/public/bindings/lib/message.cc
@@ -16,7 +16,9 @@ Message::Message()
 
 Message::~Message() {
   free(data);
-  std::for_each(handles.begin(), handles.end(), CloseRaw);
+  // TODO(davemoore): We don't close the handles because they're typically
+  // owned by the Connection. This could result in some Handle leaks. This will
+  // be fixed by a later cl.
 }
 
 void Message::Swap(Message* other) {
diff --git a/mojo/public/bindings/lib/message.h b/mojo/public/bindings/lib/message.h
index 1df1461..b5962ac 100644
--- a/mojo/public/bindings/lib/message.h
+++ b/mojo/public/bindings/lib/message.h
@@ -38,8 +38,7 @@ class Message {
   void Swap(Message* other);
 
   MessageData* data;  // Heap-allocated using malloc.
-  // TODO(vtl): Should these be ScopedHandles? How does that interact with
-  // encoding/decoding?
+  // TODO(davemoore): Turn these into ScopedHandles and fix bindings generation.
   std::vector<Handle> handles;
 
  private:
diff --git a/mojo/public/bindings/lib/remote_ptr.h b/mojo/public/bindings/lib/remote_ptr.h
index 91b292b..26056e9 100644
--- a/mojo/public/bindings/lib/remote_ptr.h
+++ b/mojo/public/bindings/lib/remote_ptr.h
@@ -48,8 +48,8 @@ namespace mojo {
 template <typename S>
 class RemotePtr {
  public:
-  explicit RemotePtr(const MessagePipeHandle& message_pipe)
-      : connector_(message_pipe),
+  explicit RemotePtr(ScopedMessagePipeHandle message_pipe)
+      : connector_(message_pipe.Pass()),
         proxy_(&connector_) {
   }
 
diff --git a/mojo/public/tests/bindings_connector_unittest.cc b/mojo/public/tests/bindings_connector_unittest.cc
index 6fe7750..10de52c 100644
--- a/mojo/public/tests/bindings_connector_unittest.cc
+++ b/mojo/public/tests/bindings_connector_unittest.cc
@@ -71,8 +71,8 @@ class BindingsConnectorTest : public testing::Test {
 };
 
 TEST_F(BindingsConnectorTest, Basic) {
-  Connector connector0(handle0_);
-  Connector connector1(handle1_);
+  Connector connector0(handle0_.Pass());
+  Connector connector1(handle1_.Pass());
 
   const char kText[] = "hello world";
 
@@ -97,8 +97,8 @@ TEST_F(BindingsConnectorTest, Basic) {
 }
 
 TEST_F(BindingsConnectorTest, Basic_EarlyIncomingReceiver) {
-  Connector connector0(handle0_);
-  Connector connector1(handle1_);
+  Connector connector0(handle0_.Pass());
+  Connector connector1(handle1_.Pass());
 
   MessageAccumulator accumulator;
   connector1.SetIncomingReceiver(&accumulator);
@@ -123,8 +123,8 @@ TEST_F(BindingsConnectorTest, Basic_EarlyIncomingReceiver) {
 }
 
 TEST_F(BindingsConnectorTest, Basic_TwoMessages) {
-  Connector connector0(handle0_);
-  Connector connector1(handle1_);
+  Connector connector0(handle0_.Pass());
+  Connector connector1(handle1_.Pass());
 
   const char* kText[] = { "hello", "world" };
 
@@ -153,25 +153,28 @@ TEST_F(BindingsConnectorTest, Basic_TwoMessages) {
 }
 
 TEST_F(BindingsConnectorTest, WriteToClosedPipe) {
-  Connector connector0(handle0_);
+  // Leak this, so the closed handle isn't closed again.
+  MojoHandle mojo_handle = handle0_.get().value();
+  Connector* connector0 = new Connector(handle0_.Pass());
 
   const char kText[] = "hello world";
 
   Message message;
   AllocMessage(kText, &message);
 
-  Close(handle0_.Pass());  // Close the handle before writing to it.
+  // Close handle out from under the connection
+  MojoClose(mojo_handle);
 
-  bool ok = connector0.Accept(&message);
+  bool ok = connector0->Accept(&message);
   EXPECT_FALSE(ok);
 
-  EXPECT_TRUE(connector0.EncounteredError());
+  EXPECT_TRUE(connector0->EncounteredError());
 }
 
 // Enable this test once MojoWriteMessage supports passing handles.
 TEST_F(BindingsConnectorTest, MessageWithHandles) {
-  Connector connector0(handle0_);
-  Connector connector1(handle1_);
+  Connector connector0(handle0_.Pass());
+  Connector connector1(handle1_.Pass());
 
   const char kText[] = "hello world";
 
@@ -205,9 +208,10 @@ TEST_F(BindingsConnectorTest, MessageWithHandles) {
   // Now send a message to the transferred handle and confirm it's sent through
   // to the orginal pipe.
   // TODO(vtl): Do we need a better way of "downcasting" the handle types?
-  Connector connector_received(
-      MessagePipeHandle(message_received.handles[0].value()));
-  Connector connector_original(handles[1]);
+  ScopedMessagePipeHandle smph;
+  smph.reset(MessagePipeHandle(message_received.handles[0].value()));
+  Connector connector_received(smph.Pass());
+  Connector connector_original(handles[1].Pass());
 
   AllocMessage(kText, &message);
 
diff --git a/mojo/public/tests/bindings_remote_ptr_unittest.cc b/mojo/public/tests/bindings_remote_ptr_unittest.cc
index 4005880..ebad72a 100644
--- a/mojo/public/tests/bindings_remote_ptr_unittest.cc
+++ b/mojo/public/tests/bindings_remote_ptr_unittest.cc
@@ -12,8 +12,8 @@ namespace test {
 
 class MathCalculatorImpl : public math::CalculatorStub {
  public:
-  explicit MathCalculatorImpl(const MessagePipeHandle& pipe)
-      : ui_(pipe),
+  explicit MathCalculatorImpl(ScopedMessagePipeHandle pipe)
+      : ui_(pipe.Pass()),
         total_(0.0) {
     ui_.SetPeer(this);
   }
@@ -39,8 +39,8 @@ class MathCalculatorImpl : public math::CalculatorStub {
 
 class MathCalculatorUIImpl : public math::CalculatorUIStub {
  public:
-  explicit MathCalculatorUIImpl(const MessagePipeHandle& pipe)
-      : calculator_(pipe),
+  explicit MathCalculatorUIImpl(ScopedMessagePipeHandle pipe)
+      : calculator_(pipe.Pass()),
         output_(0.0) {
     calculator_.SetPeer(this);
   }
@@ -98,10 +98,10 @@ class BindingsRemotePtrTest : public testing::Test {
 
 TEST_F(BindingsRemotePtrTest, EndToEnd) {
   // Suppose this is instantiated in a process that has pipe0_.
-  MathCalculatorImpl calculator(pipe0_);
+  MathCalculatorImpl calculator(pipe0_.Pass());
 
   // Suppose this is instantiated in a process that has pipe1_.
-  MathCalculatorUIImpl calculator_ui(pipe1_);
+  MathCalculatorUIImpl calculator_ui(pipe1_.Pass());
 
   calculator_ui.Add(2.0);
   calculator_ui.Multiply(5.0);