Make a fairer combined message loop

This will make us insert a new Message into the java message loop each time a new task is added. When those
tasks are handled we will handle one native task.

This is not the full truth as the Android DoRunLoopOnce does not actually just run the loop once, but that is for a later CL to clear up.

BUG=250924

Review URL: https://chromiumcodereview.appspot.com/18181011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211303 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 35 insertions, 60 deletions

diff --git a/base/android/java/src/org/chromium/base/SystemMessageHandler.java b/base/android/java/src/org/chromium/base/SystemMessageHandler.java
index f7bb19f..fc25ff8 100644
--- a/base/android/java/src/org/chromium/base/SystemMessageHandler.java
+++ b/base/android/java/src/org/chromium/base/SystemMessageHandler.java
@@ -18,70 +18,32 @@ class SystemMessageHandler extends Handler {
     // Native class pointer set by the constructor of the SharedClient native class.
     private int mMessagePumpDelegateNative = 0;
 
-    // Used to ensure we have at most one TIMER_MESSAGE pending at once.
-    private AtomicBoolean mTimerFired = new AtomicBoolean(true);
-
-    // Used to insert TIMER_MESSAGE on the front of the system message queue during startup only.
-    // This is a wee hack, to give a priority boost to native tasks during startup as they tend to
-    // be on the critical path. (After startup, handling the UI with minimum latency is more
-    // important).
-    private boolean mStartupComplete = false;
-    private final long mStartupCompleteTime = System.currentTimeMillis() + 2000;
-    private final boolean startupComplete() {
-        if (!mStartupComplete && System.currentTimeMillis() > mStartupCompleteTime) {
-            mStartupComplete = true;
-        }
-        return mStartupComplete;
-    }
-
     private SystemMessageHandler(int messagePumpDelegateNative) {
         mMessagePumpDelegateNative = messagePumpDelegateNative;
     }
 
     @Override
     public void handleMessage(Message msg) {
-        if (msg.what == TIMER_MESSAGE) {
-            mTimerFired.set(true);
-        }
-        while (nativeDoRunLoopOnce(mMessagePumpDelegateNative)) {
-            if (startupComplete()) {
-                setTimer();
-                break;
-            }
-        }
+        nativeDoRunLoopOnce(mMessagePumpDelegateNative);
     }
 
+    @SuppressWarnings("unused")
     @CalledByNative
     private void setTimer() {
-        if (!mTimerFired.getAndSet(false)) {
-            // mTimerFired was already false.
-            return;
-        }
-        if (startupComplete()) {
-            sendEmptyMessage(TIMER_MESSAGE);
-        } else {
-            sendMessageAtFrontOfQueue(obtainMessage(TIMER_MESSAGE));
-        }
+        sendEmptyMessage(TIMER_MESSAGE);
     }
 
-    // If millis <=0, it'll send a TIMER_MESSAGE instead of
-    // a DELAYED_TIMER_MESSAGE.
     @SuppressWarnings("unused")
     @CalledByNative
     private void setDelayedTimer(long millis) {
-        if (millis <= 0) {
-            setTimer();
-        } else {
-            removeMessages(DELAYED_TIMER_MESSAGE);
-            sendEmptyMessageDelayed(DELAYED_TIMER_MESSAGE, millis);
-        }
+        removeMessages(DELAYED_TIMER_MESSAGE);
+        sendEmptyMessageDelayed(DELAYED_TIMER_MESSAGE, millis);
     }
 
     @SuppressWarnings("unused")
     @CalledByNative
     private void removeTimer() {
         removeMessages(TIMER_MESSAGE);
-        removeMessages(DELAYED_TIMER_MESSAGE);
     }
 
     @CalledByNative
@@ -89,5 +51,5 @@ class SystemMessageHandler extends Handler {
         return new SystemMessageHandler(messagePumpDelegateNative);
     }
 
-    private native boolean nativeDoRunLoopOnce(int messagePumpDelegateNative);
+    private native void nativeDoRunLoopOnce(int messagePumpDelegateNative);
 }
diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index 96192a7..bd3542a 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -97,6 +97,16 @@ uint64 GetTaskTraceID(const PendingTask& task, MessageLoop* loop) {
          static_cast<uint64>(reinterpret_cast<intptr_t>(loop));
 }
 
+// Returns true if MessagePump::ScheduleWork() must be called one
+// time for every task that is added to the MessageLoop incoming queue.
+bool AlwaysNotifyPump(MessageLoop::Type type) {
+#if defined(OS_ANDROID)
+  return type == MessageLoop::TYPE_UI;
+#else
+  return false;
+#endif
+}
+
 }  // namespace
 
 //------------------------------------------------------------------------------
@@ -627,7 +637,9 @@ bool MessageLoop::AddToIncomingQueue(PendingTask* pending_task,
     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.
+    if (!was_empty && !AlwaysNotifyPump(type_))
       return true;  // Someone else should have started the sub-pump.
 
     pump = pump_;
diff --git a/base/message_loop/message_pump_android.cc b/base/message_loop/message_pump_android.cc
index 70eb6ef..f3f1c9b 100644
--- a/base/message_loop/message_pump_android.cc
+++ b/base/message_loop/message_pump_android.cc
@@ -21,7 +21,7 @@ using base::android::ScopedJavaLocalRef;
 // ----------------------------------------------------------------------------
 // This method can not move to anonymous namespace as it has been declared as
 // 'static' in system_message_handler_jni.h.
-static jboolean DoRunLoopOnce(JNIEnv* env, jobject obj, jint native_delegate) {
+static void DoRunLoopOnce(JNIEnv* env, jobject obj, jint native_delegate) {
   base::MessagePump::Delegate* delegate =
       reinterpret_cast<base::MessagePump::Delegate*>(native_delegate);
   DCHECK(delegate);
@@ -31,26 +31,27 @@ static jboolean DoRunLoopOnce(JNIEnv* env, jobject obj, jint native_delegate) {
   // we call DoWork() / DoDelayedWork().
   // On Android, the java message queue may contain messages for other handlers
   // that will be processed before calling here again.
-  bool more_work_is_plausible = delegate->DoWork();
+  bool did_work = delegate->DoWork();
 
   // This is the time when we need to do delayed work.
   base::TimeTicks delayed_work_time;
-  more_work_is_plausible |= delegate->DoDelayedWork(&delayed_work_time);
+  did_work |= delegate->DoDelayedWork(&delayed_work_time);
+
+  // Always call this if there is a delayed message waiting in the queue
+  // since is at most one delayed message in the Java message handler, and this
+  // function call may be the result of that message being handled.
+  if (!delayed_work_time.is_null()) {
+    Java_SystemMessageHandler_setDelayedTimer(env, obj,
+        (delayed_work_time - base::TimeTicks::Now()).InMillisecondsRoundedUp());
+  }
 
   // This is a major difference between android and other platforms: since we
   // can't inspect it and process just one single message, instead we'll yeld
-  // the callstack, and post a message to call us back soon.
-  if (more_work_is_plausible)
-    return true;
-
-  more_work_is_plausible = delegate->DoIdleWork();
-  if (!more_work_is_plausible && !delayed_work_time.is_null()) {
-    // We only set the timer here as returning true would post a message.
-    jlong millis =
-        (delayed_work_time - base::TimeTicks::Now()).InMillisecondsRoundedUp();
-    Java_SystemMessageHandler_setDelayedTimer(env, obj, millis);
-  }
-  return more_work_is_plausible;
+  // the callstack.
+  if (did_work)
+    return;
+
+  delegate->DoIdleWork();
 }
 
 namespace base {