From a235c040edb9f4132b4cb30eb09e3dbc7cd85dbb Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Tue, 2 May 2023 09:59:09 -0700 Subject: Treat HOVER_EXIT as continuation of existing gesture Before this CL, the HOVER_EXIT events used to get treated the same way as the HOVER_ENTER or HOVER_MOVE events. However, that's not quite correct. These events can end gestures, but it never makes sense to use these for starting a new gesture. In this CL, we move the handling of HOVER_EXIT to Case 2 instead of Case 1 inside findTouchedWindowTargetsLocked. Now, both hovering and touching pointers will be considered there. This also means that we should modify the requirement of "at least 1 foreground window", because the gesture needs to be ended correctly even if there's no foreground window present. For example, the window might no longer be foreground, but it already started receiving touch. The foreground check is replaced by an explicit check for empty or exclusively ACTION_OUTSIDE targets. Bug: 273376858 Test: atest inputflinger_tests:InputDispatcherTest.HoverWhileWindowAppears Change-Id: Ib7562ae65ee505debaed92e04aea972221af255d --- .../inputflinger/dispatcher/InputDispatcher.cpp | 56 ++++++---- .../inputflinger/tests/InputDispatcher_test.cpp | 115 ++++++++++++++++++++- 2 files changed, 151 insertions(+), 20 deletions(-) diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index e4f367dd5f..a10c957d15 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -2239,7 +2239,9 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( const bool wasDown = oldState != nullptr && oldState->isDown(); const bool isDown = (maskedAction == AMOTION_EVENT_ACTION_DOWN) || (maskedAction == AMOTION_EVENT_ACTION_POINTER_DOWN && !wasDown); - const bool newGesture = isDown || maskedAction == AMOTION_EVENT_ACTION_SCROLL || isHoverAction; + const bool newGesture = isDown || maskedAction == AMOTION_EVENT_ACTION_SCROLL || + maskedAction == AMOTION_EVENT_ACTION_HOVER_ENTER || + maskedAction == AMOTION_EVENT_ACTION_HOVER_MOVE; const bool isFromMouse = isFromSource(entry.source, AINPUT_SOURCE_MOUSE); // If pointers are already down, let's finish the current gesture and ignore the new events @@ -2427,7 +2429,7 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( /* Case 2: Pointer move, up, cancel or non-splittable pointer down. */ // If the pointer is not currently down, then ignore the event. - if (!tempTouchState.isDown()) { + if (!tempTouchState.isDown() && maskedAction != AMOTION_EVENT_ACTION_HOVER_EXIT) { LOG(INFO) << "Dropping event because the pointer is not down or we previously " "dropped the pointer down event in display " << displayId << ": " << entry.getDescription(); @@ -2435,6 +2437,20 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( return {}; } + // If the pointer is not currently hovering, then ignore the event. + if (maskedAction == AMOTION_EVENT_ACTION_HOVER_EXIT) { + const int32_t pointerId = entry.pointerProperties[0].id; + if (oldState == nullptr || + oldState->getWindowsWithHoveringPointer(entry.deviceId, pointerId).empty()) { + LOG(INFO) << "Dropping event because the hovering pointer is not in any windows in " + "display " + << displayId << ": " << entry.getDescription(); + outInjectionResult = InputEventInjectionResult::FAILED; + return {}; + } + tempTouchState.removeHoveringPointer(entry.deviceId, pointerId); + } + addDragEventLocked(entry); // Check whether touches should slip outside of the current foreground window. @@ -2530,21 +2546,6 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( targets); } } - // Ensure that we have at least one foreground window or at least one window that cannot be a - // foreground target. If we only have windows that are not receiving foreground touches (e.g. we - // only have windows getting ACTION_OUTSIDE), then drop the event, because there is no window - // that is actually receiving the entire gesture. - if (std::none_of(tempTouchState.windows.begin(), tempTouchState.windows.end(), - [](const TouchedWindow& touchedWindow) { - return !canReceiveForegroundTouches( - *touchedWindow.windowHandle->getInfo()) || - touchedWindow.targetFlags.test(InputTarget::Flags::FOREGROUND); - })) { - ALOGI("Dropping event because there is no touched window on display %d to receive it: %s", - displayId, entry.getDescription().c_str()); - outInjectionResult = InputEventInjectionResult::FAILED; - return {}; - } // Ensure that all touched windows are valid for injection. if (entry.injectionState != nullptr) { @@ -2586,7 +2587,7 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( } } - // Success! Output targets from the touch state. + // Output targets from the touch state. for (const TouchedWindow& touchedWindow : tempTouchState.windows) { if (touchedWindow.pointerIds.none() && !touchedWindow.hasHoveringPointers(entry.deviceId)) { // Windows with hovering pointers are getting persisted inside TouchState. @@ -2598,6 +2599,23 @@ std::vector InputDispatcher::findTouchedWindowTargetsLocked( targets); } + if (targets.empty()) { + LOG(INFO) << "Dropping event because no targets were found: " << entry.getDescription(); + outInjectionResult = InputEventInjectionResult::FAILED; + return {}; + } + + // If we only have windows getting ACTION_OUTSIDE, then drop the event, because there is no + // window that is actually receiving the entire gesture. + if (std::all_of(targets.begin(), targets.end(), [](const InputTarget& target) { + return target.flags.test(InputTarget::Flags::DISPATCH_AS_OUTSIDE); + })) { + LOG(INFO) << "Dropping event because all windows would just receive ACTION_OUTSIDE: " + << entry.getDescription(); + outInjectionResult = InputEventInjectionResult::FAILED; + return {}; + } + outInjectionResult = InputEventInjectionResult::SUCCEEDED; // Drop the outside or hover touch windows since we will not care about them // in the next iteration. @@ -5483,7 +5501,7 @@ void InputDispatcher::logDispatchStateLocked() const { std::string line; while (std::getline(stream, line, '\n')) { - ALOGD("%s", line.c_str()); + ALOGI("%s", line.c_str()); } } diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 147b7d4a83..41c4d69946 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -62,6 +62,7 @@ static constexpr int32_t ACTION_DOWN = AMOTION_EVENT_ACTION_DOWN; static constexpr int32_t ACTION_MOVE = AMOTION_EVENT_ACTION_MOVE; static constexpr int32_t ACTION_UP = AMOTION_EVENT_ACTION_UP; static constexpr int32_t ACTION_HOVER_ENTER = AMOTION_EVENT_ACTION_HOVER_ENTER; +static constexpr int32_t ACTION_HOVER_MOVE = AMOTION_EVENT_ACTION_HOVER_MOVE; static constexpr int32_t ACTION_HOVER_EXIT = AMOTION_EVENT_ACTION_HOVER_EXIT; static constexpr int32_t ACTION_OUTSIDE = AMOTION_EVENT_ACTION_OUTSIDE; static constexpr int32_t ACTION_CANCEL = AMOTION_EVENT_ACTION_CANCEL; @@ -2454,6 +2455,116 @@ TEST_F(InputDispatcherTest, HoverFromLeftToRightAndTap) { rightWindow->assertNoEvents(); } +/** + * Start hovering in a window. While this hover is still active, make another window appear on top. + * The top, obstructing window has no input channel, so it's not supposed to receive input. + * While the top window is present, the hovering is stopped. + * Later, hovering gets resumed again. + * Ensure that new hover gesture is handled correctly. + * This test reproduces a crash where the HOVER_EXIT event wasn't getting dispatched correctly + * to the window that's currently being hovered over. + */ +TEST_F(InputDispatcherTest, HoverWhileWindowAppears) { + std::shared_ptr application = std::make_shared(); + sp window = + sp::make(application, mDispatcher, "Window", ADISPLAY_ID_DEFAULT); + window->setFrame(Rect(0, 0, 200, 200)); + + // Only a single window is present at first + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {window}}}); + + // Start hovering in the window + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_ENTER)); + + // Now, an obscuring window appears! + sp obscuringWindow = + sp::make(application, mDispatcher, "Obscuring window", + ADISPLAY_ID_DEFAULT, + /*token=*/std::make_optional>(nullptr)); + obscuringWindow->setFrame(Rect(0, 0, 200, 200)); + obscuringWindow->setTouchOcclusionMode(TouchOcclusionMode::BLOCK_UNTRUSTED); + obscuringWindow->setOwnerInfo(SECONDARY_WINDOW_PID, SECONDARY_WINDOW_UID); + obscuringWindow->setNoInputChannel(true); + obscuringWindow->setFocusable(false); + obscuringWindow->setAlpha(1.0); + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {obscuringWindow, window}}}); + + // While this new obscuring window is present, the hovering is stopped + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_EXIT, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_EXIT)); + + // Now the obscuring window goes away. + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {window}}}); + + // And a new hover gesture starts. + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_ENTER)); +} + +/** + * Same test as 'HoverWhileWindowAppears' above, but here, we also send some HOVER_MOVE events to + * the obscuring window. + */ +TEST_F(InputDispatcherTest, HoverMoveWhileWindowAppears) { + std::shared_ptr application = std::make_shared(); + sp window = + sp::make(application, mDispatcher, "Window", ADISPLAY_ID_DEFAULT); + window->setFrame(Rect(0, 0, 200, 200)); + + // Only a single window is present at first + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {window}}}); + + // Start hovering in the window + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_ENTER, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_ENTER)); + + // Now, an obscuring window appears! + sp obscuringWindow = + sp::make(application, mDispatcher, "Obscuring window", + ADISPLAY_ID_DEFAULT, + /*token=*/std::make_optional>(nullptr)); + obscuringWindow->setFrame(Rect(0, 0, 200, 200)); + obscuringWindow->setTouchOcclusionMode(TouchOcclusionMode::BLOCK_UNTRUSTED); + obscuringWindow->setOwnerInfo(SECONDARY_WINDOW_PID, SECONDARY_WINDOW_UID); + obscuringWindow->setNoInputChannel(true); + obscuringWindow->setFocusable(false); + obscuringWindow->setAlpha(1.0); + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {obscuringWindow, window}}}); + + // While this new obscuring window is present, the hovering continues. The event can't go to the + // bottom window due to obstructed touches, so it should generate HOVER_EXIT for that window. + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_MOVE, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + obscuringWindow->assertNoEvents(); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_EXIT)); + + // Now the obscuring window goes away. + mDispatcher->setInputWindows({{ADISPLAY_ID_DEFAULT, {window}}}); + + // Hovering continues in the same position. The hovering pointer re-enters the bottom window, + // so it should generate a HOVER_ENTER + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_MOVE, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(100).y(100)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_ENTER)); + + // Now the MOVE should be getting dispatched normally + mDispatcher->notifyMotion(MotionArgsBuilder(ACTION_HOVER_MOVE, AINPUT_SOURCE_STYLUS) + .pointer(PointerBuilder(0, ToolType::STYLUS).x(110).y(110)) + .build()); + window->consumeMotionEvent(WithMotionAction(ACTION_HOVER_MOVE)); +} + /** * Two windows: a window on the left and a window on the right. * Mouse is clicked on the left window and remains down. Touch is touched on the right and remains @@ -3446,7 +3557,9 @@ TEST_F(InputDispatcherTest, HoverEnterMouseClickAndHoverExit) { .build())); window->consumeMotionUp(ADISPLAY_ID_DEFAULT); - ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, + // We already canceled the hovering implicitly by injecting the "DOWN" event without lifting the + // hover first. Therefore, injection of HOVER_EXIT is inconsistent, and should fail. + ASSERT_EQ(InputEventInjectionResult::FAILED, injectMotionEvent(mDispatcher, MotionEventBuilder(AMOTION_EVENT_ACTION_HOVER_EXIT, AINPUT_SOURCE_MOUSE) -- cgit v1.2.3-59-g8ed1b