Mojo: Remove the STATE_WAIT_LOCAL_DETACH_AND_REMOTE_REMOVE_ACK endpoint state.

And add a long explanatory comment about how things are shut down.

R=darin@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#292670}

mojo/system/channel.cc

@@ -258,9 +258,6 @@ void Channel::DetachMessagePipeEndpoint(
       case EndpointInfo::STATE_WAIT_REMOTE_REMOVE_ACK:
         NOTREACHED();
         break;
-      case EndpointInfo::STATE_WAIT_LOCAL_DETACH_AND_REMOTE_REMOVE_ACK:
-        it->second.state = EndpointInfo::STATE_WAIT_REMOTE_REMOVE_ACK;
-        break;
     }
   }
   if (!should_send_remove_message)

mojo/system/channel.h

@@ -132,6 +132,70 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel
   }
 
  private:
+  // Terminology:
+  //   - "Message pipe endpoint": In the implementation, a |MessagePipe| owns
+  //     two |MessagePipeEndpoint| objects, one for each port. The
+  //     |MessagePipeEndpoint| objects are only accessed via the |MessagePipe|
+  //     (which has the lock), with the additional information of the port
+  //     number. So as far as the channel is concerned, a message pipe endpoint
+  //     is a pointer to a |MessagePipe| together with the port number.
+  //       - The value of |port| in |EndpointInfo| refers to the
+  //         |ProxyMessagePipeEndpoint| (i.e., the endpoint that is logically on
+  //         the other side). Messages received by a channel for a message pipe
+  //         are thus written to the *peer* of this port.
+  //   - "Attached"/"detached": A message pipe endpoint is attached to a channel
+  //     if it has a pointer to it. It must be detached before the channel gives
+  //     up its pointer to it in order to break a reference cycle. (This cycle
+  //     is needed to allow a channel to be shut down cleanly, without shutting
+  //     down everything else first.)
+  //   - "Running" (message pipe endpoint): A message pipe endpoint is running
+  //     if messages written to it (via some |MessagePipeDispatcher|, to which
+  //     some |MojoHandle| is assigned) are being transmitted through the
+  //     channel.
+  //       - Before a message pipe endpoint is run, it will queue messages.
+  //       - When a message pipe endpoint is detached from a channel, it is also
+  //         taken out of the running state. After that point, messages should
+  //         no longer be written to it.
+  //   - "Normal" message pipe endpoint (state): The channel itself does not
+  //     have knowledge of whether a message pipe endpoint has started running
+  //     yet. It will *receive* messages for a message pipe in either state (but
+  //     the message pipe endpoint won't *send* messages to the channel if it
+  //     has not started running).
+  //   - "Zombie" message pipe endpoint (state): A message pipe endpoint is a
+  //     zombie if it is still in |local_id_to_endpoint_info_map_|, but the
+  //     channel is no longer forwarding messages to it (even if it may still be
+  //     receiving messages for it).
+  //       - There are various types of zombies, depending on the reason the
+  //         message pipe endpoint cannot yet be removed.
+  //       - If the remote side is closed, it will send a "remove" control
+  //         message. After the channel receives that message (to which it
+  //         responds with a "remove ack" control message), it knows that it
+  //         shouldn't receive any more messages for that message pipe endpoint
+  //         (local ID), but it must wait for the endpoint to detach. (It can't
+  //         do so without a race, since it can't call into the message pipe
+  //         under |lock_|.) [TODO(vtl): When I add remotely-allocated IDs,
+  //         we'll have to remove the |EndpointInfo| from
+  //         |local_id_to_endpoint_info_map_| -- i.e., remove the local ID,
+  //         since it's no longer valid and may be reused by the remote side --
+  //         and keep the |EndpointInfo| alive in some other way.]
+  //       - If the local side is closed and the message pipe endpoint was
+  //         already running (so there are no queued messages left to send), it
+  //         will detach the endpoint, and send a "remove" control message.
+  //         However, the channel may still receive messages for that endpoint
+  //         until it receives a "remove ack" control message.
+  //       - If the local side is closed but the message pipe endpoint was not
+  //         yet running , the detaching is delayed until after it is run and
+  //         all the queued messages are sent to the channel. On being detached,
+  //         things proceed as in one of the above cases. The endpoint is *not*
+  //         a zombie until it is detached (or a "remove" message is received).
+  //         [TODO(vtl): Maybe we can get rid of this case? It'd only not yet be
+  //         running since under the current scheme it wouldn't have a remote ID
+  //         yet.]
+  //       - Note that even if the local side is closed, it may still receive a
+  //         "remove" message from the other side (if the other side is closed
+  //         simultaneously, and both sides send "remove" messages). In that
+  //         case, it must still remain alive until it receives the "remove
+  //         ack" (and it must ack the "remove" message that it received).
   struct EndpointInfo {
     enum State {
       // Attached, possibly running or not.
@@ -142,8 +206,6 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel
       // Waiting for a |kSubtypeChannelRemoveMessagePipeEndpointAck| before
       // removing.
       STATE_WAIT_REMOTE_REMOVE_ACK,
-      // Waiting for both of the above conditions before removing.
-      STATE_WAIT_LOCAL_DETACH_AND_REMOTE_REMOVE_ACK,
     };
 
     EndpointInfo();