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

Commit 7853e418 authored by Sally's avatar Sally
Browse files

Service requests can interrupt node prefetching

This is the re re..re-merging of ag/12923546 (where most of that original
message is posted below). This includes various bug fixes.

Slow prefetch requests would block user interactive requests, creating
noticeable sluggishness and unresponsiveness in accessibility services,
especially on the web.

Let's make it so a user interactive requests stops prefetching.
We can't interrupt an API call, but we can stop in between API calls.

On the service side, we have to separate the prefetch callbacks from the
find callback. And we have to make it asynchronous. It does dispatch
intothe main thread, so the AccessibilityCache can remain single threaded.

When the calls are interrupted on the application side,
returnPendingFindAccessibilityNodeInfosInPrefetch checks the find
requests that are waiting in the queue, to see if they can be addressed
by the prefetch results. If they can be, we don't have to call into
potentially non-performant application code. We don't  check requests
that have differing prefetch flags (FLAG_INCLUDE_NOT_IMPORTANT_VIEWS,
FLAG_REPORT_VIEW_IDS) that would result in different caches. We satisfy
at most one pending request.

We also make mPendingFindNodeByIdMessages thread-safe and ensure in
ActionReplacingCallback we don't return null results. Merged
ag/13246536, ag/13256330

Messages should be added to PrivateHandler and
mPendingFindNodeIdMessages at the same time to avoid a race condition
where we try removing a message from the handler before it's actually
enqueued. This was causing double recycling

We call into AccessibilityCache from the Binder thread, now that the
cache doesn’t call out to the app main thread with a lock(ag/14020225)

Added tests to verify AccessibilityInteractionController interactions

Test: atest AccessibilityInteractionControllerNodeRequestsTest
atest CtsAccessibilityServiceTestCases  CtsAccessibilityTestCases
CtsUiAutomationTestCases
FrameworksServicesTests:com.android.server.accessibility
FrameworksCoreTests:com.android.internal.accessibility
FrameworksCoreTests:android.view.accessibility

Bug: b/184076735
Change-Id: Iaad1b9100655ec86a788ba1c89edd2dd8a7df1f6
parent d0f987ee
Loading
Loading
Loading
Loading
+254 −130

File changed.

Preview size limit exceeded, changes collapsed.

+69 −14
Original line number Diff line number Diff line
@@ -130,6 +130,10 @@ public final class AccessibilityInteractionClient

    private Message mSameThreadMessage;

    private int mInteractionIdWaitingForPrefetchResult = -1;
    private int mConnectionIdWaitingForPrefetchResult;
    private String[] mPackageNamesForNextPrefetchResult;

    /**
     * @return The client for the current thread.
     */
@@ -494,25 +498,27 @@ public final class AccessibilityInteractionClient
                    Binder.restoreCallingIdentity(identityToken);
                }
                if (packageNames != null) {
                    List<AccessibilityNodeInfo> infos = getFindAccessibilityNodeInfosResultAndClear(
                            interactionId);
                    AccessibilityNodeInfo info =
                            getFindAccessibilityNodeInfoResultAndClear(interactionId);
                    if (mAccessibilityManager != null
                            && mAccessibilityManager.isAccessibilityTracingEnabled()) {
                        logTrace(connection, "findAccessibilityNodeInfoByAccessibilityId",
                                "InteractionId:" + interactionId + ";Result: " + infos
                                + ";connectionId=" + connectionId + ";accessibilityWindowId="
                                "InteractionId:" + interactionId + ";Result: " + info
                                        + ";connectionId=" + connectionId
                                        + ";accessibilityWindowId="
                                        + accessibilityWindowId + ";accessibilityNodeId="
                                        + accessibilityNodeId + ";bypassCache=" + bypassCache
                                + ";prefetchFlags=" + prefetchFlags + ";arguments=" + arguments);
                    }
                    finalizeAndCacheAccessibilityNodeInfos(infos, connectionId,
                            bypassCache, packageNames);
                    if (infos != null && !infos.isEmpty()) {
                        for (int i = 1; i < infos.size(); i++) {
                            infos.get(i).recycle();
                                        + ";prefetchFlags=" + prefetchFlags
                                        + ";arguments=" + arguments);
                    }
                        return infos.get(0);
                    if ((prefetchFlags & AccessibilityNodeInfo.FLAG_PREFETCH_MASK) != 0
                            && info != null) {
                        setInteractionWaitingForPrefetchResult(interactionId, connectionId,
                                packageNames);
                    }
                    finalizeAndCacheAccessibilityNodeInfo(info, connectionId,
                            bypassCache, packageNames);
                    return info;
                }
            } else {
                if (DEBUG) {
@@ -526,6 +532,15 @@ public final class AccessibilityInteractionClient
        return null;
    }

    private void setInteractionWaitingForPrefetchResult(int interactionId, int connectionId,
            String[] packageNames) {
        synchronized (mInstanceLock) {
            mInteractionIdWaitingForPrefetchResult = interactionId;
            mConnectionIdWaitingForPrefetchResult = connectionId;
            mPackageNamesForNextPrefetchResult = packageNames;
        }
    }

    private static String idToString(int accessibilityWindowId, long accessibilityNodeId) {
        return accessibilityWindowId + "/"
                + AccessibilityNodeInfo.idToString(accessibilityNodeId);
@@ -923,6 +938,46 @@ public final class AccessibilityInteractionClient
        }
    }

    /**
     * {@inheritDoc}
     */
    @Override
    public void setPrefetchAccessibilityNodeInfoResult(@NonNull List<AccessibilityNodeInfo> infos,
                                                       int interactionId) {
        int interactionIdWaitingForPrefetchResultCopy = -1;
        int connectionIdWaitingForPrefetchResultCopy = -1;
        String[] packageNamesForNextPrefetchResultCopy = null;

        if (infos.isEmpty()) {
            return;
        }

        synchronized (mInstanceLock) {
            if (mInteractionIdWaitingForPrefetchResult == interactionId) {
                interactionIdWaitingForPrefetchResultCopy = mInteractionIdWaitingForPrefetchResult;
                connectionIdWaitingForPrefetchResultCopy =
                        mConnectionIdWaitingForPrefetchResult;
                if (mPackageNamesForNextPrefetchResult != null) {
                    packageNamesForNextPrefetchResultCopy =
                            new String[mPackageNamesForNextPrefetchResult.length];
                    for (int i = 0; i < mPackageNamesForNextPrefetchResult.length; i++) {
                        packageNamesForNextPrefetchResultCopy[i] =
                                mPackageNamesForNextPrefetchResult[i];
                    }
                }
            }
        }

        if (interactionIdWaitingForPrefetchResultCopy == interactionId) {
            finalizeAndCacheAccessibilityNodeInfos(
                    infos, connectionIdWaitingForPrefetchResultCopy, false,
                    packageNamesForNextPrefetchResultCopy);
        } else if (DEBUG) {
            Log.w(LOG_TAG, "Prefetching for interaction with id " + interactionId + " dropped "
                    + infos.size() + " nodes");
        }
    }

    /**
     * Gets the result of a request to perform an accessibility action.
     *
+9 −0
Original line number Diff line number Diff line
@@ -46,6 +46,15 @@ oneway interface IAccessibilityInteractionConnectionCallback {
    void setFindAccessibilityNodeInfosResult(in List<AccessibilityNodeInfo> infos,
        int interactionId);

    /**
     * Sets the result of a prefetch request that returns {@link AccessibilityNodeInfo}s.
     *
     * @param root The {@link AccessibilityNodeInfo} for which the prefetching is based off of.
     * @param infos The result {@link AccessibilityNodeInfo}s.
     */
    void setPrefetchAccessibilityNodeInfoResult(
        in List<AccessibilityNodeInfo> infos, int interactionId);

    /**
     * Sets the result of a request to perform an accessibility action.
     *
+3 −6
Original line number Diff line number Diff line
@@ -33,9 +33,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;

import java.util.Arrays;
import java.util.List;

/**
 * Tests for AccessibilityInteractionClient
 */
@@ -65,7 +62,7 @@ public class AccessibilityInteractionClientTest {
        final long accessibilityNodeId = 0x4321L;
        AccessibilityNodeInfo nodeFromConnection = AccessibilityNodeInfo.obtain();
        nodeFromConnection.setSourceNodeId(accessibilityNodeId, windowId);
        mMockConnection.mInfosToReturn = Arrays.asList(nodeFromConnection);
        mMockConnection.mInfoToReturn = nodeFromConnection;
        AccessibilityInteractionClient client = AccessibilityInteractionClient.getInstance();
        AccessibilityNodeInfo node = client.findAccessibilityNodeInfoByAccessibilityId(
                MOCK_CONNECTION_ID, windowId, accessibilityNodeId, true, 0, null);
@@ -75,7 +72,7 @@ public class AccessibilityInteractionClientTest {
    }

    private static class MockConnection extends AccessibilityServiceConnectionImpl {
        List<AccessibilityNodeInfo> mInfosToReturn;
        AccessibilityNodeInfo mInfoToReturn;

        @Override
        public String[] findAccessibilityNodeInfoByAccessibilityId(int accessibilityWindowId,
@@ -83,7 +80,7 @@ public class AccessibilityInteractionClientTest {
                IAccessibilityInteractionConnectionCallback callback, int flags, long threadId,
                Bundle arguments) {
            try {
                callback.setFindAccessibilityNodeInfosResult(mInfosToReturn, interactionId);
                callback.setFindAccessibilityNodeInfoResult(mInfoToReturn, interactionId);
            } catch (RemoteException e) {
                throw new RuntimeException(e);
            }
+128 −93
Original line number Diff line number Diff line
@@ -40,29 +40,34 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection
    private final IAccessibilityInteractionConnectionCallback mServiceCallback;
    private final IAccessibilityInteractionConnection mConnectionWithReplacementActions;
    private final int mInteractionId;
    private final int mNodeWithReplacementActionsInteractionId;
    private final Object mLock = new Object();

    @GuardedBy("mLock")
    List<AccessibilityNodeInfo> mNodesWithReplacementActions;
    private boolean mReplacementNodeIsReadyOrFailed;

    @GuardedBy("mLock")
    AccessibilityNodeInfo mNodeWithReplacementActions;

    @GuardedBy("mLock")
    List<AccessibilityNodeInfo> mNodesFromOriginalWindow;

    @GuardedBy("mLock")
    boolean mSetFindNodeFromOriginalWindowCalled = false;

    @GuardedBy("mLock")
    AccessibilityNodeInfo mNodeFromOriginalWindow;

    // Keep track of whether or not we've been called back for a single node
    @GuardedBy("mLock")
    boolean mSingleNodeCallbackHappened;
    boolean mSetFindNodesFromOriginalWindowCalled = false;


    // Keep track of whether or not we've been called back for multiple node
    @GuardedBy("mLock")
    boolean mMultiNodeCallbackHappened;
    List<AccessibilityNodeInfo> mPrefetchedNodesFromOriginalWindow;

    // We shouldn't get any more callbacks after we've called back the original service, but
    // keep track to make sure we catch such strange things
    @GuardedBy("mLock")
    boolean mDone;
    boolean mSetPrefetchFromOriginalWindowCalled = false;


    public ActionReplacingCallback(IAccessibilityInteractionConnectionCallback serviceCallback,
            IAccessibilityInteractionConnection connectionWithReplacementActions,
@@ -70,19 +75,20 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection
        mServiceCallback = serviceCallback;
        mConnectionWithReplacementActions = connectionWithReplacementActions;
        mInteractionId = interactionId;
        mNodeWithReplacementActionsInteractionId = interactionId + 1;

        // Request the root node of the replacing window
        final long identityToken = Binder.clearCallingIdentity();
        try {
            mConnectionWithReplacementActions.findAccessibilityNodeInfoByAccessibilityId(
                    AccessibilityNodeInfo.ROOT_NODE_ID, null, interactionId + 1, this, 0,
                    AccessibilityNodeInfo.ROOT_NODE_ID, null,
                    mNodeWithReplacementActionsInteractionId, this, 0,
                    interrogatingPid, interrogatingTid, null, null);
        } catch (RemoteException re) {
            if (DEBUG) {
                Slog.e(LOG_TAG, "Error calling findAccessibilityNodeInfoByAccessibilityId()");
            }
            // Pretend we already got a (null) list of replacement nodes
            mMultiNodeCallbackHappened = true;
            mReplacementNodeIsReadyOrFailed = true;
        } finally {
            Binder.restoreCallingIdentity(identityToken);
        }
@@ -90,46 +96,67 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection

    @Override
    public void setFindAccessibilityNodeInfoResult(AccessibilityNodeInfo info, int interactionId) {
        boolean readyForCallback;
        synchronized (mLock) {
            if (interactionId == mInteractionId) {
                mNodeFromOriginalWindow = info;
                mSetFindNodeFromOriginalWindowCalled = true;
            } else if (interactionId == mNodeWithReplacementActionsInteractionId) {
                mNodeWithReplacementActions = info;
                mReplacementNodeIsReadyOrFailed = true;
            } else {
                Slog.e(LOG_TAG, "Callback with unexpected interactionId");
                return;
            }

            mSingleNodeCallbackHappened = true;
            readyForCallback = mMultiNodeCallbackHappened;
        }
        if (readyForCallback) {
            replaceInfoActionsAndCallService();
        }
        replaceInfoActionsAndCallServiceIfReady();
    }

    @Override
    public void setFindAccessibilityNodeInfosResult(List<AccessibilityNodeInfo> infos,
            int interactionId) {
        boolean callbackForSingleNode;
        boolean callbackForMultipleNodes;
        synchronized (mLock) {
            if (interactionId == mInteractionId) {
                mNodesFromOriginalWindow = infos;
            } else if (interactionId == mInteractionId + 1) {
                mNodesWithReplacementActions = infos;
                mSetFindNodesFromOriginalWindowCalled = true;
            } else if (interactionId == mNodeWithReplacementActionsInteractionId) {
                setNodeWithReplacementActionsFromList(infos);
                mReplacementNodeIsReadyOrFailed = true;
            } else {
                Slog.e(LOG_TAG, "Callback with unexpected interactionId");
                return;
            }
            callbackForSingleNode = mSingleNodeCallbackHappened;
            callbackForMultipleNodes = mMultiNodeCallbackHappened;
            mMultiNodeCallbackHappened = true;
        }
        if (callbackForSingleNode) {
            replaceInfoActionsAndCallService();
        replaceInfoActionsAndCallServiceIfReady();
    }
        if (callbackForMultipleNodes) {

    @Override
    public void setPrefetchAccessibilityNodeInfoResult(List<AccessibilityNodeInfo> infos,
                                                       int interactionId)
            throws RemoteException {
        synchronized (mLock) {
            if (interactionId == mInteractionId) {
                mPrefetchedNodesFromOriginalWindow = infos;
                mSetPrefetchFromOriginalWindowCalled = true;
            }  else {
                Slog.e(LOG_TAG, "Callback with unexpected interactionId");
                return;
            }
        }
        replaceInfoActionsAndCallServiceIfReady();
    }

    private void replaceInfoActionsAndCallServiceIfReady() {
        replaceInfoActionsAndCallService();
        replaceInfosActionsAndCallService();
        replacePrefetchInfosActionsAndCallService();
    }

    private void setNodeWithReplacementActionsFromList(List<AccessibilityNodeInfo> infos) {
        for (int i = 0; i < infos.size(); i++) {
            AccessibilityNodeInfo info = infos.get(i);
            if (info.getSourceNodeId() == AccessibilityNodeInfo.ROOT_NODE_ID) {
                mNodeWithReplacementActions = info;
            }
        }
    }

@@ -142,20 +169,17 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection

    private void replaceInfoActionsAndCallService() {
        final AccessibilityNodeInfo nodeToReturn;
        boolean doCallback = false;
        synchronized (mLock) {
            if (mDone) {
                if (DEBUG) {
                    Slog.e(LOG_TAG, "Extra callback");
                }
                return;
            }
            if (mNodeFromOriginalWindow != null) {
            doCallback = mReplacementNodeIsReadyOrFailed
                    && mSetFindNodeFromOriginalWindowCalled;
            if (doCallback && mNodeFromOriginalWindow != null) {
                replaceActionsOnInfoLocked(mNodeFromOriginalWindow);
                mSetFindNodeFromOriginalWindowCalled = false;
            }
            recycleReplaceActionNodesLocked();
            nodeToReturn = mNodeFromOriginalWindow;
            mDone = true;
        }
        if (doCallback) {
            try {
                mServiceCallback.setFindAccessibilityNodeInfoResult(nodeToReturn, mInteractionId);
            } catch (RemoteException re) {
@@ -164,34 +188,63 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection
                }
            }
        }
    }

    private void replaceInfosActionsAndCallService() {
        final List<AccessibilityNodeInfo> nodesToReturn;
        List<AccessibilityNodeInfo> nodesToReturn = null;
        boolean doCallback = false;
        synchronized (mLock) {
            if (mDone) {
            doCallback = mReplacementNodeIsReadyOrFailed
                    && mSetFindNodesFromOriginalWindowCalled;
            if (doCallback) {
                nodesToReturn = replaceActionsLocked(mNodesFromOriginalWindow);
                mSetFindNodesFromOriginalWindowCalled = false;
            }
        }
        if (doCallback) {
            try {
                mServiceCallback.setFindAccessibilityNodeInfosResult(nodesToReturn, mInteractionId);
            } catch (RemoteException re) {
                if (DEBUG) {
                    Slog.e(LOG_TAG, "Extra callback");
                    Slog.e(LOG_TAG, "Failed to setFindAccessibilityNodeInfosResult");
                }
                return;
            }
            if (mNodesFromOriginalWindow != null) {
                for (int i = 0; i < mNodesFromOriginalWindow.size(); i++) {
                    replaceActionsOnInfoLocked(mNodesFromOriginalWindow.get(i));
        }
    }
            recycleReplaceActionNodesLocked();
            nodesToReturn = (mNodesFromOriginalWindow == null)
                    ? null : new ArrayList<>(mNodesFromOriginalWindow);
            mDone = true;

    private void replacePrefetchInfosActionsAndCallService() {
        List<AccessibilityNodeInfo> nodesToReturn = null;
        boolean doCallback = false;
        synchronized (mLock) {
            doCallback = mReplacementNodeIsReadyOrFailed
                    && mSetPrefetchFromOriginalWindowCalled;
            if (doCallback) {
                nodesToReturn = replaceActionsLocked(mPrefetchedNodesFromOriginalWindow);
                mSetPrefetchFromOriginalWindowCalled = false;
            }
        }
        if (doCallback) {
            try {
            mServiceCallback.setFindAccessibilityNodeInfosResult(nodesToReturn, mInteractionId);
                mServiceCallback.setPrefetchAccessibilityNodeInfoResult(
                        nodesToReturn, mInteractionId);
            } catch (RemoteException re) {
                if (DEBUG) {
                    Slog.e(LOG_TAG, "Failed to setFindAccessibilityNodeInfosResult");
                }
            }
        }
    }

    @GuardedBy("mLock")
    private List<AccessibilityNodeInfo> replaceActionsLocked(List<AccessibilityNodeInfo> infos) {
        if (infos != null) {
            for (int i = 0; i < infos.size(); i++) {
                replaceActionsOnInfoLocked(infos.get(i));
            }
        }
        return (infos == null)
                ? null : new ArrayList<>(infos);
    }

    @GuardedBy("mLock")
    private void replaceActionsOnInfoLocked(AccessibilityNodeInfo info) {
@@ -204,14 +257,8 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection
        info.setDismissable(false);
        // We currently only replace actions for the root node
        if ((info.getSourceNodeId() == AccessibilityNodeInfo.ROOT_NODE_ID)
                && mNodesWithReplacementActions != null) {
            // This list should always contain a single node with the root ID
            for (int i = 0; i < mNodesWithReplacementActions.size(); i++) {
                AccessibilityNodeInfo nodeWithReplacementActions =
                        mNodesWithReplacementActions.get(i);
                if (nodeWithReplacementActions.getSourceNodeId()
                        == AccessibilityNodeInfo.ROOT_NODE_ID) {
                    List<AccessibilityAction> actions = nodeWithReplacementActions.getActionList();
                && mNodeWithReplacementActions != null) {
            List<AccessibilityAction> actions = mNodeWithReplacementActions.getActionList();
            if (actions != null) {
                for (int j = 0; j < actions.size(); j++) {
                    info.addAction(actions.get(j));
@@ -220,24 +267,12 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection
                info.addAction(AccessibilityAction.ACTION_ACCESSIBILITY_FOCUS);
                info.addAction(AccessibilityAction.ACTION_CLEAR_ACCESSIBILITY_FOCUS);
            }
                    info.setClickable(nodeWithReplacementActions.isClickable());
                    info.setFocusable(nodeWithReplacementActions.isFocusable());
                    info.setContextClickable(nodeWithReplacementActions.isContextClickable());
                    info.setScrollable(nodeWithReplacementActions.isScrollable());
                    info.setLongClickable(nodeWithReplacementActions.isLongClickable());
                    info.setDismissable(nodeWithReplacementActions.isDismissable());
                }
            }
        }
    }

    @GuardedBy("mLock")
    private void recycleReplaceActionNodesLocked() {
        if (mNodesWithReplacementActions == null) return;
        for (int i = mNodesWithReplacementActions.size() - 1; i >= 0; i--) {
            AccessibilityNodeInfo nodeWithReplacementAction = mNodesWithReplacementActions.get(i);
            nodeWithReplacementAction.recycle();
            info.setClickable(mNodeWithReplacementActions.isClickable());
            info.setFocusable(mNodeWithReplacementActions.isFocusable());
            info.setContextClickable(mNodeWithReplacementActions.isContextClickable());
            info.setScrollable(mNodeWithReplacementActions.isScrollable());
            info.setLongClickable(mNodeWithReplacementActions.isLongClickable());
            info.setDismissable(mNodeWithReplacementActions.isDismissable());
        }
        mNodesWithReplacementActions = null;
    }
}
Loading