From d88bfd50156b67e9a96059f6a06e7b762937c5cb Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Fri, 24 Jan 2025 20:52:31 +0000 Subject: Cleanup mOpenGesturesInProgress to use a simple boolean. This SparseArray is only ever used to make decisions for events with source == EVENT_SOURCE, so this change simplifies that array to use a single boolean instead of an array of booleans. Bug: 384451671 Test: existing tests in presubmit Flag: EXEMPT no-op refactoring Change-Id: Ie20fc3a43d4a15f433953ead94bdc31c578d0eb8 --- .../server/accessibility/MotionEventInjector.java | 35 +++++++++++----------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'services/accessibility/java') diff --git a/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java b/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java index b2169535d0de..caab2138ecc7 100644 --- a/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java +++ b/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java @@ -28,7 +28,6 @@ import android.os.RemoteException; import android.os.SystemClock; import android.util.IntArray; import android.util.Slog; -import android.util.SparseArray; import android.util.SparseIntArray; import android.view.InputDevice; import android.view.KeyCharacterMap; @@ -67,7 +66,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement private static MotionEvent.PointerProperties[] sPointerProps; private final Handler mHandler; - private final SparseArray mOpenGesturesInProgress = new SparseArray<>(); + private boolean mOpenTouchGestureInProgress = false; private final AccessibilityTraceManager mTrace; private IAccessibilityServiceClient mServiceInterfaceForCurrentGesture; @@ -130,7 +129,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement // cancellation of the gesture. if ((event.isFromSource(InputDevice.SOURCE_MOUSE) && event.getActionMasked() == MotionEvent.ACTION_HOVER_MOVE) - && mOpenGesturesInProgress.get(EVENT_SOURCE, false)) { + && mOpenTouchGestureInProgress) { return; } cancelAnyPendingInjectedEvents(); @@ -148,8 +147,8 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement * Reset state for motion events passing through so we won't send a cancel event for * them. */ - if (!mHandler.hasMessages(MESSAGE_SEND_MOTION_EVENT)) { - mOpenGesturesInProgress.put(inputSource, false); + if (!mHandler.hasMessages(MESSAGE_SEND_MOTION_EVENT) && inputSource == EVENT_SOURCE) { + mOpenTouchGestureInProgress = false; } } @@ -225,7 +224,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement if (!continuingGesture) { cancelAnyPendingInjectedEvents(); // Injected gestures have been canceled, but real gestures still need cancelling - cancelAnyGestureInProgress(EVENT_SOURCE); + cancelAnyGestureInProgress(); } mServiceInterfaceForCurrentGesture = serviceInterface; @@ -323,18 +322,20 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement int policyFlags) { if (getNext() != null) { super.onMotionEvent(event, rawEvent, policyFlags); - if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { - mOpenGesturesInProgress.put(event.getSource(), true); - } - if ((event.getActionMasked() == MotionEvent.ACTION_UP) - || (event.getActionMasked() == MotionEvent.ACTION_CANCEL)) { - mOpenGesturesInProgress.put(event.getSource(), false); + if (event.getSource() == EVENT_SOURCE) { + if (event.getActionMasked() == MotionEvent.ACTION_DOWN) { + mOpenTouchGestureInProgress = true; + } + if ((event.getActionMasked() == MotionEvent.ACTION_UP) + || (event.getActionMasked() == MotionEvent.ACTION_CANCEL)) { + mOpenTouchGestureInProgress = false; + } } } } - private void cancelAnyGestureInProgress(int source) { - if ((getNext() != null) && mOpenGesturesInProgress.get(source, false)) { + private void cancelAnyGestureInProgress() { + if ((getNext() != null) && mOpenTouchGestureInProgress) { long now = SystemClock.uptimeMillis(); MotionEvent cancelEvent = obtainMotionEvent(now, now, MotionEvent.ACTION_CANCEL, getLastTouchPoints(), 1); @@ -348,14 +349,14 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement policyFlags |= WindowManagerPolicyConstants.FLAG_INJECTED_FROM_ACCESSIBILITY_TOOL; } sendMotionEventToNext(cancelEvent, cancelEvent, policyFlags); - mOpenGesturesInProgress.put(source, false); + mOpenTouchGestureInProgress = false; } } private void cancelAnyPendingInjectedEvents() { if (mHandler.hasMessages(MESSAGE_SEND_MOTION_EVENT)) { mHandler.removeMessages(MESSAGE_SEND_MOTION_EVENT); - cancelAnyGestureInProgress(EVENT_SOURCE); + cancelAnyGestureInProgress(); for (int i = mSequencesInProgress.size() - 1; i >= 0; i--) { notifyService(mServiceInterfaceForCurrentGesture, mSequencesInProgress.get(i), false); @@ -363,7 +364,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement } } else if (mNumLastTouchPoints != 0) { // An injected gesture is in progress and waiting for a continuation. Cancel it. - cancelAnyGestureInProgress(EVENT_SOURCE); + cancelAnyGestureInProgress(); } mNumLastTouchPoints = 0; mStrokeIdToPointerId.clear(); -- cgit v1.2.3-59-g8ed1b From 614c6be5aea885432f86b9638de15c1f292f6402 Mon Sep 17 00:00:00 2001 From: Daniel Norman Date: Wed, 5 Feb 2025 19:38:01 +0000 Subject: Removes ACTION_CANCEL which caused InputDispatcher inconsistency. This ACTION_CANCEL was used for two cases: 1. Cancelling an in-progress injected gesture if another injected gesture is requested. 2. Cancelling an in-progress real gesture if a new injected gesture is requested. (2) was not functional because this ACTION_CANCEL had the deviceId of the injected device, not the device used by real gestures. This bad ACTION_CANCEL was then causing an inconsistent stream for future injected events. Rather than fix (2) this change removes that CANCEL case altogether; as of http://ag/21431224 CANCELing the real gesture is not necessary because InputDispatcher already tracks deviceId and cancels the real gesture on its own when A11y sends the injected gesture. Bug: 384451671 Test: Manual testing; see b/384451671 #comment20 Test: atest MotionEventInjectorTest Flag: com.android.server.accessibility.motion_event_injector_cancel_fix Change-Id: I09aef2c853dfe86142e8b60e5b64c4560b2265a0 --- services/accessibility/accessibility.aconfig | 10 ++++ .../server/accessibility/MotionEventInjector.java | 42 +++++++++----- .../accessibility/MotionEventInjectorTest.java | 67 +++++++++++++++++++++- 3 files changed, 102 insertions(+), 17 deletions(-) (limited to 'services/accessibility/java') diff --git a/services/accessibility/accessibility.aconfig b/services/accessibility/accessibility.aconfig index 722255404dd1..e8dddcb537cd 100644 --- a/services/accessibility/accessibility.aconfig +++ b/services/accessibility/accessibility.aconfig @@ -208,6 +208,16 @@ flag { } } +flag { + name: "motion_event_injector_cancel_fix" + namespace: "accessibility" + description: "Fix the ACTION_CANCEL logic used in MotionEventInjector to avoid InputDispatcher inconsistency" + bug: "384451671" + metadata { + purpose: PURPOSE_BUGFIX + } +} + flag { name: "package_monitor_dedicated_thread" namespace: "accessibility" diff --git a/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java b/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java index caab2138ecc7..a7203b0b83f1 100644 --- a/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java +++ b/services/accessibility/java/com/android/server/accessibility/MotionEventInjector.java @@ -115,6 +115,8 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement mHandler.sendMessage(mHandler.obtainMessage(MESSAGE_INJECT_EVENTS, args)); } + // Note: MotionEventInjector is the first transformation in the AccessibilityInputFilter stream + // so any event that arrives here is a real event from a real user interaction. @Override public void onMotionEvent(MotionEvent event, MotionEvent rawEvent, int policyFlags) { if (mTrace.isA11yTracingEnabledForTypes( @@ -123,22 +125,30 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement AccessibilityTrace.FLAGS_INPUT_FILTER | AccessibilityTrace.FLAGS_GESTURE, "event=" + event + ";rawEvent=" + rawEvent + ";policyFlags=" + policyFlags); } - // MotionEventInjector would cancel any injected gesture when any MotionEvent arrives. - // For user using an external device to control the pointer movement, it's almost - // impossible to perform the gestures. Any slightly unintended movement results in the - // cancellation of the gesture. + // InputDispatcher cancels an injected touch gesture if another MotionEvent arrives on this + // display from another device or source, like a real touch or a real mouse pointer. + // For user using an external device to control the pointer movement, it becomes difficult + // to perform injected gestures because slight unintended movement results in cancellation + // of the injected gesture; to fix this we swallow real mouse MotionEvents while an injected + // touch gesture is in progress, preventing the mouse events from reaching InputDispatcher. if ((event.isFromSource(InputDevice.SOURCE_MOUSE) && event.getActionMasked() == MotionEvent.ACTION_HOVER_MOVE) && mOpenTouchGestureInProgress) { return; } - cancelAnyPendingInjectedEvents(); - if (!android.view.accessibility.Flags.preventA11yNontoolFromInjectingIntoSensitiveViews()) { - // Indicate that the input event is injected from accessibility, to let applications - // distinguish it from events injected by other means. - policyFlags |= WindowManagerPolicyConstants.FLAG_INJECTED_FROM_ACCESSIBILITY; + if (Flags.motionEventInjectorCancelFix()) { + // Pass this real event down the stream unmodified. + super.onMotionEvent(event, rawEvent, policyFlags); + } else { + cancelAnyPendingInjectedEvents(); + if (!android.view.accessibility.Flags + .preventA11yNontoolFromInjectingIntoSensitiveViews()) { + // Indicate that the input event is injected from accessibility, to let applications + // distinguish it from events injected by other means. + policyFlags |= WindowManagerPolicyConstants.FLAG_INJECTED_FROM_ACCESSIBILITY; + } + sendMotionEventToNext(event, rawEvent, policyFlags); } - sendMotionEventToNext(event, rawEvent, policyFlags); } @Override @@ -223,8 +233,10 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement } if (!continuingGesture) { cancelAnyPendingInjectedEvents(); - // Injected gestures have been canceled, but real gestures still need cancelling - cancelAnyGestureInProgress(); + if (!Flags.motionEventInjectorCancelFix()) { + // Injected gestures have been canceled, but real gestures still need cancelling + cancelInjectedGestureInProgress(); + } } mServiceInterfaceForCurrentGesture = serviceInterface; @@ -334,7 +346,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement } } - private void cancelAnyGestureInProgress() { + private void cancelInjectedGestureInProgress() { if ((getNext() != null) && mOpenTouchGestureInProgress) { long now = SystemClock.uptimeMillis(); MotionEvent cancelEvent = @@ -356,7 +368,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement private void cancelAnyPendingInjectedEvents() { if (mHandler.hasMessages(MESSAGE_SEND_MOTION_EVENT)) { mHandler.removeMessages(MESSAGE_SEND_MOTION_EVENT); - cancelAnyGestureInProgress(); + cancelInjectedGestureInProgress(); for (int i = mSequencesInProgress.size() - 1; i >= 0; i--) { notifyService(mServiceInterfaceForCurrentGesture, mSequencesInProgress.get(i), false); @@ -364,7 +376,7 @@ public class MotionEventInjector extends BaseEventStreamTransformation implement } } else if (mNumLastTouchPoints != 0) { // An injected gesture is in progress and waiting for a continuation. Cancel it. - cancelAnyGestureInProgress(); + cancelInjectedGestureInProgress(); } mNumLastTouchPoints = 0; mStrokeIdToPointerId.clear(); diff --git a/services/tests/servicestests/src/com/android/server/accessibility/MotionEventInjectorTest.java b/services/tests/servicestests/src/com/android/server/accessibility/MotionEventInjectorTest.java index d2d8c682ed90..c75cfe67ab79 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/MotionEventInjectorTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/MotionEventInjectorTest.java @@ -274,7 +274,10 @@ public class MotionEventInjectorTest { } @Test - @DisableFlags(FLAG_PREVENT_A11Y_NONTOOL_FROM_INJECTING_INTO_SENSITIVE_VIEWS) + @DisableFlags({ + FLAG_PREVENT_A11Y_NONTOOL_FROM_INJECTING_INTO_SENSITIVE_VIEWS, + Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX + }) public void testRegularEvent_afterGestureComplete_shouldPassToNext_withFlagInjectedFromA11y() { EventStreamTransformation next = attachMockNext(mMotionEventInjector); injectEventsSync(mLineList, mServiceInterface, LINE_SEQUENCE); @@ -286,7 +289,10 @@ public class MotionEventInjectorTest { } @Test - @EnableFlags(FLAG_PREVENT_A11Y_NONTOOL_FROM_INJECTING_INTO_SENSITIVE_VIEWS) + @EnableFlags({ + FLAG_PREVENT_A11Y_NONTOOL_FROM_INJECTING_INTO_SENSITIVE_VIEWS, + Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX + }) public void testRegularEvent_afterGestureComplete_shouldPassToNext_withNoPolicyFlagChanges() { EventStreamTransformation next = attachMockNext(mMotionEventInjector); injectEventsSync(mLineList, mServiceInterface, LINE_SEQUENCE); @@ -299,6 +305,7 @@ public class MotionEventInjectorTest { } @Test + @DisableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) public void testInjectEvents_withRealGestureUnderway_shouldCancelRealAndPassInjected() { EventStreamTransformation next = attachMockNext(mMotionEventInjector); mMotionEventInjector.onMotionEvent(mClickDownEvent, mClickDownEvent, 0); @@ -316,6 +323,24 @@ public class MotionEventInjectorTest { | WindowManagerPolicyConstants.FLAG_INJECTED_FROM_ACCESSIBILITY)); } + @Test + @EnableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) + public void testInjectEvents_withRealGestureUnderway_shouldNotCancelReal_ShouldPassInjected() { + EventStreamTransformation next = attachMockNext(mMotionEventInjector); + mMotionEventInjector.onMotionEvent(mClickDownEvent, mClickDownEvent, 0); + injectEventsSync(mLineList, mServiceInterface, LINE_SEQUENCE); + + verify(next, times(1)).onMotionEvent(mCaptor1.capture(), mCaptor2.capture(), anyInt()); + assertThat(mCaptor1.getAllValues().get(0), mIsClickDown); + reset(next); + + mMessageCapturingHandler.sendOneMessage(); // Send a motion event + verify(next).onMotionEvent( + argThat(mIsLineStart), argThat(mIsLineStart), + eq(WindowManagerPolicyConstants.FLAG_PASS_TO_USER + | WindowManagerPolicyConstants.FLAG_INJECTED_FROM_ACCESSIBILITY)); + } + @Test public void testInjectEvents_withRealMouseGestureUnderway_shouldContinueRealAndPassInjected() { EventStreamTransformation next = attachMockNext(mMotionEventInjector); @@ -354,6 +379,7 @@ public class MotionEventInjectorTest { } @Test + @DisableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) public void testOnMotionEvents_openInjectedGestureInProgress_shouldCancelAndNotifyAndPassReal() throws RemoteException { EventStreamTransformation next = attachMockNext(mMotionEventInjector); @@ -368,6 +394,24 @@ public class MotionEventInjectorTest { verify(mServiceInterface).onPerformGestureResult(LINE_SEQUENCE, false); } + @Test + @EnableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) + public void testOnMotionEvents_openInjectedGestureInProgress_shouldNotCancel_shouldPassReal() + throws RemoteException { + EventStreamTransformation next = attachMockNext(mMotionEventInjector); + injectEventsSync(mLineList, mServiceInterface, LINE_SEQUENCE); + mMessageCapturingHandler.sendOneMessage(); // Send a motion event + mMotionEventInjector.onMotionEvent(mClickDownEvent, mClickDownEvent, 0); + mMessageCapturingHandler.sendAllMessages(); + + verify(next, times(4)).onMotionEvent(mCaptor1.capture(), mCaptor2.capture(), anyInt()); + assertThat(mCaptor1.getAllValues().get(0), mIsLineStart); + assertThat(mCaptor1.getAllValues().get(1), mIsClickDown); + assertThat(mCaptor1.getAllValues().get(2), mIsLineMiddle); + assertThat(mCaptor1.getAllValues().get(3), mIsLineEnd); + verify(mServiceInterface).onPerformGestureResult(LINE_SEQUENCE, true); + } + @Test public void testOnMotionEvents_fromMouseWithInjectedGestureInProgress_shouldNotCancelAndPassReal() @@ -386,6 +430,7 @@ public class MotionEventInjectorTest { } @Test + @DisableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) public void testOnMotionEvents_closedInjectedGestureInProgress_shouldOnlyNotifyAndPassReal() throws RemoteException { EventStreamTransformation next = attachMockNext(mMotionEventInjector); @@ -676,6 +721,7 @@ public class MotionEventInjectorTest { } @Test + @DisableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) public void testContinuedGesture_realGestureArrivesInBetween_getsCanceled() throws Exception { EventStreamTransformation next = attachMockNext(mMotionEventInjector); @@ -710,6 +756,7 @@ public class MotionEventInjectorTest { } @Test + @DisableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) public void testClearEventsOnOtherSource_realGestureInProgress_shouldNotForgetAboutGesture() { EventStreamTransformation next = attachMockNext(mMotionEventInjector); mMotionEventInjector.onMotionEvent(mClickDownEvent, mClickDownEvent, 0); @@ -723,6 +770,22 @@ public class MotionEventInjectorTest { assertThat(mCaptor1.getAllValues().get(2), mIsLineStart); } + @Test + @EnableFlags(Flags.FLAG_MOTION_EVENT_INJECTOR_CANCEL_FIX) + public void testClearEventsOnOtherSource_shouldNotCancelRealOrInjectedGesture() { + EventStreamTransformation next = attachMockNext(mMotionEventInjector); + mMotionEventInjector.onMotionEvent(mClickDownEvent, mClickDownEvent, 0); + mMotionEventInjector.clearEvents(OTHER_EVENT_SOURCE); + injectEventsSync(mLineList, mServiceInterface, LINE_SEQUENCE); + mMessageCapturingHandler.sendAllMessages(); + + verify(next, times(4)).onMotionEvent(mCaptor1.capture(), mCaptor2.capture(), anyInt()); + assertThat(mCaptor1.getAllValues().get(0), mIsClickDown); + assertThat(mCaptor1.getAllValues().get(1), mIsLineStart); + assertThat(mCaptor1.getAllValues().get(2), mIsLineMiddle); + assertThat(mCaptor1.getAllValues().get(3), mIsLineEnd); + } + @Test public void testOnDestroy_shouldCancelGestures() throws RemoteException { mMotionEventInjector.onDestroy(); -- cgit v1.2.3-59-g8ed1b