Mojo Bindings: Fix lock-order inversion in associated controllers

ipc/ipc_mojo_bootstrap.cc

Index: ipc/ipc_mojo_bootstrap.cc
diff --git a/ipc/ipc_mojo_bootstrap.cc b/ipc/ipc_mojo_bootstrap.cc
index 8f27cbd477bc5c14980de00f649fed3910304233..3ba136619a6bd920a3f0929284c5e4927f14f913 100644
--- a/ipc/ipc_mojo_bootstrap.cc
+++ b/ipc/ipc_mojo_bootstrap.cc
@@ -169,6 +169,8 @@ class ChannelAssociatedGroupController
     if (!is_local) {
       DCHECK(ContainsKey(endpoints_, id));
       DCHECK(!mojo::IsMasterInterfaceId(id));
+
+      base::AutoUnlock unlocker(lock_);
       control_message_proxy_.NotifyEndpointClosedBeforeSent(id);

[yzshen1, 2016/11/10 18:02:34]

PipeControlMessageProxy has a |context_| member which is not thread safe. For
those control messages, we actually don't use it in anyway (we don't have
associated interfaces, handles or custom types). So it shouldn't cause trouble.
But it seems nice to not rely on that. Instead, we could create a new context
object as local variable whenever we need to do serialization/deserialization.

And maybe it makes sense to add a comment to PipeControlMessageProxy saying that
the class could be used from multiple threads.

[Ken Rockot(use gerrit already), 2016/11/10 18:26:44]

Done

       return;
     }
@@ -179,6 +181,7 @@ class ChannelAssociatedGroupController
     DCHECK(!endpoint->closed());
     MarkClosedAndMaybeRemove(endpoint);
 
+    base::AutoUnlock unlocker(lock_);
     if (!mojo::IsMasterInterfaceId(id))
       control_message_proxy_.NotifyPeerEndpointClosed(id);
   }
@@ -742,10 +745,13 @@ class ChannelAssociatedGroupController
     if (mojo::IsMasterInterfaceId(id))
       return false;
 
-    base::AutoLock locker(lock_);
-    Endpoint* endpoint = FindOrInsertEndpoint(id, nullptr);
-    DCHECK(!endpoint->closed());
-    MarkClosedAndMaybeRemove(endpoint);
+    {
+      base::AutoLock locker(lock_);
+      Endpoint* endpoint = FindOrInsertEndpoint(id, nullptr);
+      DCHECK(!endpoint->closed());
+      MarkClosedAndMaybeRemove(endpoint);
+    }
+
     control_message_proxy_.NotifyPeerEndpointClosed(id);
     return true;
   }
@@ -762,6 +768,8 @@ class ChannelAssociatedGroupController
   mojo::FilterChain filters_;
   mojo::PipeControlMessageHandler control_message_handler_;
   ControlMessageProxyThunk control_message_proxy_thunk_;
+
+  // NOTE: It is unsafe to call into this object while holding |lock_|.
   mojo::PipeControlMessageProxy control_message_proxy_;
 
   // Outgoing messages that were sent before this controller was bound to a