Fix races with MessagePipeReader due to the Mojo IPC channel being thread-safe.

ipc/mojo/ipc_message_pipe_reader.h

Index: ipc/mojo/ipc_message_pipe_reader.h
diff --git a/ipc/mojo/ipc_message_pipe_reader.h b/ipc/mojo/ipc_message_pipe_reader.h
index 937930eef33b6f6785bb911f365894a31a63e6b2..693f3b0b026551ca42f9debd005f411c60e7edf1 100644
--- a/ipc/mojo/ipc_message_pipe_reader.h
+++ b/ipc/mojo/ipc_message_pipe_reader.h
@@ -7,8 +7,10 @@
 
 #include <vector>
 
+#include "base/atomicops.h"
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/threading/thread_checker.h"
 #include "ipc/ipc_message.h"
 #include "third_party/mojo/src/mojo/public/c/environment/async_waiter.h"
 #include "third_party/mojo/src/mojo/public/cpp/system/core.h"
@@ -30,7 +32,9 @@ class AsyncHandleWaiter;
 //  * Create the subclass instance with a MessagePipeHandle.
 //    The constructor automatically start listening on the pipe.
 //
-// MessageReader has to be used in IO thread. It isn't thread-safe.
+// All functions must be called on the IO thread, except for Send(), which can
+// be called on any thread. All |Delegate| functions will be called on the IO
+// thread.
 //
 class MessagePipeReader {
  public:
@@ -62,7 +66,7 @@ class MessagePipeReader {
   MessagePipeReader(mojo::ScopedMessagePipeHandle handle, Delegate* delegate);
   virtual ~MessagePipeReader();
 
-  MojoHandle handle() const { return pipe_.get().value(); }
+  MojoHandle handle() const { return handle_copy_; }
 
   // Returns received bytes.
   const std::vector<char>& data_buffer() const {
@@ -100,10 +104,18 @@ class MessagePipeReader {
   std::vector<char>  data_buffer_;
   std::vector<MojoHandle> handle_buffer_;
   mojo::ScopedMessagePipeHandle pipe_;
+  // Constant copy of the message pipe handle. For use by Send(), which can run
+  // concurrently on non-IO threads.
+  // TODO(amistry): This isn't quite right because handles can be re-used and

[Ken Rockot(use gerrit already), 2015/08/26 06:13:53]

yeah, this is a little unfortunate, but not a practical concern at the moment.
can you please file a bug to address this and include a link to it here?

[Anand Mistry (off Chromium), 2015/08/26 07:19:17]

Done.

+  // using this can run into the ABA problem. Currently, this is highly unlikely
+  // because Mojo internally uses an increasing uint32_t as handle values, but
+  // this could change.
+  const MojoHandle handle_copy_;
   // |delegate_| and |async_waiter_| are null once the message pipe is closed.
   Delegate* delegate_;
   scoped_ptr<AsyncHandleWaiter> async_waiter_;
-  MojoResult pending_send_error_;
+  base::subtle::Atomic32 pending_send_error_;
+  base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(MessagePipeReader);
 };