Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 55a24c6c authored by Linnan Li's avatar Linnan Li Committed by Daniel Norman
Browse files

Fix DelayedTransition async use original MotionEvent obj



Because MotionEvent can reuse, so if we async use MotionEvent, we
should use it's copy, or it will cause some exception randomly.

Bug: 280130713
Test: existing internal+CTS gesture tests
Test: atest TwoFingersDownOrSwipeTest
Change-Id: I5d123ac19e158a490f0f05e3f3112403ddf4e03e
Signed-off-by: default avatarLinnan Li <lilinnan@xiaomi.corp-partner.google.com>
parent 2a3724b9
Loading
Loading
Loading
Loading
+30 −6
Original line number Diff line number Diff line
@@ -152,6 +152,7 @@ public final class AccessibilityGestureEvent implements Parcelable {

    /**
     * Constructs an AccessibilityGestureEvent to be dispatched to an accessibility service.
     *
     * @param gestureId    the id number of the gesture.
     * @param displayId    the display on which this gesture was performed.
     * @param motionEvents the motion events that lead to this gesture.
@@ -205,6 +206,29 @@ public final class AccessibilityGestureEvent implements Parcelable {
        return mMotionEvents;
    }

    /**
     * When we asynchronously use {@link AccessibilityGestureEvent}, we should make a copy,
     * because motionEvent may be recycled before we use async.
     *
     * @hide
     */
    @NonNull
    public AccessibilityGestureEvent copyForAsync() {
        return new AccessibilityGestureEvent(mGestureId, mDisplayId,
                mMotionEvents.stream().map(MotionEvent::copy).toList());
    }

    /**
     * After we use {@link AccessibilityGestureEvent} asynchronously, we should recycle the
     * MotionEvent, avoid memory leaks.
     *
     * @hide
     */
    public void recycle() {
        mMotionEvents.forEach(MotionEvent::recycle);
        mMotionEvents.clear();
    }

    @NonNull
    @Override
    public String toString() {
+8 −1
Original line number Diff line number Diff line
@@ -37,6 +37,13 @@ flag {
    bug: "302376158"
}

flag {
    namespace: "accessibility"
    name: "copy_events_for_gesture_detection"
    description: "Creates copies of MotionEvents and GestureEvents in GestureMatcher"
    bug: "280130713"
}

flag {
    namespace: "accessibility"
    name: "flash_notification_system_api"
+14 −4
Original line number Diff line number Diff line
@@ -1848,9 +1848,15 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
    }

    public void notifyGesture(AccessibilityGestureEvent gestureEvent) {
        if (android.view.accessibility.Flags.copyEventsForGestureDetection()) {
            // We will use this event async, so copy it because it contains MotionEvents.
            mInvocationHandler.obtainMessage(InvocationHandler.MSG_ON_GESTURE,
                    gestureEvent.copyForAsync()).sendToTarget();
        } else {
            mInvocationHandler.obtainMessage(InvocationHandler.MSG_ON_GESTURE,
                    gestureEvent).sendToTarget();
        }
    }

    public void notifySystemActionsChangedLocked() {
        mInvocationHandler.sendEmptyMessage(
@@ -2323,9 +2329,13 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
            final int type = message.what;
            switch (type) {
                case MSG_ON_GESTURE: {
                    notifyGestureInternal((AccessibilityGestureEvent) message.obj);
                    if (message.obj instanceof AccessibilityGestureEvent gesture) {
                        notifyGestureInternal(gesture);
                        if (android.view.accessibility.Flags.copyEventsForGestureDetection()) {
                            gesture.recycle();
                        }
                    }
                } break;

                case MSG_CLEAR_ACCESSIBILITY_CACHE: {
                    notifyClearAccessibilityCacheInternal();
                } break;
+23 −2
Original line number Diff line number Diff line
@@ -328,13 +328,21 @@ public abstract class GestureMatcher {
                                + getStateSymbolicName(mTargetState));
            }
            mHandler.removeCallbacks(this);
            recycleEvent();
        }

        public void post(
                int state, long delay, MotionEvent event, MotionEvent rawEvent, int policyFlags) {
            // Recycle the old event first if necessary, to handle duplicate calls to post.
            recycleEvent();
            mTargetState = state;
            if (android.view.accessibility.Flags.copyEventsForGestureDetection()) {
                mEvent = event.copy();
                mRawEvent = rawEvent.copy();
            } else {
                mEvent = event;
                mRawEvent = rawEvent;
            }
            mPolicyFlags = policyFlags;
            mHandler.postDelayed(this, delay);
            if (DEBUG) {
@@ -367,6 +375,19 @@ public abstract class GestureMatcher {
                                + getStateSymbolicName(mTargetState));
            }
            setState(mTargetState, mEvent, mRawEvent, mPolicyFlags);
            recycleEvent();
        }

        private void recycleEvent() {
            if (android.view.accessibility.Flags.copyEventsForGestureDetection()) {
                if (mEvent == null || mRawEvent == null) {
                    return;
                }
                mEvent.recycle();
                mRawEvent.recycle();
                mEvent = null;
                mRawEvent = null;
            }
        }
    }

+28 −1
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static com.android.server.accessibility.utils.TouchEventGenerator.movePoi
import static com.android.server.accessibility.utils.TouchEventGenerator.twoPointersDownEvents;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.after;
import static org.mockito.Mockito.timeout;
@@ -27,6 +28,10 @@ import static org.mockito.Mockito.verify;

import android.content.Context;
import android.graphics.PointF;
import android.platform.test.annotations.RequiresFlagsDisabled;
import android.platform.test.annotations.RequiresFlagsEnabled;
import android.platform.test.flag.junit.CheckFlagsRule;
import android.platform.test.flag.junit.DeviceFlagsValueProvider;
import android.view.Display;
import android.view.MotionEvent;
import android.view.ViewConfiguration;
@@ -37,6 +42,7 @@ import com.android.server.accessibility.utils.TouchEventGenerator;

import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@@ -48,6 +54,9 @@ import java.util.List;
 */
public class TwoFingersDownOrSwipeTest {

    @Rule
    public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();

    private static final float DEFAULT_X = 100f;
    private static final float DEFAULT_Y = 100f;

@@ -85,7 +94,8 @@ public class TwoFingersDownOrSwipeTest {
    }

    @Test
    public void sendTwoFingerDownEvent_onGestureCompleted() {
    @RequiresFlagsDisabled(android.view.accessibility.Flags.FLAG_COPY_EVENTS_FOR_GESTURE_DETECTION)
    public void sendTwoFingerDownEvent_onGestureCompleted_withoutCopiedEvents() {
        final List<MotionEvent> downEvents = twoPointersDownEvents(Display.DEFAULT_DISPLAY,
                new PointF(DEFAULT_X, DEFAULT_Y), new PointF(DEFAULT_X + 10, DEFAULT_Y + 10));

@@ -98,6 +108,23 @@ public class TwoFingersDownOrSwipeTest {
                downEvents.get(1), 0);
    }

    @Test
    @RequiresFlagsEnabled(android.view.accessibility.Flags.FLAG_COPY_EVENTS_FOR_GESTURE_DETECTION)
    public void sendTwoFingerDownEvent_onGestureCompleted() {
        final List<MotionEvent> downEvents = twoPointersDownEvents(Display.DEFAULT_DISPLAY,
                new PointF(DEFAULT_X, DEFAULT_Y), new PointF(DEFAULT_X + 10, DEFAULT_Y + 10));

        for (MotionEvent event : downEvents) {
            mGesturesObserver.onMotionEvent(event, event, 0);
        }

        verify(mListener, timeout(sTimeoutMillis)).onGestureCompleted(
                eq(MagnificationGestureMatcher.GESTURE_TWO_FINGERS_DOWN_OR_SWIPE),
                argThat(argument -> downEvents.get(1).getId() == argument.getId()),
                argThat(argument -> downEvents.get(1).getId() == argument.getId()),
                eq(0));
    }

    @Test
    public void sendSingleTapEvent_onGestureCancelled() {
        final MotionEvent downEvent = TouchEventGenerator.downEvent(Display.DEFAULT_DISPLAY,