Made MessagePump a non-thread safe class.

base/message_loop/message_loop_proxy_impl.cc

Index: base/message_loop/message_loop_proxy_impl.cc
diff --git a/base/message_loop/message_loop_proxy_impl.cc b/base/message_loop/message_loop_proxy_impl.cc
index 7dc8caa9f4c047f93cf7366c2e373ad1eebdd0fb..b88faa7c2db40af57f246f13ad5f6485d4cded43 100644
--- a/base/message_loop/message_loop_proxy_impl.cc
+++ b/base/message_loop/message_loop_proxy_impl.cc
@@ -4,6 +4,7 @@
 
 #include "base/message_loop/message_loop_proxy_impl.h"
 
+#include "base/debug/trace_event.h"
 #include "base/location.h"
 #include "base/threading/thread_restrictions.h"
 
@@ -16,14 +17,16 @@ bool MessageLoopProxyImpl::PostDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
-  return PostTaskHelper(from_here, task, delay, true);
+  DCHECK(!task.is_null()) << from_here.ToString();
+  return AddToIncomingQueue(from_here, task, delay, true);
 }
 
 bool MessageLoopProxyImpl::PostNonNestableDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
-  return PostTaskHelper(from_here, task, delay, false);
+  DCHECK(!task.is_null()) << from_here.ToString();
+  return AddToIncomingQueue(from_here, task, delay, false);
 }
 
 bool MessageLoopProxyImpl::RunsTasksOnCurrentThread() const {
@@ -32,15 +35,24 @@ bool MessageLoopProxyImpl::RunsTasksOnCurrentThread() const {
   // function.
   // http://crbug.com/63678
   base::ThreadRestrictions::ScopedAllowSingleton allow_singleton;
-  AutoLock lock(message_loop_lock_);
-  return (target_message_loop_ &&
-          (MessageLoop::current() == target_message_loop_));
+  AutoLock lock(incoming_queue_lock_);
+  return (message_loop_ && (MessageLoop::current() == message_loop_));
 }
 
 // MessageLoop::DestructionObserver implementation
 void MessageLoopProxyImpl::WillDestroyCurrentMessageLoop() {
-  AutoLock lock(message_loop_lock_);
-  target_message_loop_ = NULL;
+#if defined(OS_WIN)
+  // If we left the high-resolution timer activated, deactivate it now.
+  // Doing this is not-critical, it is mainly to make sure we track
+  // the high resolution timer activations properly in our unit tests.
+  if (!high_resolution_timer_expiration_.is_null()) {
+    Time::ActivateHighResolutionTimer(false);
+    high_resolution_timer_expiration_ = TimeTicks();
+  }
+#endif
+
+  AutoLock lock(incoming_queue_lock_);
+  message_loop_ = NULL;
 }
 
 void MessageLoopProxyImpl::OnDestruct() const {
@@ -51,10 +63,9 @@ void MessageLoopProxyImpl::OnDestruct() const {
   base::ThreadRestrictions::ScopedAllowSingleton allow_singleton;
   bool delete_later = false;
   {
-    AutoLock lock(message_loop_lock_);
-    if (target_message_loop_ &&
-        (MessageLoop::current() != target_message_loop_)) {
-      target_message_loop_->DeleteSoon(FROM_HERE, this);
+    AutoLock lock(incoming_queue_lock_);
+    if (message_loop_ && (MessageLoop::current() != message_loop_)) {

[rvargas (doing something else), 2013/06/28 22:59:44]

This should be always false. (we should only reach refcount 0 after
WillDestroyCurrentML). DCHECK?

[alexeypa (please no reviews), 2013/07/03 18:56:48]

Done. I deleted OnDestruct and added DCHECK to the destructor.

+      message_loop_->DeleteSoon(FROM_HERE, this);
       delete_later = true;
     }
   }
@@ -63,22 +74,129 @@ void MessageLoopProxyImpl::OnDestruct() const {
 }
 
 MessageLoopProxyImpl::MessageLoopProxyImpl()
-    : target_message_loop_(MessageLoop::current()) {
+    : message_loop_(MessageLoop::current()),
+      next_sequence_num_(0) {
+}
+
+bool MessageLoopProxyImpl::AddToIncomingQueue(
+    const tracked_objects::Location& from_here,
+    const Closure& task,
+    TimeDelta delay,
+    bool nestable) {
+  PendingTask pending_task(
+      from_here, task, CalculateDelayedRuntime(delay), nestable);
+
+  AutoLock locked(incoming_queue_lock_);
+  return PostPendingTask(&pending_task);
 }
 
-bool MessageLoopProxyImpl::PostTaskHelper(
-    const tracked_objects::Location& from_here, const base::Closure& task,
-    base::TimeDelta delay, bool nestable) {
-  AutoLock lock(message_loop_lock_);
-  if (target_message_loop_) {
-    if (nestable) {
-      target_message_loop_->PostDelayedTask(from_here, task, delay);
-    } else {
-      target_message_loop_->PostNonNestableDelayedTask(from_here, task, delay);
+TimeTicks MessageLoopProxyImpl::CalculateDelayedRuntime(TimeDelta delay) {
+  TimeTicks delayed_run_time;
+  if (delay > TimeDelta()) {
+    delayed_run_time = TimeTicks::Now() + delay;
+
+#if defined(OS_WIN)
+    if (high_resolution_timer_expiration_.is_null()) {
+      // Windows timers are granular to 15.6ms.  If we only set high-res
+      // timers for those under 15.6ms, then a 18ms timer ticks at ~32ms,
+      // which as a percentage is pretty inaccurate.  So enable high
+      // res timers for any timer which is within 2x of the granularity.
+      // This is a tradeoff between accuracy and power management.
+      bool needs_high_res_timers = delay.InMilliseconds() <
+          (2 * Time::kMinLowResolutionThresholdMs);
+      if (needs_high_res_timers) {
+        if (Time::ActivateHighResolutionTimer(true)) {
+          high_resolution_timer_expiration_ = TimeTicks::Now() +

[rvargas (doing something else), 2013/06/28 22:59:44]

This looks like racing to me (same thing on the old code). Not that it matters,
but tools may start complaining about it. Actually, ActivateHighResolutionTimer
seem to have bigger issues when racing. It may be a good idea to push the whole
thing behind the lock.

[alexeypa (please no reviews), 2013/07/03 18:56:48]

Done.

+              TimeDelta::FromMilliseconds(
+                  MessageLoop::kHighResolutionTimerModeLeaseTimeMs);
+        }
+      }
+    }
+#endif
+  } else {
+    DCHECK_EQ(delay.InMilliseconds(), 0) << "delay should not be negative";
+  }
+
+#if defined(OS_WIN)
+  if (!high_resolution_timer_expiration_.is_null()) {
+    if (TimeTicks::Now() > high_resolution_timer_expiration_) {
+      Time::ActivateHighResolutionTimer(false);
+      high_resolution_timer_expiration_ = TimeTicks();
     }
-    return true;
   }
-  return false;
+#endif
+
+  return delayed_run_time;
+}
+
+bool MessageLoopProxyImpl::IsHishResolutionTimersEnabledForTest() {
+#if defined(OS_WIN)
+  return !high_resolution_timer_expiration_.is_null();
+#else
+  return true;
+#endif
+}
+
+bool MessageLoopProxyImpl::IsIdleForTest() {
+  AutoLock lock(incoming_queue_lock_);
+  return incoming_queue_.empty();
+}
+
+bool MessageLoopProxyImpl::TryAddToIncomingQueue(
+    const tracked_objects::Location& from_here,
+    const Closure& task) {
+  PendingTask pending_task(
+      from_here, task, CalculateDelayedRuntime(TimeDelta()), true);
+  if (!incoming_queue_lock_.Try()) {
+    pending_task.task.Reset();
+    return false;
+  }
+
+  AutoLock locked(incoming_queue_lock_, AutoLock::AlreadyAcquired());
+  return PostPendingTask(&pending_task);
+}
+
+bool MessageLoopProxyImpl::PostPendingTask(PendingTask* pending_task) {
+  // Warning: Don't try to short-circuit, and handle this thread's tasks more
+  // directly, as it could starve handling of foreign threads.  Put every task
+  // into this queue.
+
+  // This should only be called while the lock is taken.
+  incoming_queue_lock_.AssertAcquired();
+
+  // Initialize the sequence number. The sequence number is used for delayed
+  // tasks (to faciliate FIFO sorting when two tasks have the same
+  // delayed_run_time value) and for identifying the task in about:tracing.
+  pending_task->sequence_num = next_sequence_num_++;
+
+  TRACE_EVENT_FLOW_BEGIN0("task", "MessageLoop::PostTask",
+      TRACE_ID_MANGLE(message_loop_->GetTaskTraceID(*pending_task)));
+
+  bool was_empty = incoming_queue_.empty();
+  incoming_queue_.push(*pending_task);
+  pending_task->task.Reset();
+
+  // Wake up the pump.
+  if (was_empty && message_loop_)
+    message_loop_->StartPump();
+
+  return !!message_loop_;

[rvargas (doing something else), 2013/06/28 22:59:44]

early return before adding the task to the queue.

[alexeypa (please no reviews), 2013/07/03 18:56:48]

Done.

+}
+
+void MessageLoopProxyImpl::ReloadWorkQueue(TaskQueue* work_queue) {
+  // We can improve performance of our loading tasks from |incoming_queue_| to
+  // |*work_queue| by waiting until the last minute (|*work_queue| is empty) to
+  // load. That reduces the number of locks-per-task significantly when our
+  // queues get large.
+  if (!work_queue->empty())

[rvargas (doing something else), 2013/06/28 22:59:44]

nit: we should probably do this check at the caller side.

[alexeypa (please no reviews), 2013/07/03 18:56:48]

Done.

+    return;  // Wait till we *really* need to lock and load.
+
+  // Acquire all we can from the inter-thread queue with one lock acquisition.
+  AutoLock lock(incoming_queue_lock_);
+  if (!incoming_queue_.empty())
+    incoming_queue_.Swap(work_queue);  // Constant time
+
+  DCHECK(incoming_queue_.empty());
 }
 
 scoped_refptr<MessageLoopProxy>