Update mKeyIsWaitingForEventsTimeout separately from the prune check
Before this CL, the variable mKeyIsWaitingForEventsTimeout was getting
updated inside 'shouldPruneInboundQueueLocked', but that is not the
correct place to do this update. In this CL, we move this update to the
enqueue function, and this allows us to make
'shouldPruneInboundQueueLocked' const.
The current behaviour of:
```
When there are unprocessed motion events, we wait up to 500 ms to send
any key events, in anticipation that focus might change. However, if
there are subsequent "ACTION_DOWN" events, we don't want to be adding
this 500ms wait, so we short-circuit the timeout and process the pending
keys immediately (send them to the currently focused window).
```
is not changed in this CL.
However, we are updating the test for this behaviour to:
- Be correctly named. The pending key is not dropped, but it's processed
immediately instead.
- Actually test the behaviour in question. Previously, the test passed
even with the 'mKeyIsWaitingForEventsTimeout' not getting updated
during the queueing stage. This is because the test used a large
(1000ms) timeout for event consumption. So in practice, the test was
still waiting for 500 ms and only then the consumption passed.
In this CL, the test is changed so that it consumes within the allotted
timeframe. If this becomes problematic (flaky), we would need to change
the KEY_WAITING_FOR_EVENTS_TIMEOUT and possibly provide that value from
the policy, thus allowing the test to override it. Or, another way to
fix flakes here would be to switch to an injectable clock, although in
that case we would still ideally open up KEY_WAITING_FOR_EVENTS_TIMEOUT
for tests, or maybe just hardcode the value with a comment, similarly to
how it's currently done.
Bug: 294553757
Test: TEST=inputflinger_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST
Change-Id: Ife71634960b365df1d106fd28f0dc0047ac2ed2b
diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp
index 163eab0..ab667d0 100644
--- a/services/inputflinger/tests/InputDispatcher_test.cpp
+++ b/services/inputflinger/tests/InputDispatcher_test.cpp
@@ -363,6 +363,8 @@
mInterceptKeyTimeout = timeout;
}
+ std::chrono::nanoseconds getKeyWaitingForEventsTimeout() override { return 500ms; }
+
void setStaleEventTimeout(std::chrono::nanoseconds timeout) { mStaleEventTimeout = timeout; }
void assertUserActivityNotPoked() {
@@ -1083,8 +1085,8 @@
EXPECT_EQ(inTouchMode, touchModeEvent.isInTouchMode());
}
- void assertNoEvents() {
- std::unique_ptr<InputEvent> event = consume(CONSUME_TIMEOUT_NO_EVENT_EXPECTED);
+ void assertNoEvents(std::chrono::milliseconds timeout) {
+ std::unique_ptr<InputEvent> event = consume(timeout);
if (event == nullptr) {
return;
}
@@ -1412,14 +1414,14 @@
mInputReceiver->sendTimeline(inputEventId, timeline);
}
- void assertNoEvents() {
+ void assertNoEvents(std::chrono::milliseconds timeout = CONSUME_TIMEOUT_NO_EVENT_EXPECTED) {
if (mInputReceiver == nullptr &&
mInfo.inputConfig.test(WindowInfo::InputConfig::NO_INPUT_CHANNEL)) {
return; // Can't receive events if the window does not have input channel
}
ASSERT_NE(nullptr, mInputReceiver)
<< "Window without InputReceiver must specify feature NO_INPUT_CHANNEL";
- mInputReceiver->assertNoEvents();
+ mInputReceiver->assertNoEvents(timeout);
}
sp<IBinder> getToken() { return mInfo.token; }
@@ -1437,13 +1439,6 @@
int getChannelFd() { return mInputReceiver->getChannelFd(); }
-private:
- FakeWindowHandle(std::string name) : mName(name){};
- const std::string mName;
- std::shared_ptr<FakeInputReceiver> mInputReceiver;
- static std::atomic<int32_t> sId; // each window gets a unique id, like in surfaceflinger
- friend class sp<FakeWindowHandle>;
-
std::unique_ptr<InputEvent> consume(std::chrono::milliseconds timeout, bool handled = true) {
if (mInputReceiver == nullptr) {
LOG(FATAL) << "Cannot consume event from a window with no input event receiver";
@@ -1454,6 +1449,13 @@
}
return event;
}
+
+private:
+ FakeWindowHandle(std::string name) : mName(name){};
+ const std::string mName;
+ std::shared_ptr<FakeInputReceiver> mInputReceiver;
+ static std::atomic<int32_t> sId; // each window gets a unique id, like in surfaceflinger
+ friend class sp<FakeWindowHandle>;
};
std::atomic<int32_t> FakeWindowHandle::sId{1};
@@ -1512,7 +1514,7 @@
std::unique_ptr<MotionEvent> consumeMotion() { return mInputReceiver.consumeMotion(); }
- void assertNoEvents() { mInputReceiver.assertNoEvents(); }
+ void assertNoEvents() { mInputReceiver.assertNoEvents(CONSUME_TIMEOUT_NO_EVENT_EXPECTED); }
private:
FakeInputReceiver mInputReceiver;
@@ -8824,16 +8826,11 @@
/**
* If a window is processing a motion event, and then a key event comes in, the key event should
* not get delivered to the focused window until the motion is processed.
- *
- * Warning!!!
- * This test depends on the value of android::inputdispatcher::KEY_WAITING_FOR_MOTION_TIMEOUT
- * and the injection timeout that we specify when injecting the key.
- * We must have the injection timeout (100ms) be smaller than
- * KEY_WAITING_FOR_MOTION_TIMEOUT (currently 500ms).
- *
- * If that value changes, this test should also change.
*/
TEST_F(InputDispatcherSingleWindowAnr, Key_StaysPendingWhileMotionIsProcessed) {
+ // The timeouts in this test are established by relying on the fact that the "key waiting for
+ // events timeout" is equal to 500ms.
+ ASSERT_EQ(mFakePolicy->getKeyWaitingForEventsTimeout(), 500ms);
mWindow->setDispatchingTimeout(2s); // Set a long ANR timeout to prevent it from triggering
mDispatcher->onWindowInfosChanged({{*mWindow->getInfo()}, {}, 0, 0});
@@ -8842,23 +8839,18 @@
ASSERT_TRUE(downSequenceNum);
const auto& [upSequenceNum, upEvent] = mWindow->receiveEvent();
ASSERT_TRUE(upSequenceNum);
- // Don't finish the events yet, and send a key
- // Injection will "succeed" because we will eventually give up and send the key to the focused
- // window even if motions are still being processed. But because the injection timeout is short,
- // we will receive INJECTION_TIMED_OUT as the result.
- InputEventInjectionResult result =
- injectKey(*mDispatcher, AKEY_EVENT_ACTION_DOWN, /*repeatCount=*/0, ADISPLAY_ID_DEFAULT,
- InputEventInjectionSync::WAIT_FOR_RESULT, 100ms);
- ASSERT_EQ(InputEventInjectionResult::TIMED_OUT, result);
+ // Don't finish the events yet, and send a key
+ mDispatcher->notifyKey(
+ KeyArgsBuilder(AKEY_EVENT_ACTION_DOWN, AINPUT_SOURCE_KEYBOARD)
+ .policyFlags(DEFAULT_POLICY_FLAGS | POLICY_FLAG_DISABLE_KEY_REPEAT)
+ .build());
// Key will not be sent to the window, yet, because the window is still processing events
// and the key remains pending, waiting for the touch events to be processed
// Make sure that `assertNoEvents` doesn't wait too long, because it could cause an ANR.
- // Rely here on the fact that it uses CONSUME_TIMEOUT_NO_EVENT_EXPECTED under the hood.
- static_assert(CONSUME_TIMEOUT_NO_EVENT_EXPECTED < 100ms);
- mWindow->assertNoEvents();
+ mWindow->assertNoEvents(100ms);
- std::this_thread::sleep_for(500ms);
+ std::this_thread::sleep_for(400ms);
// if we wait long enough though, dispatcher will give up, and still send the key
// to the focused window, even though we have not yet finished the motion event
mWindow->consumeKeyDown(ADISPLAY_ID_DEFAULT);
@@ -8873,7 +8865,10 @@
* focused window right away.
*/
TEST_F(InputDispatcherSingleWindowAnr,
- PendingKey_IsDroppedWhileMotionIsProcessedAndNewTouchComesIn) {
+ PendingKey_IsDeliveredWhileMotionIsProcessingAndNewTouchComesIn) {
+ // The timeouts in this test are established by relying on the fact that the "key waiting for
+ // events timeout" is equal to 500ms.
+ ASSERT_EQ(mFakePolicy->getKeyWaitingForEventsTimeout(), 500ms);
mWindow->setDispatchingTimeout(2s); // Set a long ANR timeout to prevent it from triggering
mDispatcher->onWindowInfosChanged({{*mWindow->getInfo()}, {}, 0, 0});
@@ -8888,15 +8883,19 @@
.policyFlags(DEFAULT_POLICY_FLAGS | POLICY_FLAG_DISABLE_KEY_REPEAT)
.build());
// At this point, key is still pending, and should not be sent to the application yet.
- // Make sure the `assertNoEvents` check doesn't take too long. It uses
- // CONSUME_TIMEOUT_NO_EVENT_EXPECTED under the hood.
- static_assert(CONSUME_TIMEOUT_NO_EVENT_EXPECTED < 100ms);
- mWindow->assertNoEvents();
+ mWindow->assertNoEvents(100ms);
// Now tap down again. It should cause the pending key to go to the focused window right away.
tapOnWindow();
- mWindow->consumeKeyEvent(WithKeyAction(AKEY_EVENT_ACTION_DOWN)); // it doesn't matter that we
- // haven't ack'd the other events yet. We can finish events in any order.
+ // Now that we tapped, we should receive the key immediately.
+ // Since there's still room for slowness, we use 200ms, which is much less than
+ // the "key waiting for events' timeout of 500ms minus the already waited 100ms duration.
+ std::unique_ptr<InputEvent> keyEvent = mWindow->consume(200ms);
+ ASSERT_NE(nullptr, keyEvent);
+ ASSERT_EQ(InputEventType::KEY, keyEvent->getType());
+ ASSERT_THAT(static_cast<KeyEvent&>(*keyEvent), WithKeyAction(AKEY_EVENT_ACTION_DOWN));
+ // it doesn't matter that we haven't ack'd the other events yet. We can finish events in any
+ // order.
mWindow->finishEvent(*downSequenceNum); // first tap's ACTION_DOWN
mWindow->finishEvent(*upSequenceNum); // first tap's ACTION_UP
mWindow->consumeMotionEvent(WithMotionAction(ACTION_DOWN));