Use sequence numbers to synchronize enabling Pointer Capture (1/2)
InputReader only processes configuration change from its main thread.
This means that if there is more than one Pointer Capture change
request when the thread is busy or sleeping, it will only process the
latest one. To ensure requests to enable Pointer Capture are synchronized
with Dispatcher, we must use sequence numbers.
Requests to enable Pointer Capture have a sequence number. Requests to
disable Pointer Capture do not have a value.
Bug: 195312888
Test: atest inputflinger_tests
Test: manual with GeforceNow app, see bug.
Merged-In: I6ae9c5498dc2f783b4c7211fa3665d42e29d2919
Change-Id: I6ae9c5498dc2f783b4c7211fa3665d42e29d2919
diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp
index f884a4b..dc04a6f 100644
--- a/services/inputflinger/tests/InputDispatcher_test.cpp
+++ b/services/inputflinger/tests/InputDispatcher_test.cpp
@@ -242,19 +242,22 @@
mConfig.keyRepeatDelay = delay;
}
- void waitForSetPointerCapture(bool enabled) {
+ PointerCaptureRequest assertSetPointerCaptureCalled(bool enabled) {
std::unique_lock lock(mLock);
base::ScopedLockAssertion assumeLocked(mLock);
if (!mPointerCaptureChangedCondition.wait_for(lock, 100ms,
[this, enabled]() REQUIRES(mLock) {
- return mPointerCaptureEnabled &&
- *mPointerCaptureEnabled ==
+ return mPointerCaptureRequest->enable ==
enabled;
})) {
- FAIL() << "Timed out waiting for setPointerCapture(" << enabled << ") to be called.";
+ ADD_FAILURE() << "Timed out waiting for setPointerCapture(" << enabled
+ << ") to be called.";
+ return {};
}
- mPointerCaptureEnabled.reset();
+ auto request = *mPointerCaptureRequest;
+ mPointerCaptureRequest.reset();
+ return request;
}
void assertSetPointerCaptureNotCalled() {
@@ -262,11 +265,11 @@
base::ScopedLockAssertion assumeLocked(mLock);
if (mPointerCaptureChangedCondition.wait_for(lock, 100ms) != std::cv_status::timeout) {
- FAIL() << "Expected setPointerCapture(enabled) to not be called, but was called. "
+ FAIL() << "Expected setPointerCapture(request) to not be called, but was called. "
"enabled = "
- << *mPointerCaptureEnabled;
+ << std::to_string(mPointerCaptureRequest->enable);
}
- mPointerCaptureEnabled.reset();
+ mPointerCaptureRequest.reset();
}
void assertDropTargetEquals(const sp<IBinder>& targetToken) {
@@ -284,7 +287,8 @@
std::optional<NotifySwitchArgs> mLastNotifySwitch GUARDED_BY(mLock);
std::condition_variable mPointerCaptureChangedCondition;
- std::optional<bool> mPointerCaptureEnabled GUARDED_BY(mLock);
+
+ std::optional<PointerCaptureRequest> mPointerCaptureRequest GUARDED_BY(mLock);
// ANR handling
std::queue<std::shared_ptr<InputApplicationHandle>> mAnrApplications GUARDED_BY(mLock);
@@ -401,9 +405,9 @@
mOnPointerDownToken = newToken;
}
- void setPointerCapture(bool enabled) override {
+ void setPointerCapture(const PointerCaptureRequest& request) override {
std::scoped_lock lock(mLock);
- mPointerCaptureEnabled = {enabled};
+ mPointerCaptureRequest = {request};
mPointerCaptureChangedCondition.notify_all();
}
@@ -1380,8 +1384,9 @@
return generateMotionArgs(action, source, displayId, {PointF{100, 200}});
}
-static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs(bool enabled) {
- return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), enabled);
+static NotifyPointerCaptureChangedArgs generatePointerCaptureChangedArgs(
+ const PointerCaptureRequest& request) {
+ return NotifyPointerCaptureChangedArgs(/* id */ 0, systemTime(SYSTEM_TIME_MONOTONIC), request);
}
TEST_F(InputDispatcherTest, SetInputWindow_SingleWindowTouch) {
@@ -4581,16 +4586,18 @@
mWindow->consumeFocusEvent(true);
}
- void notifyPointerCaptureChanged(bool enabled) {
- const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(enabled);
+ void notifyPointerCaptureChanged(const PointerCaptureRequest& request) {
+ const NotifyPointerCaptureChangedArgs args = generatePointerCaptureChangedArgs(request);
mDispatcher->notifyPointerCaptureChanged(&args);
}
- void requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window, bool enabled) {
+ PointerCaptureRequest requestAndVerifyPointerCapture(const sp<FakeWindowHandle>& window,
+ bool enabled) {
mDispatcher->requestPointerCapture(window->getToken(), enabled);
- mFakePolicy->waitForSetPointerCapture(enabled);
- notifyPointerCaptureChanged(enabled);
+ auto request = mFakePolicy->assertSetPointerCaptureCalled(enabled);
+ notifyPointerCaptureChanged(request);
window->consumeCaptureEvent(enabled);
+ return request;
}
};
@@ -4612,7 +4619,7 @@
}
TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLosesFocus) {
- requestAndVerifyPointerCapture(mWindow, true);
+ auto request = requestAndVerifyPointerCapture(mWindow, true);
setFocusedWindow(mSecondWindow);
@@ -4620,26 +4627,26 @@
mWindow->consumeCaptureEvent(false);
mWindow->consumeFocusEvent(false);
mSecondWindow->consumeFocusEvent(true);
- mFakePolicy->waitForSetPointerCapture(false);
+ mFakePolicy->assertSetPointerCaptureCalled(false);
// Ensure that additional state changes from InputReader are not sent to the window.
- notifyPointerCaptureChanged(false);
- notifyPointerCaptureChanged(true);
- notifyPointerCaptureChanged(false);
+ notifyPointerCaptureChanged({});
+ notifyPointerCaptureChanged(request);
+ notifyPointerCaptureChanged({});
mWindow->assertNoEvents();
mSecondWindow->assertNoEvents();
mFakePolicy->assertSetPointerCaptureNotCalled();
}
TEST_F(InputDispatcherPointerCaptureTests, UnexpectedStateChangeDisablesPointerCapture) {
- requestAndVerifyPointerCapture(mWindow, true);
+ auto request = requestAndVerifyPointerCapture(mWindow, true);
// InputReader unexpectedly disables and enables pointer capture.
- notifyPointerCaptureChanged(false);
- notifyPointerCaptureChanged(true);
+ notifyPointerCaptureChanged({});
+ notifyPointerCaptureChanged(request);
// Ensure that Pointer Capture is disabled.
- mFakePolicy->waitForSetPointerCapture(false);
+ mFakePolicy->assertSetPointerCaptureCalled(false);
mWindow->consumeCaptureEvent(false);
mWindow->assertNoEvents();
}
@@ -4649,24 +4656,43 @@
// The first window loses focus.
setFocusedWindow(mSecondWindow);
- mFakePolicy->waitForSetPointerCapture(false);
+ mFakePolicy->assertSetPointerCaptureCalled(false);
mWindow->consumeCaptureEvent(false);
// Request Pointer Capture from the second window before the notification from InputReader
// arrives.
mDispatcher->requestPointerCapture(mSecondWindow->getToken(), true);
- mFakePolicy->waitForSetPointerCapture(true);
+ auto request = mFakePolicy->assertSetPointerCaptureCalled(true);
// InputReader notifies Pointer Capture was disabled (because of the focus change).
- notifyPointerCaptureChanged(false);
+ notifyPointerCaptureChanged({});
// InputReader notifies Pointer Capture was enabled (because of mSecondWindow's request).
- notifyPointerCaptureChanged(true);
+ notifyPointerCaptureChanged(request);
mSecondWindow->consumeFocusEvent(true);
mSecondWindow->consumeCaptureEvent(true);
}
+TEST_F(InputDispatcherPointerCaptureTests, EnableRequestFollowsSequenceNumbers) {
+ // App repeatedly enables and disables capture.
+ mDispatcher->requestPointerCapture(mWindow->getToken(), true);
+ auto firstRequest = mFakePolicy->assertSetPointerCaptureCalled(true);
+ mDispatcher->requestPointerCapture(mWindow->getToken(), false);
+ mFakePolicy->assertSetPointerCaptureCalled(false);
+ mDispatcher->requestPointerCapture(mWindow->getToken(), true);
+ auto secondRequest = mFakePolicy->assertSetPointerCaptureCalled(true);
+
+ // InputReader notifies that PointerCapture has been enabled for the first request. Since the
+ // first request is now stale, this should do nothing.
+ notifyPointerCaptureChanged(firstRequest);
+ mWindow->assertNoEvents();
+
+ // InputReader notifies that the second request was enabled.
+ notifyPointerCaptureChanged(secondRequest);
+ mWindow->consumeCaptureEvent(true);
+}
+
class InputDispatcherUntrustedTouchesTest : public InputDispatcherTest {
protected:
constexpr static const float MAXIMUM_OBSCURING_OPACITY = 0.8;
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 91da0d5..b419d9a 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -312,8 +312,9 @@
transform = t;
}
- void setPointerCapture(bool enabled) {
- mConfig.pointerCapture = enabled;
+ PointerCaptureRequest setPointerCapture(bool enabled) {
+ mConfig.pointerCaptureRequest = {enabled, mNextPointerCaptureSequenceNumber++};
+ return mConfig.pointerCaptureRequest;
}
void setShowTouches(bool enabled) {
@@ -327,6 +328,8 @@
float getPointerGestureMovementSpeedRatio() { return mConfig.pointerGestureMovementSpeedRatio; }
private:
+ uint32_t mNextPointerCaptureSequenceNumber = 0;
+
DisplayViewport createDisplayViewport(int32_t displayId, int32_t width, int32_t height,
int32_t orientation, bool isActive,
const std::string& uniqueId,
@@ -1974,24 +1977,24 @@
TEST_F(InputReaderTest, ChangingPointerCaptureNotifiesInputListener) {
NotifyPointerCaptureChangedArgs args;
- mFakePolicy->setPointerCapture(true);
+ auto request = mFakePolicy->setPointerCapture(true);
mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE);
mReader->loopOnce();
mFakeListener->assertNotifyCaptureWasCalled(&args);
- ASSERT_TRUE(args.enabled) << "Pointer Capture should be enabled.";
+ ASSERT_TRUE(args.request.enable) << "Pointer Capture should be enabled.";
+ ASSERT_EQ(args.request, request) << "Pointer Capture sequence number should match.";
mFakePolicy->setPointerCapture(false);
mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE);
mReader->loopOnce();
mFakeListener->assertNotifyCaptureWasCalled(&args);
- ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled.";
+ ASSERT_FALSE(args.request.enable) << "Pointer Capture should be disabled.";
- // Verify that the Pointer Capture state is re-configured correctly when the configuration value
+ // Verify that the Pointer Capture state is not updated when the configuration value
// does not change.
mReader->requestRefreshConfiguration(InputReaderConfiguration::CHANGE_POINTER_CAPTURE);
mReader->loopOnce();
- mFakeListener->assertNotifyCaptureWasCalled(&args);
- ASSERT_FALSE(args.enabled) << "Pointer Capture should be disabled.";
+ mFakeListener->assertNotifyCaptureWasNotCalled();
}
class FakeVibratorInputMapper : public FakeInputMapper {
diff --git a/services/inputflinger/tests/TestInputListener.cpp b/services/inputflinger/tests/TestInputListener.cpp
index fb7de97..6a26c63 100644
--- a/services/inputflinger/tests/TestInputListener.cpp
+++ b/services/inputflinger/tests/TestInputListener.cpp
@@ -100,6 +100,11 @@
"to have been called."));
}
+void TestInputListener::assertNotifyCaptureWasNotCalled() {
+ ASSERT_NO_FATAL_FAILURE(assertNotCalled<NotifyPointerCaptureChangedArgs>(
+ "notifyPointerCaptureChanged() should not be called."));
+}
+
template <class NotifyArgsType>
void TestInputListener::assertCalled(NotifyArgsType* outEventArgs, std::string message) {
std::unique_lock<std::mutex> lock(mLock);
diff --git a/services/inputflinger/tests/TestInputListener.h b/services/inputflinger/tests/TestInputListener.h
index 0ffcaaa..0a1dc4b 100644
--- a/services/inputflinger/tests/TestInputListener.h
+++ b/services/inputflinger/tests/TestInputListener.h
@@ -55,6 +55,7 @@
void assertNotifySwitchWasCalled(NotifySwitchArgs* outEventArgs = nullptr);
void assertNotifyCaptureWasCalled(NotifyPointerCaptureChangedArgs* outEventArgs = nullptr);
+ void assertNotifyCaptureWasNotCalled();
void assertNotifySensorWasCalled(NotifySensorArgs* outEventArgs = nullptr);
void assertNotifyVibratorStateWasCalled(NotifyVibratorStateArgs* outEventArgs = nullptr);