From 17761fff2a5ba2e35b84c932c58508c43191ea38 Mon Sep 17 00:00:00 2001
From: Michael Wright <michaelwr@google.com>
Date: Tue, 23 Jun 2020 01:33:08 +0100
Subject: Isolate batched input scheduling logic.

Having whether something is currently scheduled get modified in the
actual processing logic and away from the scheduling logic makes it
unnecessarily more difficult to reason about.

In this case, we were only checking whether the Choreographer-aligned
runnable was actually scheduled before we'd consume input. When trying
to unbuffer dispatches so that things were no longer
Choreographer-aligned, however, we'd unschedule the that runnable just
before we tried to consume input and hence just before the check . This
meant we never actually consumed the batches until the next one came in
where we would then consume them immediately.  Worse, if another one
never came in then we'd never actually consume the batch.

By removing the scheduling logic from the processing logic we avoid this
situation. Now only the schedule, unschedule, and actual runnable logic
touch their own scheduled state, so that we don't have to consider the
two pieces of logic together except in isolated places.

Bug: 158540186
Bug: 159239700

Test: manual, using app that injects unbuffered dispatch requests either
      at ACTION_DOWN or first ACTION_MOVE
Change-Id: I7521a5a16ed52afc41261f501128b5498ea0602c
---
 core/java/android/view/ViewRootImpl.java | 37 +++++++++++++++++---------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index b860bac0d001..a15f66a60281 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -8107,7 +8107,9 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     void scheduleConsumeBatchedInput() {
-        if (!mConsumeBatchedInputScheduled) {
+        // If anything is currently scheduled to consume batched input then there's no point in
+        // scheduling it again.
+        if (!mConsumeBatchedInputScheduled && !mConsumeBatchedInputImmediatelyScheduled) {
             mConsumeBatchedInputScheduled = true;
             mChoreographer.postCallback(Choreographer.CALLBACK_INPUT,
                     mConsumedBatchedInputRunnable, null);
@@ -8130,22 +8132,15 @@ public final class ViewRootImpl implements ViewParent,
         }
     }
 
-    void doConsumeBatchedInput(long frameTimeNanos) {
-        if (mConsumeBatchedInputScheduled) {
-            mConsumeBatchedInputScheduled = false;
-            if (mInputEventReceiver != null) {
-                if (mInputEventReceiver.consumeBatchedInputEvents(frameTimeNanos)
-                        && frameTimeNanos != -1) {
-                    // If we consumed a batch here, we want to go ahead and schedule the
-                    // consumption of batched input events on the next frame. Otherwise, we would
-                    // wait until we have more input events pending and might get starved by other
-                    // things occurring in the process. If the frame time is -1, however, then
-                    // we're in a non-batching mode, so there's no need to schedule this.
-                    scheduleConsumeBatchedInput();
-                }
-            }
-            doProcessInputEvents();
+    boolean doConsumeBatchedInput(long frameTimeNanos) {
+        final boolean consumedBatches;
+        if (mInputEventReceiver != null) {
+            consumedBatches = mInputEventReceiver.consumeBatchedInputEvents(frameTimeNanos);
+        } else {
+            consumedBatches = false;
         }
+        doProcessInputEvents();
+        return consumedBatches;
     }
 
     final class TraversalRunnable implements Runnable {
@@ -8218,7 +8213,14 @@ public final class ViewRootImpl implements ViewParent,
     final class ConsumeBatchedInputRunnable implements Runnable {
         @Override
         public void run() {
-            doConsumeBatchedInput(mChoreographer.getFrameTimeNanos());
+            mConsumeBatchedInputScheduled = false;
+            if (doConsumeBatchedInput(mChoreographer.getFrameTimeNanos())) {
+                // If we consumed a batch here, we want to go ahead and schedule the
+                // consumption of batched input events on the next frame. Otherwise, we would
+                // wait until we have more input events pending and might get starved by other
+                // things occurring in the process.
+                scheduleConsumeBatchedInput();
+            }
         }
     }
     final ConsumeBatchedInputRunnable mConsumedBatchedInputRunnable =
@@ -8228,6 +8230,7 @@ public final class ViewRootImpl implements ViewParent,
     final class ConsumeBatchedInputImmediatelyRunnable implements Runnable {
         @Override
         public void run() {
+            mConsumeBatchedInputImmediatelyScheduled = false;
             doConsumeBatchedInput(-1);
         }
     }
-- 
cgit v1.2.3-59-g8ed1b