Mojo: Convert MessageInTransit* -> scoped_ptr<MessageInTransit>, part 2.

R=yzshen@chromium.org

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

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

8 files changed, 55 insertions, 63 deletions

diff --git a/mojo/system/channel.cc b/mojo/system/channel.cc
index 93a0559..c45ce4c 100644
--- a/mojo/system/channel.cc
+++ b/mojo/system/channel.cc
@@ -195,15 +195,15 @@ void Channel::OnReadMessageForDownstream(const MessageInTransit& message) {
   // We need to duplicate the message, because |EnqueueMessage()| will take
   // ownership of it.
   // TODO(vtl): Need to enforce limits on message size and handle count.
-  MessageInTransit* own_message =
-      new MessageInTransit(MessageInTransit::OWNED_BUFFER, message);
+  scoped_ptr<MessageInTransit> own_message(
+      new MessageInTransit(MessageInTransit::OWNED_BUFFER, message));
   std::vector<DispatcherTransport> transports(message.num_handles());
   // TODO(vtl): Create dispatchers for handles.
   // TODO(vtl): It's bad that the current API will create equivalent dispatchers
   // for the freshly-created ones, which is totally redundant. Make a version of
   // |EnqueueMessage()| that passes ownership.
   if (endpoint_info.message_pipe->EnqueueMessage(
-          MessagePipe::GetPeerPort(endpoint_info.port), own_message,
+          MessagePipe::GetPeerPort(endpoint_info.port), own_message.Pass(),
           message.num_handles() ? &transports : NULL) != MOJO_RESULT_OK) {
     HandleLocalError(base::StringPrintf(
         "Failed to enqueue message to local destination ID %u",
diff --git a/mojo/system/local_message_pipe_endpoint.cc b/mojo/system/local_message_pipe_endpoint.cc
index 792c315..a58e489 100644
--- a/mojo/system/local_message_pipe_endpoint.cc
+++ b/mojo/system/local_message_pipe_endpoint.cc
@@ -41,14 +41,14 @@ LocalMessagePipeEndpoint::MessageQueueEntry::~MessageQueueEntry() {
 }
 
 void LocalMessagePipeEndpoint::MessageQueueEntry::Init(
-    MessageInTransit* message,
+    scoped_ptr<MessageInTransit> message,
     std::vector<DispatcherTransport>* transports) {
-  DCHECK(message);
+  DCHECK(message.get());
   DCHECK(!transports || !transports->empty());
   DCHECK(!message_);
   DCHECK(dispatchers_.empty());
 
-  message_ = message;
+  message_ = message.release();
   if (transports) {
     dispatchers_.reserve(transports->size());
     for (size_t i = 0; i < transports->size(); i++) {
@@ -104,7 +104,7 @@ void LocalMessagePipeEndpoint::OnPeerClose() {
 }
 
 MojoResult LocalMessagePipeEndpoint::EnqueueMessage(
-    MessageInTransit* message,
+    scoped_ptr<MessageInTransit> message,
     std::vector<DispatcherTransport>* transports) {
   DCHECK(is_open_);
   DCHECK(is_peer_open_);
@@ -114,7 +114,7 @@ MojoResult LocalMessagePipeEndpoint::EnqueueMessage(
   // TODO(vtl): Use |emplace_back()| (and a suitable constructor, instead of
   // |Init()|) when that becomes available.
   message_queue_.push_back(MessageQueueEntry());
-  message_queue_.back().Init(message, transports);
+  message_queue_.back().Init(message.Pass(), transports);
   if (was_empty) {
     waiter_list_.AwakeWaitersForStateChange(SatisfiedFlags(),
                                             SatisfiableFlags());
diff --git a/mojo/system/local_message_pipe_endpoint.h b/mojo/system/local_message_pipe_endpoint.h
index 0f1697e..98a665e 100644
--- a/mojo/system/local_message_pipe_endpoint.h
+++ b/mojo/system/local_message_pipe_endpoint.h
@@ -28,7 +28,7 @@ class MOJO_SYSTEM_IMPL_EXPORT LocalMessagePipeEndpoint
   virtual void Close() OVERRIDE;
   virtual void OnPeerClose() OVERRIDE;
   virtual MojoResult EnqueueMessage(
-      MessageInTransit* message,
+      scoped_ptr<MessageInTransit> message,
       std::vector<DispatcherTransport>* transports) OVERRIDE;
 
   // There's a dispatcher for |LocalMessagePipeEndpoint|s, so we have to
@@ -55,12 +55,11 @@ class MOJO_SYSTEM_IMPL_EXPORT LocalMessagePipeEndpoint
     MessageQueueEntry(const MessageQueueEntry& other);
     ~MessageQueueEntry();
 
-    // Initialize, taking ownership of |message| and creating equivalent
-    // "duplicate" dispatchers. |transports| should be non-null only if
-    // nonempty.
+    // Initialize, creating equivalent "duplicate" dispatchers. |transports|
+    // should be non-null only if nonempty.
     // TODO(vtl): This would simply be a constructor, but we don't have C++11's
     // emplace operations yet, and I don't want to copy |dispatchers_|.
-    void Init(MessageInTransit* message,
+    void Init(scoped_ptr<MessageInTransit> message,
               std::vector<DispatcherTransport>* transports);
 
     MessageInTransit* message() {
diff --git a/mojo/system/message_pipe.cc b/mojo/system/message_pipe.cc
index 062208a..ed5b4ca 100644
--- a/mojo/system/message_pipe.cc
+++ b/mojo/system/message_pipe.cc
@@ -67,10 +67,11 @@ MojoResult MessagePipe::WriteMessage(
       transports ? static_cast<uint32_t>(transports->size()) : 0;
   return EnqueueMessage(
       GetPeerPort(port),
-      new MessageInTransit(MessageInTransit::OWNED_BUFFER,
-                           MessageInTransit::kTypeMessagePipeEndpoint,
-                           MessageInTransit::kSubtypeMessagePipeEndpointData,
-                           num_bytes, num_handles, bytes),
+      make_scoped_ptr(new MessageInTransit(
+          MessageInTransit::OWNED_BUFFER,
+          MessageInTransit::kTypeMessagePipeEndpoint,
+          MessageInTransit::kSubtypeMessagePipeEndpointData,
+          num_bytes, num_handles, bytes)),
       transports);
 }
 
@@ -113,17 +114,17 @@ void MessagePipe::RemoveWaiter(unsigned port, Waiter* waiter) {
 
 MojoResult MessagePipe::EnqueueMessage(
     unsigned port,
-    MessageInTransit* message,
+    scoped_ptr<MessageInTransit> message,
     std::vector<DispatcherTransport>* transports) {
   DCHECK(port == 0 || port == 1);
-  DCHECK(message);
+  DCHECK(message.get());
   DCHECK((!transports && message->num_handles() == 0) ||
          (transports && transports->size() > 0 &&
               message->num_handles() == transports->size()));
 
   if (message->type() == MessageInTransit::kTypeMessagePipe) {
     DCHECK(!transports);
-    return HandleControlMessage(port, message);
+    return HandleControlMessage(port, message.Pass());
   }
 
   DCHECK_EQ(message->type(), MessageInTransit::kTypeMessagePipeEndpoint);
@@ -132,10 +133,8 @@ MojoResult MessagePipe::EnqueueMessage(
   DCHECK(endpoints_[GetPeerPort(port)].get());
 
   // The destination port need not be open, unlike the source port.
-  if (!endpoints_[port].get()) {
-    delete message;
+  if (!endpoints_[port].get())
     return MOJO_RESULT_FAILED_PRECONDITION;
-  }
 
   if (transports) {
     // You're not allowed to send either handle to a message pipe over the
@@ -160,7 +159,7 @@ MojoResult MessagePipe::EnqueueMessage(
     }
   }
 
-  return endpoints_[port]->EnqueueMessage(message, transports);
+  return endpoints_[port]->EnqueueMessage(message.Pass(), transports);
 }
 
 void MessagePipe::Attach(unsigned port,
@@ -193,13 +192,13 @@ MessagePipe::~MessagePipe() {
   DCHECK(!endpoints_[1].get());
 }
 
-MojoResult MessagePipe::HandleControlMessage(unsigned port,
-                                             MessageInTransit* message) {
+MojoResult MessagePipe::HandleControlMessage(
+    unsigned port,
+    scoped_ptr<MessageInTransit> message) {
   DCHECK(port == 0 || port == 1);
-  DCHECK(message);
+  DCHECK(message.get());
   DCHECK_EQ(message->type(), MessageInTransit::kTypeMessagePipe);
 
-  MojoResult rv = MOJO_RESULT_OK;
   switch (message->subtype()) {
     case MessageInTransit::kSubtypeMessagePipePeerClosed: {
       unsigned source_port = GetPeerPort(port);
@@ -212,17 +211,13 @@ MojoResult MessagePipe::HandleControlMessage(unsigned port,
         endpoints_[port]->OnPeerClose();
 
       endpoints_[source_port].reset();
-      break;
+      return MOJO_RESULT_OK;
     }
-    default:
-      LOG(WARNING) << "Unrecognized MessagePipe control message subtype "
-                   << message->subtype();
-      rv = MOJO_RESULT_UNKNOWN;
-      break;
   }
 
-  delete message;
-  return rv;
+  LOG(WARNING) << "Unrecognized MessagePipe control message subtype "
+               << message->subtype();
+  return MOJO_RESULT_UNKNOWN;
 }
 
 }  // namespace system
diff --git a/mojo/system/message_pipe.h b/mojo/system/message_pipe.h
index da7bf2e..7bfffd1 100644
--- a/mojo/system/message_pipe.h
+++ b/mojo/system/message_pipe.h
@@ -67,10 +67,10 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipe :
 
   // This is used internally by |WriteMessage()| and by |Channel| to enqueue
   // messages (typically to a |LocalMessagePipeEndpoint|). Unlike
-  // |WriteMessage()|, |port| is the *destination* port. Takes ownership of
-  // |message|. |dispatchers| should be non-null only if it's nonempty.
+  // |WriteMessage()|, |port| is the *destination* port. |dispatchers| should be
+  // non-null only if it's nonempty.
   MojoResult EnqueueMessage(unsigned port,
-                            MessageInTransit* message,
+                            scoped_ptr<MessageInTransit> message,
                             std::vector<DispatcherTransport>* transports);
 
   // These are used by |Channel|.
@@ -85,7 +85,8 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipe :
 
   // Used by |EnqueueMessage()| to handle control messages that are actually
   // meant for us.
-  MojoResult HandleControlMessage(unsigned port, MessageInTransit* message);
+  MojoResult HandleControlMessage(unsigned port,
+                                  scoped_ptr<MessageInTransit> message);
 
   base::Lock lock_;  // Protects the following members.
   scoped_ptr<MessagePipeEndpoint> endpoints_[2];
diff --git a/mojo/system/message_pipe_endpoint.h b/mojo/system/message_pipe_endpoint.h
index a07cc0b..5cad2e6 100644
--- a/mojo/system/message_pipe_endpoint.h
+++ b/mojo/system/message_pipe_endpoint.h
@@ -11,6 +11,7 @@
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "mojo/public/system/core.h"
 #include "mojo/system/dispatcher.h"
 #include "mojo/system/message_in_transit.h"
@@ -40,7 +41,7 @@ class MOJO_SYSTEM_IMPL_EXPORT MessagePipeEndpoint {
   // Implements |MessagePipe::EnqueueMessage()| (see its description for
   // details).
   virtual MojoResult EnqueueMessage(
-      MessageInTransit* message,
+      scoped_ptr<MessageInTransit> message,
       std::vector<DispatcherTransport>* transports) = 0;
 
   // Implementations must override these if they represent a local endpoint,
diff --git a/mojo/system/proxy_message_pipe_endpoint.cc b/mojo/system/proxy_message_pipe_endpoint.cc
index e3a79a0..28abe9e 100644
--- a/mojo/system/proxy_message_pipe_endpoint.cc
+++ b/mojo/system/proxy_message_pipe_endpoint.cc
@@ -38,14 +38,7 @@ void ProxyMessagePipeEndpoint::Close() {
   channel_ = NULL;
   local_id_ = MessageInTransit::kInvalidEndpointId;
   remote_id_ = MessageInTransit::kInvalidEndpointId;
-
-  for (std::deque<MessageInTransit*>::iterator it =
-           paused_message_queue_.begin();
-       it != paused_message_queue_.end();
-       ++it) {
-    delete *it;
-  }
-  paused_message_queue_.clear();
+  STLDeleteElements(&paused_message_queue_);
 }
 
 void ProxyMessagePipeEndpoint::OnPeerClose() {
@@ -53,21 +46,22 @@ void ProxyMessagePipeEndpoint::OnPeerClose() {
   DCHECK(is_peer_open_);
 
   is_peer_open_ = false;
-  MessageInTransit* message = new MessageInTransit(
-      MessageInTransit::OWNED_BUFFER, MessageInTransit::kTypeMessagePipe,
-      MessageInTransit::kSubtypeMessagePipePeerClosed, 0, 0, NULL);
-  EnqueueMessageInternal(message);
+  EnqueueMessageInternal(make_scoped_ptr(
+      new MessageInTransit(MessageInTransit::OWNED_BUFFER,
+                           MessageInTransit::kTypeMessagePipe,
+                           MessageInTransit::kSubtypeMessagePipePeerClosed,
+                           0, 0, NULL)));
 }
 
 MojoResult ProxyMessagePipeEndpoint::EnqueueMessage(
-    MessageInTransit* message,
+    scoped_ptr<MessageInTransit> message,
     std::vector<DispatcherTransport>* transports) {
   DCHECK(!transports || !transports->empty());
 
   if (transports)
-    AttachAndCloseDispatchers(message, transports);
+    AttachAndCloseDispatchers(message.get(), transports);
 
-  EnqueueMessageInternal(message);
+  EnqueueMessageInternal(message.Pass());
   return MOJO_RESULT_OK;
 }
 
@@ -99,7 +93,7 @@ void ProxyMessagePipeEndpoint::Run(MessageInTransit::EndpointId remote_id) {
   for (std::deque<MessageInTransit*>::iterator it =
            paused_message_queue_.begin(); it != paused_message_queue_.end();
        ++it)
-    EnqueueMessageInternal(*it);
+    EnqueueMessageInternal(make_scoped_ptr(*it));
   paused_message_queue_.clear();
 }
 
@@ -120,7 +114,7 @@ void ProxyMessagePipeEndpoint::AttachAndCloseDispatchers(
 // -- it may have been written to and closed immediately, before we were ready.
 // This case is handled in |Run()| (which will call us).
 void ProxyMessagePipeEndpoint::EnqueueMessageInternal(
-    MessageInTransit* message) {
+    scoped_ptr<MessageInTransit> message) {
   DCHECK(is_open_);
 
   if (is_running()) {
@@ -129,11 +123,10 @@ void ProxyMessagePipeEndpoint::EnqueueMessageInternal(
     // If it fails at this point, the message gets dropped. (This is no
     // different from any other in-transit errors.)
     // Note: |WriteMessage()| will destroy the message even on failure.
-    // TODO(vtl): Convert |message| (and so on "upstream") to a |scoped_ptr|.
-    if (!channel_->WriteMessage(make_scoped_ptr(message)))
+    if (!channel_->WriteMessage(message.Pass()))
       LOG(WARNING) << "Failed to write message to channel";
   } else {
-    paused_message_queue_.push_back(message);
+    paused_message_queue_.push_back(message.release());
   }
 }
 
diff --git a/mojo/system/proxy_message_pipe_endpoint.h b/mojo/system/proxy_message_pipe_endpoint.h
index 137f192..c99a60a 100644
--- a/mojo/system/proxy_message_pipe_endpoint.h
+++ b/mojo/system/proxy_message_pipe_endpoint.h
@@ -48,7 +48,7 @@ class MOJO_SYSTEM_IMPL_EXPORT ProxyMessagePipeEndpoint
   virtual void Close() OVERRIDE;
   virtual void OnPeerClose() OVERRIDE;
   virtual MojoResult EnqueueMessage(
-      MessageInTransit* message,
+      scoped_ptr<MessageInTransit> message,
       std::vector<DispatcherTransport>* transports) OVERRIDE;
   virtual void Attach(scoped_refptr<Channel> channel,
                       MessageInTransit::EndpointId local_id) OVERRIDE;
@@ -65,9 +65,10 @@ class MOJO_SYSTEM_IMPL_EXPORT ProxyMessagePipeEndpoint
 
   // "Attaches" |transports| (which must be non-null and nonempty) to |message|
   // by "serializing" them in an appropriate way, and closes each dispatcher.
+  // (Note that this simply modifies |*message|, and doesn't take ownership.)
   void AttachAndCloseDispatchers(MessageInTransit* message,
                                  std::vector<DispatcherTransport>* transports);
-  void EnqueueMessageInternal(MessageInTransit* message);
+  void EnqueueMessageInternal(scoped_ptr<MessageInTransit> message);
 
 #ifdef NDEBUG
   void AssertConsistentState() const {}
@@ -93,6 +94,8 @@ class MOJO_SYSTEM_IMPL_EXPORT ProxyMessagePipeEndpoint
 
   // This queue is only used while we're detached, to store messages while we're
   // not ready to send them yet.
+  // TODO(vtl): When C++11 is available, switch this to a deque of
+  // |scoped_ptr|/|unique_ptr|s.
   std::deque<MessageInTransit*> paused_message_queue_;
 
   DISALLOW_COPY_AND_ASSIGN(ProxyMessagePipeEndpoint);