Made MessagePump a non-thread safe class.

base/message_loop/message_loop_proxy_impl.h

Index: base/message_loop/message_loop_proxy_impl.h
diff --git a/base/message_loop/message_loop_proxy_impl.h b/base/message_loop/message_loop_proxy_impl.h
index 2269023ac2d19c0f9074fcadf181a3a9da1f3f51..9c4b676c4f43127bfcd5ca6115dc127c6eb69290 100644
--- a/base/message_loop/message_loop_proxy_impl.h
+++ b/base/message_loop/message_loop_proxy_impl.h
@@ -8,7 +8,9 @@
 #include "base/base_export.h"
 #include "base/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/pending_task.h"
 #include "base/synchronization/lock.h"
+#include "base/time/time.h"
 
 namespace base {
 
@@ -30,13 +32,10 @@ class BASE_EXPORT MessageLoopProxyImpl : public MessageLoopProxy {
  protected:
   virtual ~MessageLoopProxyImpl();
 
-  // Override OnDestruct so that we can delete the object on the target message
-  // loop if it still exists.
-  virtual void OnDestruct() const OVERRIDE;
-
  private:
   // Allow the MessageLoop to create a MessageLoopProxyImpl.
   friend class MessageLoop;
+  friend class MessageLoopLockTest;
   friend class DeleteHelper<MessageLoopProxyImpl>;
 
   MessageLoopProxyImpl();
@@ -44,15 +43,67 @@ class BASE_EXPORT MessageLoopProxyImpl : public MessageLoopProxy {
   // Called directly by MessageLoop::~MessageLoop.
   virtual void WillDestroyCurrentMessageLoop();
 
+  // Acquires |incoming_queue_lock_| and appends a task to |incoming_queue_|.
+  // Posting of all tasks is routed though AddToIncomingQueue() or
+  // TryAddToIncomingQueue() to make sure that posting task is properly
+  // synchronized between different threads.
+  //
+  // Returns true if the task was successfully added to the queue, otherwise
+  // returns false. In all cases, the ownership of |task| is transferred to the
+  // called method.

[rvargas (doing something else), 2013/07/09 21:46:09]

nit: Given the number of private methods we should clearly state which methods
are supposed to be used by the message loop and which ones are truly private.

[alexeypa (please no reviews), 2013/07/09 23:37:40]

I converted MessageLoopProxyImpl into a private nested class declared in
MessageLoop. This way it remains an implementation detail of MessageLoop, but
MessageLoop can now call it public methods. |friend class MessageLoop| is not
needed any more. 

+  bool AddToIncomingQueue(const tracked_objects::Location& from_here,
+                          const Closure& task,
+                          TimeDelta delay,
+                          bool nestable);
+
+  // Calculates the time at which a PendingTask should run.
+  TimeTicks CalculateDelayedRuntime(TimeDelta delay);
+
+  // Returns true if the message loop has high resolution timers enabled.
+  // Provided for testing.
+  bool IsHishResolutionTimersEnabledForTest();
+
+  // Returns true if the message loop is "idle". Provided for testing.
+  bool IsIdleForTest();
+
+  // Same as AddToIncomingQueue() except that it will avoid blocking if the lock
+  // is already held, and will in that case (when the lock is contended) fail to
+  // add the task, and will return false.
+  bool TryAddToIncomingQueue(const tracked_objects::Location& from_here,
+                             const Closure& task);
+
+  // Adds a task to |incoming_queue_|. This method can be called on any thread.
+  // The caller is responsible for synchronizing calls to this method.

[rvargas (doing something else), 2013/07/09 21:46:09]

nit: This is a private method not used by anyone else so documenting here what
the caller is supposed to do looks out of place. (the part about
synchronization)

[alexeypa (please no reviews), 2013/07/09 23:37:40]

Done.

+  //
+  // In all cases, the caller retains ownership of |pending_task|, but this
+  // function will reset the value of |pending_task->task|. This is needed to
+  // ensure that the posting call stack does not retain |pending_task->task|
+  // beyond this function call.
+  bool PostPendingTask(PendingTask* pending_task);
+
+  // Loads tasks from the |incoming_queue_| into |*work_queue|. |*work_queue|
+  // must be empty. Accessing |incoming_queue_| requires a lock. The caller
+  // arranges synchronization for |*work_queue|.

[rvargas (doing something else), 2013/07/09 21:46:09]

nit: the last sentence is confusing. I understand that the point of the comment
is to state that this method doesn't do anything to synchronize work_queue, but
it may be easier to state that this method should be called from the thread that
is running the loop.

[alexeypa (please no reviews), 2013/07/09 23:37:40]

Done.

+  void ReloadWorkQueue(TaskQueue* work_queue);
+
+#if defined(OS_WIN)
+  TimeTicks high_resolution_timer_expiration_;
+#endif
+
+  // The lock that protects access to |incoming_queue_|, |message_loop_| and
+  // |next_sequence_num_|.
+  mutable base::Lock incoming_queue_lock_;

[rvargas (doing something else), 2013/07/09 21:46:09]

nit: it would be nice to have a better name here (given that the lock protects
more than the queue) but I'm short of suggestions.

[alexeypa (please no reviews), 2013/07/09 23:37:40]

I prefer to keep it |incoming_queue_lock_| unless someone comes up with a better
name. It is mainly all about managing the input queue even if it protects
something else too.

+
+  // An incoming queue of tasks that are acquired under a mutex for processing
+  // on this instance's thread. These tasks have not yet been been pushed to
+  // |message_loop_|.
+  TaskQueue incoming_queue_;
 
-  bool PostTaskHelper(const tracked_objects::Location& from_here,
-                      const base::Closure& task,
-                      base::TimeDelta delay,
-                      bool nestable);
+  // Points to the message loop that owns |this|.
+  MessageLoop* message_loop_;
 
-  // The lock that protects access to target_message_loop_.
-  mutable base::Lock message_loop_lock_;
-  MessageLoop* target_message_loop_;
+  // The next sequence number to use for delayed tasks.
+  int next_sequence_num_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageLoopProxyImpl);
 };