diff options
4 files changed, 76 insertions, 58 deletions
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index af72eb9bcb..77c222290e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -121,11 +121,6 @@ constexpr nsecs_t SLOW_EVENT_PROCESSING_WARNING_TIMEOUT = 2000 * 1000000LL; // 2 // Log a warning when an interception call takes longer than this to process. constexpr std::chrono::milliseconds SLOW_INTERCEPTION_THRESHOLD = 50ms; -// Additional key latency in case a connection is still processing some motion events. -// This will help with the case when a user touched a button that opens a new window, -// and gives us the chance to dispatch the key to this new window. -constexpr std::chrono::nanoseconds KEY_WAITING_FOR_EVENTS_TIMEOUT = 500ms; - // Number of recent events to keep for debugging purposes. constexpr size_t RECENT_QUEUE_MAX_SIZE = 10; @@ -1192,7 +1187,7 @@ bool InputDispatcher::isStaleEvent(nsecs_t currentTime, const EventEntry& entry) * Return true if the events preceding this incoming motion event should be dropped * Return false otherwise (the default behaviour) */ -bool InputDispatcher::shouldPruneInboundQueueLocked(const MotionEntry& motionEntry) { +bool InputDispatcher::shouldPruneInboundQueueLocked(const MotionEntry& motionEntry) const { const bool isPointerDownEvent = motionEntry.action == AMOTION_EVENT_ACTION_DOWN && isFromSource(motionEntry.source, AINPUT_SOURCE_CLASS_POINTER); @@ -1234,16 +1229,6 @@ bool InputDispatcher::shouldPruneInboundQueueLocked(const MotionEntry& motionEnt } } - // Prevent getting stuck: if we have a pending key event, and some motion events that have not - // yet been processed by some connections, the dispatcher will wait for these motion - // events to be processed before dispatching the key event. This is because these motion events - // may cause a new window to be launched, which the user might expect to receive focus. - // To prevent waiting forever for such events, just send the key to the currently focused window - if (isPointerDownEvent && mKeyIsWaitingForEventsTimeout) { - ALOGD("Received a new pointer down event, stop waiting for events to process and " - "just send the pending key event to the focused window."); - mKeyIsWaitingForEventsTimeout = now(); - } return false; } @@ -1291,6 +1276,20 @@ bool InputDispatcher::enqueueInboundEventLocked(std::unique_ptr<EventEntry> newE mNextUnblockedEvent = mInboundQueue.back(); needWake = true; } + + const bool isPointerDownEvent = motionEntry.action == AMOTION_EVENT_ACTION_DOWN && + isFromSource(motionEntry.source, AINPUT_SOURCE_CLASS_POINTER); + if (isPointerDownEvent && mKeyIsWaitingForEventsTimeout) { + // Prevent waiting too long for unprocessed events: if we have a pending key event, + // and some other events have not yet been processed, the dispatcher will wait for + // these events to be processed before dispatching the key event. This is because + // the unprocessed events may cause the focus to change (for example, by launching a + // new window or tapping a different window). To prevent waiting too long, we force + // the key to be sent to the currently focused window when a new tap comes in. + ALOGD("Received a new pointer down event, stop waiting for events to process and " + "just send the pending key event to the currently focused window."); + mKeyIsWaitingForEventsTimeout = now(); + } break; } case EventEntry::Type::FOCUS: { @@ -2137,7 +2136,8 @@ bool InputDispatcher::shouldWaitToSendKeyLocked(nsecs_t currentTime, // Start the timer // Wait to send key because there are unprocessed events that may cause focus to change mKeyIsWaitingForEventsTimeout = currentTime + - std::chrono::duration_cast<std::chrono::nanoseconds>(KEY_WAITING_FOR_EVENTS_TIMEOUT) + std::chrono::duration_cast<std::chrono::nanoseconds>( + mPolicy.getKeyWaitingForEventsTimeout()) .count(); return true; } @@ -4879,7 +4879,7 @@ std::unique_ptr<VerifiedInputEvent> InputDispatcher::verifyInputEvent(const Inpu break; } default: { - ALOGE("Cannot verify events of type %" PRId32, event.getType()); + LOG(ERROR) << "Cannot verify events of type " << ftl::enum_string(event.getType()); return nullptr; } } diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 155d485ae4..dcd156675e 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -470,7 +470,7 @@ private: bool isStaleEvent(nsecs_t currentTime, const EventEntry& entry); - bool shouldPruneInboundQueueLocked(const MotionEntry& motionEntry) REQUIRES(mLock); + bool shouldPruneInboundQueueLocked(const MotionEntry& motionEntry) const REQUIRES(mLock); /** * Time to stop waiting for the events to be processed while trying to dispatch a key. diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h index 9e6209b3d9..62c2b02967 100644 --- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h +++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h @@ -131,6 +131,18 @@ public: return std::chrono::nanoseconds(currentTime - eventTime) >= STALE_EVENT_TIMEOUT; } + /** + * Get the additional latency to add while waiting for other input events to process before + * dispatching the pending key. + * If there are unprocessed events, the pending key will not be dispatched immediately. Instead, + * the dispatcher will wait for this timeout, to account for the possibility that the focus + * might change due to touch or other events (such as another app getting launched by keys). + * This would give the pending key the opportunity to go to a newly focused window instead. + */ + virtual std::chrono::nanoseconds getKeyWaitingForEventsTimeout() { + return KEY_WAITING_FOR_EVENTS_TIMEOUT; + } + /* Notifies the policy that a pointer down event has occurred outside the current focused * window. * @@ -150,6 +162,13 @@ public: /* Notifies the policy that there was an input device interaction with apps. */ virtual void notifyDeviceInteraction(DeviceId deviceId, nsecs_t timestamp, const std::set<gui::Uid>& uids) = 0; + +private: + // Additional key latency in case a connection is still processing some motion events. + // This will help with the case when a user touched a button that opens a new window, + // and gives us the chance to dispatch the key to this new window. + static constexpr std::chrono::nanoseconds KEY_WAITING_FOR_EVENTS_TIMEOUT = + std::chrono::milliseconds(500); }; } // namespace android diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 163eab078e..ab667d01fc 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -363,6 +363,8 @@ public: mInterceptKeyTimeout = timeout; } + std::chrono::nanoseconds getKeyWaitingForEventsTimeout() override { return 500ms; } + void setStaleEventTimeout(std::chrono::nanoseconds timeout) { mStaleEventTimeout = timeout; } void assertUserActivityNotPoked() { @@ -1083,8 +1085,8 @@ public: 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 @@ public: 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 @@ public: 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 @@ private: } 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 @@ public: 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 @@ TEST_F(InputDispatcherSingleWindowAnr, Policy_DoesNotGetDuplicateAnr) { /** * 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 @@ TEST_F(InputDispatcherSingleWindowAnr, Key_StaysPendingWhileMotionIsProcessed) { 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 @@ TEST_F(InputDispatcherSingleWindowAnr, Key_StaysPendingWhileMotionIsProcessed) { * 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 @@ TEST_F(InputDispatcherSingleWindowAnr, .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)); |