IPC: Fix attachment brokering race condition.

ipc/ipc_channel.h

 // Copyright (c) 2012 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.
 
 #ifndef IPC_IPC_CHANNEL_H_
 #define IPC_IPC_CHANNEL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 147 matching lines @@
       const IPC::ChannelHandle& channel_handle,
       Listener* listener);
 
   ~Channel() override;
 
   // Connect the pipe.  On the server side, this will initiate
   // waiting for connections.  On the client, it attempts to
   // connect to a pre-existing pipe.  Note, calling Connect()
   // will not block the calling thread and may complete
   // asynchronously.
+  //
+  // The subclass implementation must call OnConnect().

[Tom Sepez, 2016/04/22 00:33:22]

nit: when? "when the connection is completed"? or what happens?

[erikchen, 2016/04/22 01:42:18]

I renamed the method to WillConnect() and updated the comment to indicate that
the method must be called at the beginning of the implementation.

This exists entirely to power the DCHECK in OnSetAttachmentBrokerEndpoint() to
prevent this error from happening again.

   virtual bool Connect() WARN_UNUSED_RESULT = 0;
 
   // Close this Channel explicitly.  May be called multiple times.
   // On POSIX calling close on an IPC channel that listens for connections will
   // cause it to close any accepted connections, and it will stop listening for
   // new connections. If you just want to close the currently accepted
   // connection and listen for new ones, use ResetToAcceptingConnectionState.
   virtual void Close() = 0;
 
   // Get its own process id. This value is told to the peer.
@@ skipping 70 matching lines @@
     ~OutputElement();
     size_t size() const { return message_ ? message_->size() : length_; }
     const void* data() const { return message_ ? message_->data() : buffer_; }
     Message* get_message() const { return message_.get(); }
 
    private:
     scoped_ptr<Message> message_;
     void* buffer_;
     size_t length_;
   };
+
+  // Endpoint overrides.
+  void OnSetAttachmentBrokerEndpoint() override;
+
+  // Subclasses should call this method after connecting.

[Tom Sepez, 2016/04/22 00:33:22]

should vs. must?

[erikchen, 2016/04/22 01:42:18]

Updated the comment to use MUST.

+  void OnConnect();
+
+ private:
+  bool did_connect_ = false;
 };
 
 #if defined(OS_POSIX)
 // SocketPair() creates a pair of socket FDs suitable for using with
 // IPC::Channel.
 IPC_EXPORT bool SocketPair(int* fd1, int* fd2);
 #endif
 
 }  // namespace IPC
 
 #endif  // IPC_IPC_CHANNEL_H_