From f192a1085a4ea16458a8cc569e945bf090375733 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Fri, 6 Aug 2021 14:01:18 +0000 Subject: 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 --- include/input/Input.h | 19 +++++ services/inputflinger/InputListener.cpp | 10 +-- services/inputflinger/InputReaderBase.cpp | 3 + .../benchmarks/InputDispatcher_benchmarks.cpp | 2 +- services/inputflinger/dispatcher/Entry.cpp | 9 +-- services/inputflinger/dispatcher/Entry.h | 6 +- .../inputflinger/dispatcher/InputDispatcher.cpp | 83 +++++++++++--------- services/inputflinger/dispatcher/InputDispatcher.h | 12 +-- .../include/InputDispatcherPolicyInterface.h | 2 +- services/inputflinger/docs/pointer_capture.md | 4 +- services/inputflinger/include/InputListener.h | 5 +- services/inputflinger/include/InputReaderBase.h | 25 +++--- services/inputflinger/reader/InputReader.cpp | 12 ++- services/inputflinger/reader/include/InputReader.h | 2 + .../reader/mapper/CursorInputMapper.cpp | 4 +- .../reader/mapper/TouchInputMapper.cpp | 7 +- .../inputflinger/tests/InputDispatcher_test.cpp | 88 ++++++++++++++-------- services/inputflinger/tests/InputReader_test.cpp | 19 +++-- services/inputflinger/tests/TestInputListener.cpp | 5 ++ services/inputflinger/tests/TestInputListener.h | 1 + 20 files changed, 201 insertions(+), 117 deletions(-) diff --git a/include/input/Input.h b/include/input/Input.h index 2e326cb102..dce6ccb3d5 100644 --- a/include/input/Input.h +++ b/include/input/Input.h @@ -1015,6 +1015,25 @@ private: std::queue> mDragEventPool; }; +/* + * Describes a unique request to enable or disable Pointer Capture. + */ +struct PointerCaptureRequest { +public: + inline PointerCaptureRequest() : enable(false), seq(0) {} + inline PointerCaptureRequest(bool enable, uint32_t seq) : enable(enable), seq(seq) {} + inline bool operator==(const PointerCaptureRequest& other) const { + return enable == other.enable && seq == other.seq; + } + explicit inline operator bool() const { return enable; } + + // True iff this is a request to enable Pointer Capture. + bool enable; + + // The sequence number for the request. + uint32_t seq; +}; + } // namespace android #endif // _LIBINPUT_INPUT_H diff --git a/services/inputflinger/InputListener.cpp b/services/inputflinger/InputListener.cpp index 33b3e1ee2f..71b0f5fec9 100644 --- a/services/inputflinger/InputListener.cpp +++ b/services/inputflinger/InputListener.cpp @@ -287,16 +287,16 @@ void NotifyDeviceResetArgs::notify(const sp& listener) c // --- NotifyPointerCaptureChangedArgs --- -NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, - bool enabled) - : NotifyArgs(id, eventTime), enabled(enabled) {} +NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( + int32_t id, nsecs_t eventTime, const PointerCaptureRequest& request) + : NotifyArgs(id, eventTime), request(request) {} NotifyPointerCaptureChangedArgs::NotifyPointerCaptureChangedArgs( const NotifyPointerCaptureChangedArgs& other) - : NotifyArgs(other.id, other.eventTime), enabled(other.enabled) {} + : NotifyArgs(other.id, other.eventTime), request(other.request) {} bool NotifyPointerCaptureChangedArgs::operator==(const NotifyPointerCaptureChangedArgs& rhs) const { - return id == rhs.id && eventTime == rhs.eventTime && enabled == rhs.enabled; + return id == rhs.id && eventTime == rhs.eventTime && request == rhs.request; } void NotifyPointerCaptureChangedArgs::notify(const sp& listener) const { diff --git a/services/inputflinger/InputReaderBase.cpp b/services/inputflinger/InputReaderBase.cpp index 9cc777d450..d34482f506 100644 --- a/services/inputflinger/InputReaderBase.cpp +++ b/services/inputflinger/InputReaderBase.cpp @@ -67,6 +67,9 @@ std::string InputReaderConfiguration::changesToString(uint32_t changes) { if (changes & CHANGE_EXTERNAL_STYLUS_PRESENCE) { result += "EXTERNAL_STYLUS_PRESENCE | "; } + if (changes & CHANGE_POINTER_CAPTURE) { + result += "POINTER_CAPTURE | "; + } if (changes & CHANGE_ENABLED_STATE) { result += "ENABLED_STATE | "; } diff --git a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp index bc77b8aef4..aa8cc302c7 100644 --- a/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp +++ b/services/inputflinger/benchmarks/InputDispatcher_benchmarks.cpp @@ -113,7 +113,7 @@ private: void onPointerDownOutsideFocus(const sp& newToken) override {} - void setPointerCapture(bool enabled) override {} + void setPointerCapture(const PointerCaptureRequest&) override {} void notifyDropWindow(const sp&, float x, float y) override {} diff --git a/services/inputflinger/dispatcher/Entry.cpp b/services/inputflinger/dispatcher/Entry.cpp index 881024fc6e..5c3747e2ec 100644 --- a/services/inputflinger/dispatcher/Entry.cpp +++ b/services/inputflinger/dispatcher/Entry.cpp @@ -119,15 +119,15 @@ std::string FocusEntry::getDescription() const { // PointerCaptureChanged notifications always go to apps, so set the flag POLICY_FLAG_PASS_TO_USER // for all entries. PointerCaptureChangedEntry::PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, - bool hasPointerCapture) + const PointerCaptureRequest& request) : EventEntry(id, Type::POINTER_CAPTURE_CHANGED, eventTime, POLICY_FLAG_PASS_TO_USER), - pointerCaptureEnabled(hasPointerCapture) {} + pointerCaptureRequest(request) {} PointerCaptureChangedEntry::~PointerCaptureChangedEntry() {} std::string PointerCaptureChangedEntry::getDescription() const { return StringPrintf("PointerCaptureChangedEvent(pointerCaptureEnabled=%s)", - pointerCaptureEnabled ? "true" : "false"); + pointerCaptureRequest.enable ? "true" : "false"); } // --- DragEntry --- @@ -324,8 +324,7 @@ CommandEntry::CommandEntry(Command command) keyEntry(nullptr), userActivityEventType(0), seq(0), - handled(false), - enabled(false) {} + handled(false) {} CommandEntry::~CommandEntry() {} diff --git a/services/inputflinger/dispatcher/Entry.h b/services/inputflinger/dispatcher/Entry.h index ebbd8e93bf..6f1dfadf59 100644 --- a/services/inputflinger/dispatcher/Entry.h +++ b/services/inputflinger/dispatcher/Entry.h @@ -104,9 +104,9 @@ struct FocusEntry : EventEntry { }; struct PointerCaptureChangedEntry : EventEntry { - bool pointerCaptureEnabled; + const PointerCaptureRequest pointerCaptureRequest; - PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, bool hasPointerCapture); + PointerCaptureChangedEntry(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); std::string getDescription() const override; ~PointerCaptureChangedEntry() override; @@ -284,7 +284,7 @@ struct CommandEntry { sp oldToken; sp newToken; std::string obscuringPackage; - bool enabled; + PointerCaptureRequest pointerCaptureRequest; int32_t pid; nsecs_t consumeTime; // time when the event was consumed by InputConsumer int32_t displayId; diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 1899c5f29d..ce1f266cfb 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -523,7 +523,6 @@ InputDispatcher::InputDispatcher(const sp& polic mInTouchMode(true), mMaximumObscuringOpacityForTouch(1.0f), mFocusedDisplayId(ADISPLAY_ID_DEFAULT), - mFocusedWindowRequestedPointerCapture(false), mWindowTokenWithPointerCapture(nullptr), mLatencyAggregator(), mLatencyTracker(&mLatencyAggregator), @@ -1306,36 +1305,51 @@ void InputDispatcher::dispatchFocusLocked(nsecs_t currentTime, std::shared_ptr& entry, DropReason& dropReason) { - const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; - if (entry->pointerCaptureEnabled && haveWindowWithPointerCapture) { - LOG_ALWAYS_FATAL("Pointer Capture has already been enabled for the window."); - } - if (!entry->pointerCaptureEnabled && !haveWindowWithPointerCapture) { - // Pointer capture was already forcefully disabled because of focus change. - dropReason = DropReason::NOT_DROPPED; - return; - } - - // Set drop reason for early returns - dropReason = DropReason::NO_POINTER_CAPTURE; + dropReason = DropReason::NOT_DROPPED; + const bool haveWindowWithPointerCapture = mWindowTokenWithPointerCapture != nullptr; sp token; - if (entry->pointerCaptureEnabled) { - // Enable Pointer Capture - if (!mFocusedWindowRequestedPointerCapture) { + + if (entry->pointerCaptureRequest.enable) { + // Enable Pointer Capture. + if (haveWindowWithPointerCapture && + (entry->pointerCaptureRequest == mCurrentPointerCaptureRequest)) { + LOG_ALWAYS_FATAL("This request to enable Pointer Capture has already been dispatched " + "to the window."); + } + if (!mCurrentPointerCaptureRequest.enable) { // This can happen if a window requests capture and immediately releases capture. ALOGW("No window requested Pointer Capture."); + dropReason = DropReason::NO_POINTER_CAPTURE; + return; + } + if (entry->pointerCaptureRequest.seq != mCurrentPointerCaptureRequest.seq) { + ALOGI("Skipping dispatch of Pointer Capture being enabled: sequence number mismatch."); return; } + token = mFocusResolver.getFocusedWindowToken(mFocusedDisplayId); LOG_ALWAYS_FATAL_IF(!token, "Cannot find focused window for Pointer Capture."); mWindowTokenWithPointerCapture = token; } else { - // Disable Pointer Capture + // Disable Pointer Capture. + // We do not check if the sequence number matches for requests to disable Pointer Capture + // for two reasons: + // 1. Pointer Capture can be disabled by a focus change, which means we can get two entries + // to disable capture with the same sequence number: one generated by + // disablePointerCaptureForcedLocked() and another as an acknowledgement of Pointer + // Capture being disabled in InputReader. + // 2. We respect any request to disable Pointer Capture generated by InputReader, since the + // actual Pointer Capture state that affects events being generated by input devices is + // in InputReader. + if (!haveWindowWithPointerCapture) { + // Pointer capture was already forcefully disabled because of focus change. + dropReason = DropReason::NOT_DROPPED; + return; + } token = mWindowTokenWithPointerCapture; mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } } @@ -1344,8 +1358,7 @@ void InputDispatcher::dispatchPointerCaptureChangedLocked( if (channel == nullptr) { // Window has gone away, clean up Pointer Capture state. mWindowTokenWithPointerCapture = nullptr; - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } return; @@ -3180,7 +3193,7 @@ void InputDispatcher::startDispatchCycleLocked(nsecs_t currentTime, static_cast(eventEntry); status = connection->inputPublisher .publishCaptureEvent(dispatchEntry->seq, captureEntry.id, - captureEntry.pointerCaptureEnabled); + captureEntry.pointerCaptureRequest.enable); break; } @@ -3978,14 +3991,14 @@ void InputDispatcher::notifyDeviceReset(const NotifyDeviceResetArgs* args) { void InputDispatcher::notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) { #if DEBUG_INBOUND_EVENT_DETAILS ALOGD("notifyPointerCaptureChanged - eventTime=%" PRId64 ", enabled=%s", args->eventTime, - args->enabled ? "true" : "false"); + args->request.enable ? "true" : "false"); #endif bool needWake; { // acquire lock std::scoped_lock _l(mLock); auto entry = std::make_unique(args->id, args->eventTime, - args->enabled); + args->request); needWake = enqueueInboundEventLocked(std::move(entry)); } // release lock @@ -4929,8 +4942,8 @@ void InputDispatcher::logDispatchStateLocked() { std::string InputDispatcher::dumpPointerCaptureStateLocked() { std::string dump; - dump += StringPrintf(INDENT "FocusedWindowRequestedPointerCapture: %s\n", - toString(mFocusedWindowRequestedPointerCapture)); + dump += StringPrintf(INDENT "Pointer Capture Requested: %s\n", + toString(mCurrentPointerCaptureRequest.enable)); std::string windowName = "None"; if (mWindowTokenWithPointerCapture) { @@ -4939,7 +4952,7 @@ std::string InputDispatcher::dumpPointerCaptureStateLocked() { windowName = captureWindowHandle ? captureWindowHandle->getName().c_str() : "token has capture without window"; } - dump += StringPrintf(INDENT "CurrentWindowWithPointerCapture: %s\n", windowName.c_str()); + dump += StringPrintf(INDENT "Current Window with Pointer Capture: %s\n", windowName.c_str()); return dump; } @@ -5407,14 +5420,13 @@ void InputDispatcher::requestPointerCapture(const sp& windowToken, bool return; } - if (enabled == mFocusedWindowRequestedPointerCapture) { + if (enabled == mCurrentPointerCaptureRequest.enable) { ALOGW("Ignoring request to %s Pointer Capture: " "window has %s requested pointer capture.", enabled ? "enable" : "disable", enabled ? "already" : "not"); return; } - mFocusedWindowRequestedPointerCapture = enabled; setPointerCaptureLocked(enabled); } // release lock @@ -6173,14 +6185,13 @@ void InputDispatcher::onFocusChangedLocked(const FocusResolver::FocusChanges& ch } void InputDispatcher::disablePointerCaptureForcedLocked() { - if (!mFocusedWindowRequestedPointerCapture && !mWindowTokenWithPointerCapture) { + if (!mCurrentPointerCaptureRequest.enable && !mWindowTokenWithPointerCapture) { return; } ALOGD_IF(DEBUG_FOCUS, "Disabling Pointer Capture because the window lost focus."); - if (mFocusedWindowRequestedPointerCapture) { - mFocusedWindowRequestedPointerCapture = false; + if (mCurrentPointerCaptureRequest.enable) { setPointerCaptureLocked(false); } @@ -6197,14 +6208,16 @@ void InputDispatcher::disablePointerCaptureForcedLocked() { } auto entry = std::make_unique(mIdGenerator.nextId(), now(), - false /* hasCapture */); + mCurrentPointerCaptureRequest); mInboundQueue.push_front(std::move(entry)); } void InputDispatcher::setPointerCaptureLocked(bool enabled) { + mCurrentPointerCaptureRequest.enable = enabled; + mCurrentPointerCaptureRequest.seq++; std::unique_ptr commandEntry = std::make_unique( &InputDispatcher::doSetPointerCaptureLockedInterruptible); - commandEntry->enabled = enabled; + commandEntry->pointerCaptureRequest = mCurrentPointerCaptureRequest; postCommandLocked(std::move(commandEntry)); } @@ -6212,7 +6225,7 @@ void InputDispatcher::doSetPointerCaptureLockedInterruptible( android::inputdispatcher::CommandEntry* commandEntry) { mLock.unlock(); - mPolicy->setPointerCapture(commandEntry->enabled); + mPolicy->setPointerCapture(commandEntry->pointerCaptureRequest); mLock.lock(); } diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h index 9edf41c9c0..30652c65ee 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.h +++ b/services/inputflinger/dispatcher/InputDispatcher.h @@ -357,10 +357,12 @@ private: // Keeps track of the focused window per display and determines focus changes. FocusResolver mFocusResolver GUARDED_BY(mLock); - // Whether the focused window on the focused display has requested Pointer Capture. - // The state of this variable should always be in sync with the state of Pointer Capture in the - // policy, which is updated through setPointerCaptureLocked(enabled). - bool mFocusedWindowRequestedPointerCapture GUARDED_BY(mLock); + + // The enabled state of this request is true iff the focused window on the focused display has + // requested Pointer Capture. This request also contains the sequence number associated with the + // current request. The state of this variable should always be in sync with the state of + // Pointer Capture in the policy, and is only updated through setPointerCaptureLocked(request). + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); // The window token that has Pointer Capture. // This should be in sync with PointerCaptureChangedEvents dispatched to the input channel. @@ -370,7 +372,7 @@ private: void disablePointerCaptureForcedLocked() REQUIRES(mLock); // Set the Pointer Capture state in the Policy. - void setPointerCaptureLocked(bool enabled) REQUIRES(mLock); + void setPointerCaptureLocked(bool enable) REQUIRES(mLock); // Dispatcher state at time of last ANR. std::string mLastAnrState GUARDED_BY(mLock); diff --git a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h index 219f45a7c3..fd591e0e1c 100644 --- a/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h +++ b/services/inputflinger/dispatcher/include/InputDispatcherPolicyInterface.h @@ -156,7 +156,7 @@ public: * * InputDispatcher is solely responsible for updating the Pointer Capture state. */ - virtual void setPointerCapture(bool enabled) = 0; + virtual void setPointerCapture(const PointerCaptureRequest&) = 0; /* Notifies the policy that the drag window has moved over to another window */ virtual void notifyDropWindow(const sp& token, float x, float y) = 0; diff --git a/services/inputflinger/docs/pointer_capture.md b/services/inputflinger/docs/pointer_capture.md index 8da699dc56..0b44187c08 100644 --- a/services/inputflinger/docs/pointer_capture.md +++ b/services/inputflinger/docs/pointer_capture.md @@ -17,6 +17,8 @@ `InputDispatcher` is responsible for controlling the state of Pointer Capture. Since the feature requires changes to how events are generated, Pointer Capture is configured in `InputReader`. +We use a sequence number to synchronize different requests to enable Pointer Capture between InputReader and InputDispatcher. + ### Enabling Pointer Capture There are four key steps that take place when Pointer Capture is enabled: @@ -40,5 +42,5 @@ When an `InputWindow` with Pointer Capture loses focus, Pointer Capture is disab `InputDispatcher` tracks two pieces of state information regarding Pointer Capture: -- `mFocusedWindowRequestedPointerCapture`: Whether or not the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. +- `mCurrentPointerCaptureRequest`: The sequence number of the current Pointer Capture request. This request is enabled iff the focused window has requested Pointer Capture. This is updated whenever the Dispatcher receives requests from `InputManagerService`. - `mWindowTokenWithPointerCapture`: The Binder token of the `InputWindow` that currently has Pointer Capture. This is only updated during the dispatch cycle. If it is not `nullptr`, it signifies that the window was notified that it has Pointer Capture. diff --git a/services/inputflinger/include/InputListener.h b/services/inputflinger/include/InputListener.h index 4b7d26df2b..fe74214b36 100644 --- a/services/inputflinger/include/InputListener.h +++ b/services/inputflinger/include/InputListener.h @@ -211,11 +211,12 @@ struct NotifyDeviceResetArgs : public NotifyArgs { /* Describes a change in the state of Pointer Capture. */ struct NotifyPointerCaptureChangedArgs : public NotifyArgs { - bool enabled; + // The sequence number of the Pointer Capture request, if enabled. + PointerCaptureRequest request; inline NotifyPointerCaptureChangedArgs() {} - NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, bool enabled); + NotifyPointerCaptureChangedArgs(int32_t id, nsecs_t eventTime, const PointerCaptureRequest&); NotifyPointerCaptureChangedArgs(const NotifyPointerCaptureChangedArgs& other); diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index 7fdbbfdac4..3c8ac1cf7b 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -279,29 +279,30 @@ struct InputReaderConfiguration { // True to show the location of touches on the touch screen as spots. bool showTouches; - // True if pointer capture is enabled. - bool pointerCapture; + // The latest request to enable or disable Pointer Capture. + PointerCaptureRequest pointerCaptureRequest; // The set of currently disabled input devices. std::set disabledDevices; - InputReaderConfiguration() : - virtualKeyQuietTime(0), + InputReaderConfiguration() + : virtualKeyQuietTime(0), pointerVelocityControlParameters(1.0f, 500.0f, 3000.0f, 3.0f), wheelVelocityControlParameters(1.0f, 15.0f, 50.0f, 4.0f), pointerGesturesEnabled(true), - pointerGestureQuietInterval(100 * 1000000LL), // 100 ms - pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second - pointerGestureTapInterval(150 * 1000000LL), // 150 ms - pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms - pointerGestureTapSlop(10.0f), // 10 pixels + pointerGestureQuietInterval(100 * 1000000LL), // 100 ms + pointerGestureDragMinSwitchSpeed(50), // 50 pixels per second + pointerGestureTapInterval(150 * 1000000LL), // 150 ms + pointerGestureTapDragInterval(150 * 1000000LL), // 150 ms + pointerGestureTapSlop(10.0f), // 10 pixels pointerGestureMultitouchSettleInterval(100 * 1000000LL), // 100 ms - pointerGestureMultitouchMinDistance(15), // 15 pixels - pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees + pointerGestureMultitouchMinDistance(15), // 15 pixels + pointerGestureSwipeTransitionAngleCosine(0.2588f), // cosine of 75 degrees pointerGestureSwipeMaxWidthRatio(0.25f), pointerGestureMovementSpeedRatio(0.8f), pointerGestureZoomSpeedRatio(0.3f), - showTouches(false), pointerCapture(false) { } + showTouches(false), + pointerCaptureRequest() {} static std::string changesToString(uint32_t changes); diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 10c04f606c..5120860503 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -367,9 +367,15 @@ void InputReader::refreshConfigurationLocked(uint32_t changes) { } if (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE) { - const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, - mConfig.pointerCapture); - mQueuedListener->notifyPointerCaptureChanged(&args); + if (mCurrentPointerCaptureRequest == mConfig.pointerCaptureRequest) { + ALOGV("Skipping notifying pointer capture changes: " + "There was no change in the pointer capture state."); + } else { + mCurrentPointerCaptureRequest = mConfig.pointerCaptureRequest; + const NotifyPointerCaptureChangedArgs args(mContext.getNextId(), now, + mCurrentPointerCaptureRequest); + mQueuedListener->notifyPointerCaptureChanged(&args); + } } } diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index a00c5af4dc..e44aa0fe27 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -230,6 +230,8 @@ private: uint32_t mConfigurationChangesToRefresh GUARDED_BY(mLock); void refreshConfigurationLocked(uint32_t changes) REQUIRES(mLock); + PointerCaptureRequest mCurrentPointerCaptureRequest GUARDED_BY(mLock); + // state queries typedef int32_t (InputDevice::*GetStateFunc)(uint32_t sourceMask, int32_t code); int32_t getStateLocked(int32_t deviceId, uint32_t sourceMask, int32_t code, diff --git a/services/inputflinger/reader/mapper/CursorInputMapper.cpp b/services/inputflinger/reader/mapper/CursorInputMapper.cpp index 437902a79b..2ac41b1e67 100644 --- a/services/inputflinger/reader/mapper/CursorInputMapper.cpp +++ b/services/inputflinger/reader/mapper/CursorInputMapper.cpp @@ -154,9 +154,9 @@ void CursorInputMapper::configure(nsecs_t when, const InputReaderConfiguration* mHWheelScale = 1.0f; } - if ((!changes && config->pointerCapture) || + if ((!changes && config->pointerCaptureRequest.enable) || (changes & InputReaderConfiguration::CHANGE_POINTER_CAPTURE)) { - if (config->pointerCapture) { + if (config->pointerCaptureRequest.enable) { if (mParameters.mode == Parameters::MODE_POINTER) { mParameters.mode = Parameters::MODE_POINTER_RELATIVE; mSource = AINPUT_SOURCE_MOUSE_RELATIVE; diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 962d8d2935..8733540e53 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -603,7 +603,7 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // Determine device mode. if (mParameters.deviceType == Parameters::DeviceType::POINTER && - mConfig.pointerGesturesEnabled && !mConfig.pointerCapture) { + mConfig.pointerGesturesEnabled && !mConfig.pointerCaptureRequest.enable) { mSource = AINPUT_SOURCE_MOUSE; mDeviceMode = DeviceMode::POINTER; if (hasStylus()) { @@ -776,11 +776,12 @@ void TouchInputMapper::configureSurface(nsecs_t when, bool* outResetNeeded) { // preserve the cursor position. if (mDeviceMode == DeviceMode::POINTER || (mDeviceMode == DeviceMode::DIRECT && mConfig.showTouches) || - (mParameters.deviceType == Parameters::DeviceType::POINTER && mConfig.pointerCapture)) { + (mParameters.deviceType == Parameters::DeviceType::POINTER && + mConfig.pointerCaptureRequest.enable)) { if (mPointerController == nullptr) { mPointerController = getContext()->getPointerController(getDeviceId()); } - if (mConfig.pointerCapture) { + if (mConfig.pointerCaptureRequest.enable) { mPointerController->fade(PointerControllerInterface::Transition::IMMEDIATE); } } else { diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 3a9dede89b..50b65cad5d 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -239,19 +239,22 @@ public: 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() { @@ -259,11 +262,11 @@ public: 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& targetToken) { @@ -281,7 +284,8 @@ private: std::optional mLastNotifySwitch GUARDED_BY(mLock); std::condition_variable mPointerCaptureChangedCondition; - std::optional mPointerCaptureEnabled GUARDED_BY(mLock); + + std::optional mPointerCaptureRequest GUARDED_BY(mLock); // ANR handling std::queue> mAnrApplications GUARDED_BY(mLock); @@ -398,9 +402,9 @@ private: 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(); } @@ -1379,8 +1383,9 @@ static NotifyMotionArgs generateMotionArgs(int32_t action, int32_t source, int32 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) { @@ -4591,16 +4596,18 @@ protected: 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& window, bool enabled) { + PointerCaptureRequest requestAndVerifyPointerCapture(const sp& 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; } }; @@ -4622,7 +4629,7 @@ TEST_F(InputDispatcherPointerCaptureTests, EnablePointerCaptureWhenFocused) { } TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLosesFocus) { - requestAndVerifyPointerCapture(mWindow, true); + auto request = requestAndVerifyPointerCapture(mWindow, true); setFocusedWindow(mSecondWindow); @@ -4630,26 +4637,26 @@ TEST_F(InputDispatcherPointerCaptureTests, DisablesPointerCaptureAfterWindowLose 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(); } @@ -4659,24 +4666,43 @@ TEST_F(InputDispatcherPointerCaptureTests, OutOfOrderRequests) { // 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 997cbe88a1..38dfe4041f 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -299,8 +299,9 @@ public: transform = t; } - void setPointerCapture(bool enabled) { - mConfig.pointerCapture = enabled; + PointerCaptureRequest setPointerCapture(bool enabled) { + mConfig.pointerCaptureRequest = {enabled, mNextPointerCaptureSequenceNumber++}; + return mConfig.pointerCaptureRequest; } void setShowTouches(bool enabled) { @@ -314,6 +315,8 @@ public: 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, @@ -1961,24 +1964,24 @@ TEST_F(InputReaderTest, GetKeyCodeState_ForwardsRequestsToSubdeviceMappers) { 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 fb7de97b49..6a26c636d9 100644 --- a/services/inputflinger/tests/TestInputListener.cpp +++ b/services/inputflinger/tests/TestInputListener.cpp @@ -100,6 +100,11 @@ void TestInputListener::assertNotifyCaptureWasCalled( "to have been called.")); } +void TestInputListener::assertNotifyCaptureWasNotCalled() { + ASSERT_NO_FATAL_FAILURE(assertNotCalled( + "notifyPointerCaptureChanged() should not be called.")); +} + template void TestInputListener::assertCalled(NotifyArgsType* outEventArgs, std::string message) { std::unique_lock lock(mLock); diff --git a/services/inputflinger/tests/TestInputListener.h b/services/inputflinger/tests/TestInputListener.h index 0ffcaaa527..0a1dc4b0b9 100644 --- a/services/inputflinger/tests/TestInputListener.h +++ b/services/inputflinger/tests/TestInputListener.h @@ -55,6 +55,7 @@ public: void assertNotifySwitchWasCalled(NotifySwitchArgs* outEventArgs = nullptr); void assertNotifyCaptureWasCalled(NotifyPointerCaptureChangedArgs* outEventArgs = nullptr); + void assertNotifyCaptureWasNotCalled(); void assertNotifySensorWasCalled(NotifySensorArgs* outEventArgs = nullptr); void assertNotifyVibratorStateWasCalled(NotifyVibratorStateArgs* outEventArgs = nullptr); -- cgit v1.2.3-59-g8ed1b