Make a fairer combined message loop

base/message_loop/message_loop.cc

 // Copyright 2013 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.
 
 #include "base/message_loop/message_loop.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 125 matching lines @@
 
 MessageLoop::MessageLoop(Type type)
     : type_(type),
       nestable_tasks_allowed_(true),
       exception_restoration_(false),
       message_histogram_(NULL),
       run_loop_(NULL),
 #if defined(OS_WIN)
       os_modal_loop_(false),
 #endif  // OS_WIN
-      next_sequence_num_(0) {
+      next_sequence_num_(0),
+      always_notify_pump_(false) {
   DCHECK(!current()) << "should only have one message loop per thread";
   lazy_tls_ptr.Pointer()->Set(this);
 
   message_loop_proxy_ = new MessageLoopProxyImpl();
   thread_task_runner_handle_.reset(
       new ThreadTaskRunnerHandle(message_loop_proxy_));
 
 // TODO(rvargas): Get rid of the OS guards.
 #if defined(OS_WIN)
 #define MESSAGE_PUMP_UI new MessagePumpForUI()
@@ skipping 22 matching lines @@
     if (message_pump_for_ui_factory_)
       pump_ = message_pump_for_ui_factory_();
     else
       pump_ = MESSAGE_PUMP_UI;
   } else if (type_ == TYPE_IO) {
     pump_ = MESSAGE_PUMP_IO;
   } else {
     DCHECK_EQ(TYPE_DEFAULT, type_);
     pump_ = new MessagePumpDefault();
   }
+  always_notify_pump_ = pump_->NeedsScheduleWorkPerTask();

[awong, 2013/07/03 19:09:22]

Always_notify_pump_ seems to effectively be a platform constant. It almost seems
cleaner to just keep the #if define(OS_ANDROID) in this file and avoid
introducing the new variable + method.

[Kristian Monsen, 2013/07/03 21:20:03]

I had a CL doing this locally, was not sure which one was best. I kind of liked
what you are describing, but one of the reviewer thought this was better.

One nice side effect of keeping it local is that reverting will be cleaner ...

 }
 
 MessageLoop::~MessageLoop() {
   DCHECK_EQ(this, current());
 
   DCHECK(!run_loop_);
 
   // Clean up any unprocessed tasks, but take care: deleting a task could
   // result in the addition of more tasks (e.g., via DeleteSoon).  We set a
   // limit on the number of times we will allow a deleted task to generate more
@@ skipping 418 matching lines @@
     // 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(GetTaskTraceID(*pending_task, this)));
 
     bool was_empty = incoming_queue_.empty();
     incoming_queue_.push(*pending_task);
     pending_task->task.Reset();
-    if (!was_empty)
+    // The Android UI message loop needs to get notified each time
+    // a task is added to the incoming queue

[awong, 2013/07/03 19:09:22]

nit: Please end sentences with a period.

[Kristian Monsen, 2013/07/03 21:20:03]

Done.

+    if (!was_empty && !always_notify_pump_)
       return true;  // Someone else should have started the sub-pump.
 
     pump = pump_;
   }
   // Since the incoming_queue_ may contain a task that destroys this message
   // loop, we cannot exit incoming_queue_lock_ until we are done with |this|.
   // We use a stack-based reference to the message pump so that we can call
   // ScheduleWork outside of incoming_queue_lock_.
 
   pump->ScheduleWork();
@@ skipping 187 matching lines @@
       fd,
       persistent,
       mode,
       controller,
       delegate);
 }
 
 #endif
 
 }  // namespace base