Mojo: Split up Channel::RemoveMessagePipeEndpoint().

mojo/system/channel.cc

 // Copyright 2013 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/system/channel.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 399 matching lines @@
       if (!RunMessagePipeEndpoint(message_view.destination_id(),
                                   message_view.source_id())) {
         HandleRemoteError(
             "Received invalid channel message to run message pipe");
       }
       break;
     case MessageInTransit::kSubtypeChannelRemoveMessagePipeEndpoint:
       DVLOG(2) << "Handling channel message to remove message pipe (local ID "
                << message_view.destination_id() << ", remote ID "
                << message_view.source_id() << ")";
-      if (!RemoveMessagePipeEndpoint(message_view.destination_id(),
-                                     message_view.source_id())) {
+      if (!OnRemoveMessagePipeEndpoint(message_view.destination_id(),
+                                       message_view.source_id())) {
         HandleRemoteError(
             "Received invalid channel message to remove message pipe");
       }
       break;
     case MessageInTransit::kSubtypeChannelRemoveMessagePipeEndpointAck:
       DVLOG(2) << "Handling channel message to ack remove message pipe (local "
                   "ID " << message_view.destination_id() << ", remote ID "
                << message_view.source_id() << ")";
-      if (!RemoveMessagePipeEndpoint(message_view.destination_id(),
-                                     message_view.source_id())) {
+      if (!OnRemoveMessagePipeEndpointAck(message_view.destination_id())) {
         HandleRemoteError(
             "Received invalid channel message to ack remove message pipe");
       }
       break;
     default:
       HandleRemoteError("Received invalid channel message");
       NOTREACHED();
       break;
   }
 }
 
-bool Channel::RemoveMessagePipeEndpoint(
+bool Channel::OnRemoveMessagePipeEndpoint(
     MessageInTransit::EndpointId local_id,
     MessageInTransit::EndpointId remote_id) {
   DCHECK(creation_thread_checker_.CalledOnValidThread());
 
-  // If this is non-null after the locked block, the endpoint should be detached
-  // (and no remove ack message sent).
-  scoped_refptr<ChannelEndpoint> endpoint_to_detach;
   scoped_refptr<MessagePipe> message_pipe;
   unsigned port = ~0u;
   {
     base::AutoLock locker(lock_);
 
     IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id);
     if (it == local_id_to_endpoint_map_.end()) {
-      DVLOG(2) << "Remove message pipe error: not found";
+      DVLOG(2) << "Remove message pipe endpoint error: not found";
       return false;
     }
 
     switch (it->second->state_) {
       case ChannelEndpoint::STATE_NORMAL:
-        it->second->state_ = ChannelEndpoint::STATE_WAIT_LOCAL_DETACH;
-        message_pipe = it->second->message_pipe_;
-        port = it->second->port_;
-        it->second->message_pipe_ = nullptr;
-        // We have to send a remove ack message (outside the lock).
+        // This is the normal case; we'll proceed on to "wait local detach".
         break;
+
       case ChannelEndpoint::STATE_WAIT_LOCAL_DETACH:
-        DVLOG(2) << "Remove message pipe error: wrong state";
+        // We can only be in this state because we got a "remove" already, so
+        // getting another such message is invalid.
+        DVLOG(2) << "Remove message pipe endpoint error: wrong state";
         return false;
+
       case ChannelEndpoint::STATE_WAIT_REMOTE_REMOVE_ACK:
-        endpoint_to_detach = it->second;
-        local_id_to_endpoint_map_.erase(it);
-        // We have to detach (outside the lock).
-        break;
+        // Remove messages "crossed"; we have to wait for the ack.
+        return true;
     }
-  }
-  if (endpoint_to_detach.get()) {
-    endpoint_to_detach->DetachFromChannel();
-    return true;
+
+    it->second->state_ = ChannelEndpoint::STATE_WAIT_LOCAL_DETACH;
+    message_pipe = it->second->message_pipe_;
+    port = it->second->port_;
+    it->second->message_pipe_ = nullptr;
+    // Send the remove ack message outside the lock.
   }
 
   if (!SendControlMessage(
           MessageInTransit::kSubtypeChannelRemoveMessagePipeEndpointAck,
           local_id,
           remote_id)) {
     HandleLocalError(base::StringPrintf(
         "Failed to send message to remove remote message pipe endpoint ack "
         "(local ID %u, remote ID %u)",
         static_cast<unsigned>(local_id),
         static_cast<unsigned>(remote_id)));
   }
 
   message_pipe->Close(port);
-
   return true;
 }
 
+bool Channel::OnRemoveMessagePipeEndpointAck(
+    MessageInTransit::EndpointId local_id) {
+  DCHECK(creation_thread_checker_.CalledOnValidThread());
+
+  scoped_refptr<ChannelEndpoint> endpoint_to_detach;
+  {
+    base::AutoLock locker(lock_);
+
+    IdToEndpointMap::iterator it = local_id_to_endpoint_map_.find(local_id);
+    if (it == local_id_to_endpoint_map_.end()) {
+      DVLOG(2) << "Remove message pipe endpoint ack error: not found";
+      return false;
+    }
+
+    if (it->second->state_ != ChannelEndpoint::STATE_WAIT_REMOTE_REMOVE_ACK) {
+      DVLOG(2) << "Remove message pipe endpoint ack error: wrong state";
+      return false;
+    }
+
+    endpoint_to_detach = it->second;
+    local_id_to_endpoint_map_.erase(it);
+    // Detach the endpoint outside the lock.
+  }
+
+  endpoint_to_detach->DetachFromChannel();
+  return true;
+}
+
 bool Channel::SendControlMessage(MessageInTransit::Subtype subtype,
                                  MessageInTransit::EndpointId local_id,
                                  MessageInTransit::EndpointId remote_id) {
   DVLOG(2) << "Sending channel control message: subtype " << subtype
            << ", local ID " << local_id << ", remote ID " << remote_id;
   scoped_ptr<MessageInTransit> message(new MessageInTransit(
       MessageInTransit::kTypeChannel, subtype, 0, nullptr));
   message->set_source_id(local_id);
   message->set_destination_id(remote_id);
   return WriteMessage(message.Pass());
 }
 
 void Channel::HandleRemoteError(const base::StringPiece& error_message) {
   // TODO(vtl): Is this how we really want to handle this? Probably we want to
   // terminate the connection, since it's spewing invalid stuff.
   LOG(WARNING) << error_message;
 }
 
 void Channel::HandleLocalError(const base::StringPiece& error_message) {
   // TODO(vtl): Is this how we really want to handle this?
   // Sometimes we'll want to propagate the error back to the message pipe
   // (endpoint), and notify it that the remote is (effectively) closed.
   // Sometimes we'll want to kill the channel (and notify all the endpoints that
   // their remotes are dead.
   LOG(WARNING) << error_message;
 }
 
 }  // namespace system
 }  // namespace mojo