From c3060eae38b3959da5cd464067ef9ab2086b063b Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 22 Dec 2020 16:06:09 +0000 Subject: [PATCH] Revert "Prefetching can be interupted by other service requests." This reverts commit 103dd32a74f9962e41499b4a39b15b32e963ce65. Reason for revert: Looking for culprits for b/176078134 Change-Id: I1e2524411fcc91a2c212d4ee5a0f5b74cac0bda4 --- .../AccessibilityInteractionController.java | 282 ++++++++---------- .../AccessibilityInteractionClient.java | 56 +--- ...sibilityInteractionConnectionCallback.aidl | 9 - .../AccessibilityInteractionClientTest.java | 9 +- .../ActionReplacingCallback.java | 185 ++++++------ 5 files changed, 221 insertions(+), 320 deletions(-) diff --git a/core/java/android/view/AccessibilityInteractionController.java b/core/java/android/view/AccessibilityInteractionController.java index 2e9061935c98f..9473845b15e6b 100644 --- a/core/java/android/view/AccessibilityInteractionController.java +++ b/core/java/android/view/AccessibilityInteractionController.java @@ -113,8 +113,6 @@ public final class AccessibilityInteractionController { private AddNodeInfosForViewId mAddNodeInfosForViewId; - private List mPendingFindNodeByIdMessages; - @GuardedBy("mLock") private int mNumActiveRequestPreparers; @GuardedBy("mLock") @@ -130,7 +128,6 @@ public final class AccessibilityInteractionController { mViewRootImpl = viewRootImpl; mPrefetcher = new AccessibilityNodePrefetcher(); mA11yManager = mViewRootImpl.mContext.getSystemService(AccessibilityManager.class); - mPendingFindNodeByIdMessages = new ArrayList<>(); } private void scheduleMessage(Message message, int interrogatingPid, long interrogatingTid, @@ -180,7 +177,6 @@ public final class AccessibilityInteractionController { args.arg4 = arguments; message.obj = args; - mPendingFindNodeByIdMessages.add(message); scheduleMessage(message, interrogatingPid, interrogatingTid, CONSIDER_REQUEST_PREPARERS); } @@ -319,8 +315,6 @@ public final class AccessibilityInteractionController { } private void findAccessibilityNodeInfoByAccessibilityIdUiThread(Message message) { - mPendingFindNodeByIdMessages.remove(message); - final int flags = message.arg1; SomeArgs args = (SomeArgs) message.obj; @@ -335,58 +329,22 @@ public final class AccessibilityInteractionController { args.recycle(); - View rootView = null; - AccessibilityNodeInfo rootNode = null; + List infos = mTempAccessibilityNodeInfoList; + infos.clear(); try { if (mViewRootImpl.mView == null || mViewRootImpl.mAttachInfo == null) { return; } mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = flags; - rootView = findViewByAccessibilityId(accessibilityViewId); - if (rootView != null && isShown(rootView)) { - rootNode = populateAccessibilityNodeInfoForView( - rootView, arguments, virtualDescendantId); + final View root = findViewByAccessibilityId(accessibilityViewId); + if (root != null && isShown(root)) { + mPrefetcher.prefetchAccessibilityNodeInfos( + root, virtualDescendantId, flags, infos, arguments); } } finally { - updateInfoForViewportAndReturnFindNodeResult( - rootNode == null ? null : AccessibilityNodeInfo.obtain(rootNode), - callback, interactionId, spec, interactiveRegion); + updateInfosForViewportAndReturnFindNodeResult( + infos, callback, interactionId, spec, interactiveRegion); } - List infos = mTempAccessibilityNodeInfoList; - infos.clear(); - mPrefetcher.prefetchAccessibilityNodeInfos( - rootView, rootNode == null ? null : AccessibilityNodeInfo.obtain(rootNode), - virtualDescendantId, flags, infos); - mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; - updateInfosForViewPort(infos, spec, interactiveRegion); - returnPrefetchResult(interactionId, infos, callback); - returnPendingFindAccessibilityNodeInfosInPrefetch(infos); - } - - private AccessibilityNodeInfo populateAccessibilityNodeInfoForView( - View view, Bundle arguments, int virtualViewId) { - AccessibilityNodeProvider provider = view.getAccessibilityNodeProvider(); - // Determine if we'll be populating extra data - final String extraDataRequested = (arguments == null) ? null - : arguments.getString(EXTRA_DATA_REQUESTED_KEY); - AccessibilityNodeInfo root = null; - if (provider == null) { - root = view.createAccessibilityNodeInfo(); - if (root != null) { - if (extraDataRequested != null) { - view.addExtraDataToAccessibilityNodeInfo(root, extraDataRequested, arguments); - } - } - } else { - root = provider.createAccessibilityNodeInfo(virtualViewId); - if (root != null) { - if (extraDataRequested != null) { - provider.addExtraDataToAccessibilityNodeInfo( - virtualViewId, root, extraDataRequested, arguments); - } - } - } - return root; } public void findAccessibilityNodeInfosByViewIdClientThread(long accessibilityNodeId, @@ -444,7 +402,6 @@ public final class AccessibilityInteractionController { mAddNodeInfosForViewId.reset(); } } finally { - mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; updateInfosForViewportAndReturnFindNodeResult( infos, callback, interactionId, spec, interactiveRegion); } @@ -527,7 +484,6 @@ public final class AccessibilityInteractionController { } } } finally { - mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; updateInfosForViewportAndReturnFindNodeResult( infos, callback, interactionId, spec, interactiveRegion); } @@ -619,7 +575,6 @@ public final class AccessibilityInteractionController { } } } finally { - mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; updateInfoForViewportAndReturnFindNodeResult( focused, callback, interactionId, spec, interactiveRegion); } @@ -674,7 +629,6 @@ public final class AccessibilityInteractionController { } } } finally { - mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; updateInfoForViewportAndReturnFindNodeResult( next, callback, interactionId, spec, interactiveRegion); } @@ -831,6 +785,33 @@ public final class AccessibilityInteractionController { } } + private void applyAppScaleAndMagnificationSpecIfNeeded(List infos, + MagnificationSpec spec) { + if (infos == null) { + return; + } + final float applicationScale = mViewRootImpl.mAttachInfo.mApplicationScale; + if (shouldApplyAppScaleAndMagnificationSpec(applicationScale, spec)) { + final int infoCount = infos.size(); + for (int i = 0; i < infoCount; i++) { + AccessibilityNodeInfo info = infos.get(i); + applyAppScaleAndMagnificationSpecIfNeeded(info, spec); + } + } + } + + private void adjustIsVisibleToUserIfNeeded(List infos, + Region interactiveRegion) { + if (interactiveRegion == null || infos == null) { + return; + } + final int infoCount = infos.size(); + for (int i = 0; i < infoCount; i++) { + AccessibilityNodeInfo info = infos.get(i); + adjustIsVisibleToUserIfNeeded(info, interactiveRegion); + } + } + private void adjustIsVisibleToUserIfNeeded(AccessibilityNodeInfo info, Region interactiveRegion) { if (interactiveRegion == null || info == null) { @@ -851,6 +832,17 @@ public final class AccessibilityInteractionController { return false; } + private void adjustBoundsInScreenIfNeeded(List infos) { + if (infos == null || shouldBypassAdjustBoundsInScreen()) { + return; + } + final int infoCount = infos.size(); + for (int i = 0; i < infoCount; i++) { + final AccessibilityNodeInfo info = infos.get(i); + adjustBoundsInScreenIfNeeded(info); + } + } + private void adjustBoundsInScreenIfNeeded(AccessibilityNodeInfo info) { if (info == null || shouldBypassAdjustBoundsInScreen()) { return; @@ -898,6 +890,17 @@ public final class AccessibilityInteractionController { return screenMatrix == null || screenMatrix.isIdentity(); } + private void associateLeashedParentIfNeeded(List infos) { + if (infos == null || shouldBypassAssociateLeashedParent()) { + return; + } + final int infoCount = infos.size(); + for (int i = 0; i < infoCount; i++) { + final AccessibilityNodeInfo info = infos.get(i); + associateLeashedParentIfNeeded(info); + } + } + private void associateLeashedParentIfNeeded(AccessibilityNodeInfo info) { if (info == null || shouldBypassAssociateLeashedParent()) { return; @@ -971,46 +974,18 @@ public final class AccessibilityInteractionController { return (appScale != 1.0f || (spec != null && !spec.isNop())); } - private void updateInfosForViewPort(List infos, MagnificationSpec spec, - Region interactiveRegion) { - for (int i = 0; i < infos.size(); i++) { - updateInfoForViewPort(infos.get(i), spec, interactiveRegion); - } - } - - private void updateInfoForViewPort(AccessibilityNodeInfo info, MagnificationSpec spec, - Region interactiveRegion) { - associateLeashedParentIfNeeded(info); - applyScreenMatrixIfNeeded(info); - adjustBoundsInScreenIfNeeded(info); - // To avoid applyAppScaleAndMagnificationSpecIfNeeded changing the bounds of node, - // then impact the visibility result, we need to adjust visibility before apply scale. - adjustIsVisibleToUserIfNeeded(info, interactiveRegion); - applyAppScaleAndMagnificationSpecIfNeeded(info, spec); - } - private void updateInfosForViewportAndReturnFindNodeResult(List infos, IAccessibilityInteractionConnectionCallback callback, int interactionId, MagnificationSpec spec, Region interactiveRegion) { - if (infos != null) { - updateInfosForViewPort(infos, spec, interactiveRegion); - } - returnFindNodesResult(infos, callback, interactionId); - } - - private void returnFindNodeResult(AccessibilityNodeInfo info, - IAccessibilityInteractionConnectionCallback callback, - int interactionId) { - try { - callback.setFindAccessibilityNodeInfoResult(info, interactionId); - } catch (RemoteException re) { - /* ignore - the other side will time out */ - } - } - - private void returnFindNodesResult(List infos, - IAccessibilityInteractionConnectionCallback callback, int interactionId) { try { + mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; + associateLeashedParentIfNeeded(infos); + applyScreenMatrixIfNeeded(infos); + adjustBoundsInScreenIfNeeded(infos); + // To avoid applyAppScaleAndMagnificationSpecIfNeeded changing the bounds of node, + // then impact the visibility result, we need to adjust visibility before apply scale. + adjustIsVisibleToUserIfNeeded(infos, interactiveRegion); + applyAppScaleAndMagnificationSpecIfNeeded(infos, spec); callback.setFindAccessibilityNodeInfosResult(infos, interactionId); if (infos != null) { infos.clear(); @@ -1020,49 +995,22 @@ public final class AccessibilityInteractionController { } } - private void returnPendingFindAccessibilityNodeInfosInPrefetch( - List infos) { - for (Message pendingMessage : mPendingFindNodeByIdMessages) { - SomeArgs args = (SomeArgs) pendingMessage.obj; - final int accessibilityViewId = args.argi1; - final int virtualDescendantId = args.argi2; - final int interactionId = args.argi3; - final IAccessibilityInteractionConnectionCallback callback = - (IAccessibilityInteractionConnectionCallback) args.arg1; - final long nodeId = - AccessibilityNodeInfo.makeNodeId(accessibilityViewId, virtualDescendantId); - for (int i = 0; i < infos.size(); i++) { - AccessibilityNodeInfo info = infos.get(i); - if (info.getSourceNodeId() == nodeId) { - returnFindNodeResult( - AccessibilityNodeInfo.obtain(info), callback, interactionId); - mHandler.removeMessages( - PrivateHandler.MSG_FIND_ACCESSIBILITY_NODE_INFO_BY_ACCESSIBILITY_ID, - pendingMessage.obj); - args.recycle(); - break; - } - } - } - mPendingFindNodeByIdMessages.clear(); - } - - private void returnPrefetchResult(int interactionId, List infos, - IAccessibilityInteractionConnectionCallback callback) { - if (infos.size() > 0) { - try { - callback.setPrefetchAccessibilityNodeInfoResult(infos, interactionId); - } catch (RemoteException re) { - /* ignore - other side isn't too bothered if this doesn't arrive */ - } - } - } - private void updateInfoForViewportAndReturnFindNodeResult(AccessibilityNodeInfo info, IAccessibilityInteractionConnectionCallback callback, int interactionId, MagnificationSpec spec, Region interactiveRegion) { - updateInfoForViewPort(info, spec, interactiveRegion); - returnFindNodeResult(info, callback, interactionId); + try { + mViewRootImpl.mAttachInfo.mAccessibilityFetchFlags = 0; + associateLeashedParentIfNeeded(info); + applyScreenMatrixIfNeeded(info); + adjustBoundsInScreenIfNeeded(info); + // To avoid applyAppScaleAndMagnificationSpecIfNeeded changing the bounds of node, + // then impact the visibility result, we need to adjust visibility before apply scale. + adjustIsVisibleToUserIfNeeded(info, interactiveRegion); + applyAppScaleAndMagnificationSpecIfNeeded(info, spec); + callback.setFindAccessibilityNodeInfoResult(info, interactionId); + } catch (RemoteException re) { + /* ignore - the other side will time out */ + } } private boolean handleClickableSpanActionUiThread( @@ -1105,11 +1053,20 @@ public final class AccessibilityInteractionController { private final ArrayList mTempViewList = new ArrayList(); - public void prefetchAccessibilityNodeInfos(View view, AccessibilityNodeInfo root, - int virtualViewId, int fetchFlags, List outInfos) { - if (root != null) { - AccessibilityNodeProvider provider = view.getAccessibilityNodeProvider(); - if (provider == null) { + public void prefetchAccessibilityNodeInfos(View view, int virtualViewId, int fetchFlags, + List outInfos, Bundle arguments) { + AccessibilityNodeProvider provider = view.getAccessibilityNodeProvider(); + // Determine if we'll be populating extra data + final String extraDataRequested = (arguments == null) ? null + : arguments.getString(EXTRA_DATA_REQUESTED_KEY); + if (provider == null) { + AccessibilityNodeInfo root = view.createAccessibilityNodeInfo(); + if (root != null) { + if (extraDataRequested != null) { + view.addExtraDataToAccessibilityNodeInfo( + root, extraDataRequested, arguments); + } + outInfos.add(root); if ((fetchFlags & AccessibilityNodeInfo.FLAG_PREFETCH_PREDECESSORS) != 0) { prefetchPredecessorsOfRealNode(view, outInfos); } @@ -1119,7 +1076,16 @@ public final class AccessibilityInteractionController { if ((fetchFlags & AccessibilityNodeInfo.FLAG_PREFETCH_DESCENDANTS) != 0) { prefetchDescendantsOfRealNode(view, outInfos); } - } else { + } + } else { + final AccessibilityNodeInfo root = + provider.createAccessibilityNodeInfo(virtualViewId); + if (root != null) { + if (extraDataRequested != null) { + provider.addExtraDataToAccessibilityNodeInfo( + virtualViewId, root, extraDataRequested, arguments); + } + outInfos.add(root); if ((fetchFlags & AccessibilityNodeInfo.FLAG_PREFETCH_PREDECESSORS) != 0) { prefetchPredecessorsOfVirtualNode(root, view, provider, outInfos); } @@ -1130,19 +1096,13 @@ public final class AccessibilityInteractionController { prefetchDescendantsOfVirtualNode(root, provider, outInfos); } } - if (ENFORCE_NODE_TREE_CONSISTENT) { - enforceNodeTreeConsistent(root, outInfos); - } + } + if (ENFORCE_NODE_TREE_CONSISTENT) { + enforceNodeTreeConsistent(outInfos); } } - private boolean shouldStopPrefetching(List prefetchededInfos) { - return mHandler.hasUserInteractiveMessagesWaiting() - || prefetchededInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE; - } - - private void enforceNodeTreeConsistent( - AccessibilityNodeInfo root, List nodes) { + private void enforceNodeTreeConsistent(List nodes) { LongSparseArray nodeMap = new LongSparseArray(); final int nodeCount = nodes.size(); @@ -1153,6 +1113,7 @@ public final class AccessibilityInteractionController { // If the nodes are a tree it does not matter from // which node we start to search for the root. + AccessibilityNodeInfo root = nodeMap.valueAt(0); AccessibilityNodeInfo parent = root; while (parent != null) { root = parent; @@ -1219,11 +1180,9 @@ public final class AccessibilityInteractionController { private void prefetchPredecessorsOfRealNode(View view, List outInfos) { - if (shouldStopPrefetching(outInfos)) { - return; - } ViewParent parent = view.getParentForAccessibility(); - while (parent instanceof View && !shouldStopPrefetching(outInfos)) { + while (parent instanceof View + && outInfos.size() < MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { View parentView = (View) parent; AccessibilityNodeInfo info = parentView.createAccessibilityNodeInfo(); if (info != null) { @@ -1235,9 +1194,6 @@ public final class AccessibilityInteractionController { private void prefetchSiblingsOfRealNode(View current, List outInfos) { - if (shouldStopPrefetching(outInfos)) { - return; - } ViewParent parent = current.getParentForAccessibility(); if (parent instanceof ViewGroup) { ViewGroup parentGroup = (ViewGroup) parent; @@ -1247,7 +1203,7 @@ public final class AccessibilityInteractionController { parentGroup.addChildrenForAccessibility(children); final int childCount = children.size(); for (int i = 0; i < childCount; i++) { - if (shouldStopPrefetching(outInfos)) { + if (outInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { return; } View child = children.get(i); @@ -1275,7 +1231,7 @@ public final class AccessibilityInteractionController { private void prefetchDescendantsOfRealNode(View root, List outInfos) { - if (shouldStopPrefetching(outInfos) || !(root instanceof ViewGroup)) { + if (!(root instanceof ViewGroup)) { return; } HashMap addedChildren = @@ -1286,7 +1242,7 @@ public final class AccessibilityInteractionController { root.addChildrenForAccessibility(children); final int childCount = children.size(); for (int i = 0; i < childCount; i++) { - if (shouldStopPrefetching(outInfos)) { + if (outInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { return; } View child = children.get(i); @@ -1311,7 +1267,7 @@ public final class AccessibilityInteractionController { } finally { children.clear(); } - if (!shouldStopPrefetching(outInfos)) { + if (outInfos.size() < MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { for (Map.Entry entry : addedChildren.entrySet()) { View addedChild = entry.getKey(); AccessibilityNodeInfo virtualRoot = entry.getValue(); @@ -1333,7 +1289,7 @@ public final class AccessibilityInteractionController { long parentNodeId = root.getParentNodeId(); int accessibilityViewId = AccessibilityNodeInfo.getAccessibilityViewId(parentNodeId); while (accessibilityViewId != AccessibilityNodeInfo.UNDEFINED_ITEM_ID) { - if (shouldStopPrefetching(outInfos)) { + if (outInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { return; } final int virtualDescendantId = @@ -1378,7 +1334,7 @@ public final class AccessibilityInteractionController { if (parent != null) { final int childCount = parent.getChildCount(); for (int i = 0; i < childCount; i++) { - if (shouldStopPrefetching(outInfos)) { + if (outInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { return; } final long childNodeId = parent.getChildId(i); @@ -1403,7 +1359,7 @@ public final class AccessibilityInteractionController { final int initialOutInfosSize = outInfos.size(); final int childCount = root.getChildCount(); for (int i = 0; i < childCount; i++) { - if (shouldStopPrefetching(outInfos)) { + if (outInfos.size() >= MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { return; } final long childNodeId = root.getChildId(i); @@ -1413,7 +1369,7 @@ public final class AccessibilityInteractionController { outInfos.add(child); } } - if (!shouldStopPrefetching(outInfos)) { + if (outInfos.size() < MAX_ACCESSIBILITY_NODE_INFO_BATCH_SIZE) { final int addedChildCount = outInfos.size() - initialOutInfosSize; for (int i = 0; i < addedChildCount; i++) { AccessibilityNodeInfo child = outInfos.get(initialOutInfosSize + i); @@ -1522,10 +1478,6 @@ public final class AccessibilityInteractionController { boolean hasAccessibilityCallback(Message message) { return message.what < FIRST_NO_ACCESSIBILITY_CALLBACK_MSG ? true : false; } - - boolean hasUserInteractiveMessagesWaiting() { - return hasMessagesOrCallbacks(); - } } private final class AddNodeInfosForViewId implements Predicate { diff --git a/core/java/android/view/accessibility/AccessibilityInteractionClient.java b/core/java/android/view/accessibility/AccessibilityInteractionClient.java index aff0b35305ea8..f63749be6df26 100644 --- a/core/java/android/view/accessibility/AccessibilityInteractionClient.java +++ b/core/java/android/view/accessibility/AccessibilityInteractionClient.java @@ -23,9 +23,7 @@ import android.compat.annotation.UnsupportedAppUsage; import android.os.Binder; import android.os.Build; import android.os.Bundle; -import android.os.Handler; import android.os.IBinder; -import android.os.Looper; import android.os.Message; import android.os.Process; import android.os.RemoteException; @@ -125,12 +123,6 @@ public final class AccessibilityInteractionClient private Message mSameThreadMessage; - private int mInteractionIdWaitingForPrefetchResult; - private int mConnectionIdWaitingForPrefetchResult; - private String[] mPackageNamesForNextPrefetchResult; - private final Handler mMainHandler; - private Runnable mPrefetchResultRunnable; - /** * @return The client for the current thread. */ @@ -205,7 +197,6 @@ public final class AccessibilityInteractionClient private AccessibilityInteractionClient() { /* reducing constructor visibility */ - mMainHandler = new Handler(Looper.getMainLooper()); } /** @@ -460,16 +451,16 @@ public final class AccessibilityInteractionClient Binder.restoreCallingIdentity(identityToken); } if (packageNames != null) { - AccessibilityNodeInfo info = - getFindAccessibilityNodeInfoResultAndClear(interactionId); - if ((prefetchFlags & AccessibilityNodeInfo.FLAG_PREFETCH_MASK) != 0 - && info != null) { - setInteractionWaitingForPrefetchResult(interactionId, connectionId, - packageNames); - } - finalizeAndCacheAccessibilityNodeInfo(info, connectionId, + List infos = getFindAccessibilityNodeInfosResultAndClear( + interactionId); + finalizeAndCacheAccessibilityNodeInfos(infos, connectionId, bypassCache, packageNames); - return info; + if (infos != null && !infos.isEmpty()) { + for (int i = 1; i < infos.size(); i++) { + infos.get(i).recycle(); + } + return infos.get(0); + } } } else { if (DEBUG) { @@ -483,15 +474,6 @@ 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); @@ -846,26 +828,6 @@ public final class AccessibilityInteractionClient } } - /** - * {@inheritDoc} - */ - @Override - public void setPrefetchAccessibilityNodeInfoResult(List infos, - int interactionId) { - synchronized (mInstanceLock) { - if (mPrefetchResultRunnable != null) { - mMainHandler.removeCallbacks(mPrefetchResultRunnable); - mPrefetchResultRunnable = null; - } - if (!infos.isEmpty() && mInteractionIdWaitingForPrefetchResult == interactionId) { - mPrefetchResultRunnable = () -> finalizeAndCacheAccessibilityNodeInfos( - infos, mConnectionIdWaitingForPrefetchResult, false, - mPackageNamesForNextPrefetchResult); - mMainHandler.post(mPrefetchResultRunnable); - } - } - } - /** * Gets the result of a request to perform an accessibility action. * diff --git a/core/java/android/view/accessibility/IAccessibilityInteractionConnectionCallback.aidl b/core/java/android/view/accessibility/IAccessibilityInteractionConnectionCallback.aidl index 231e75a19a068..049bb31adbb11 100644 --- a/core/java/android/view/accessibility/IAccessibilityInteractionConnectionCallback.aidl +++ b/core/java/android/view/accessibility/IAccessibilityInteractionConnectionCallback.aidl @@ -46,15 +46,6 @@ oneway interface IAccessibilityInteractionConnectionCallback { void setFindAccessibilityNodeInfosResult(in List 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 infos, int interactionId); - /** * Sets the result of a request to perform an accessibility action. * diff --git a/core/tests/coretests/src/android/view/accessibility/AccessibilityInteractionClientTest.java b/core/tests/coretests/src/android/view/accessibility/AccessibilityInteractionClientTest.java index 7e1e7f4bdd7fb..ab24f89015c79 100644 --- a/core/tests/coretests/src/android/view/accessibility/AccessibilityInteractionClientTest.java +++ b/core/tests/coretests/src/android/view/accessibility/AccessibilityInteractionClientTest.java @@ -33,6 +33,9 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +import java.util.Arrays; +import java.util.List; + /** * Tests for AccessibilityInteractionClient */ @@ -62,7 +65,7 @@ public class AccessibilityInteractionClientTest { final long accessibilityNodeId = 0x4321L; AccessibilityNodeInfo nodeFromConnection = AccessibilityNodeInfo.obtain(); nodeFromConnection.setSourceNodeId(accessibilityNodeId, windowId); - mMockConnection.mInfoToReturn = nodeFromConnection; + mMockConnection.mInfosToReturn = Arrays.asList(nodeFromConnection); AccessibilityInteractionClient client = AccessibilityInteractionClient.getInstance(); AccessibilityNodeInfo node = client.findAccessibilityNodeInfoByAccessibilityId( MOCK_CONNECTION_ID, windowId, accessibilityNodeId, true, 0, null); @@ -72,7 +75,7 @@ public class AccessibilityInteractionClientTest { } private static class MockConnection extends AccessibilityServiceConnectionImpl { - AccessibilityNodeInfo mInfoToReturn; + List mInfosToReturn; @Override public String[] findAccessibilityNodeInfoByAccessibilityId(int accessibilityWindowId, @@ -80,7 +83,7 @@ public class AccessibilityInteractionClientTest { IAccessibilityInteractionConnectionCallback callback, int flags, long threadId, Bundle arguments) { try { - callback.setFindAccessibilityNodeInfoResult(mInfoToReturn, interactionId); + callback.setFindAccessibilityNodeInfosResult(mInfosToReturn, interactionId); } catch (RemoteException e) { throw new RuntimeException(e); } diff --git a/services/accessibility/java/com/android/server/accessibility/ActionReplacingCallback.java b/services/accessibility/java/com/android/server/accessibility/ActionReplacingCallback.java index a2c7e4f394686..bafb641dcc9ed 100644 --- a/services/accessibility/java/com/android/server/accessibility/ActionReplacingCallback.java +++ b/services/accessibility/java/com/android/server/accessibility/ActionReplacingCallback.java @@ -40,14 +40,10 @@ 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") - private boolean mRequestForNodeWithReplacementActionFailed; - - @GuardedBy("mLock") - AccessibilityNodeInfo mNodeWithReplacementActions; + List mNodesWithReplacementActions; @GuardedBy("mLock") List mNodesFromOriginalWindow; @@ -55,8 +51,18 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection @GuardedBy("mLock") AccessibilityNodeInfo mNodeFromOriginalWindow; + // Keep track of whether or not we've been called back for a single node @GuardedBy("mLock") - List mPrefetchedNodesFromOriginalWindow; + boolean mSingleNodeCallbackHappened; + + // Keep track of whether or not we've been called back for multiple node + @GuardedBy("mLock") + boolean mMultiNodeCallbackHappened; + + // 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; public ActionReplacingCallback(IAccessibilityInteractionConnectionCallback serviceCallback, IAccessibilityInteractionConnection connectionWithReplacementActions, @@ -64,20 +70,19 @@ 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, - mNodeWithReplacementActionsInteractionId, this, 0, + AccessibilityNodeInfo.ROOT_NODE_ID, null, interactionId + 1, this, 0, interrogatingPid, interrogatingTid, null, null); } catch (RemoteException re) { if (DEBUG) { Slog.e(LOG_TAG, "Error calling findAccessibilityNodeInfoByAccessibilityId()"); } - mRequestForNodeWithReplacementActionFailed = true; + // Pretend we already got a (null) list of replacement nodes + mMultiNodeCallbackHappened = true; } finally { Binder.restoreCallingIdentity(identityToken); } @@ -85,73 +90,46 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection @Override public void setFindAccessibilityNodeInfoResult(AccessibilityNodeInfo info, int interactionId) { - synchronized (mLock) { + boolean readyForCallback; + synchronized(mLock) { if (interactionId == mInteractionId) { mNodeFromOriginalWindow = info; - } else if (interactionId == mNodeWithReplacementActionsInteractionId) { - mNodeWithReplacementActions = info; } else { Slog.e(LOG_TAG, "Callback with unexpected interactionId"); return; } + + mSingleNodeCallbackHappened = true; + readyForCallback = mMultiNodeCallbackHappened; + } + if (readyForCallback) { + replaceInfoActionsAndCallService(); } - replaceInfoActionsAndCallServiceIfReady(); } @Override public void setFindAccessibilityNodeInfosResult(List infos, int interactionId) { - synchronized (mLock) { + boolean callbackForSingleNode; + boolean callbackForMultipleNodes; + synchronized(mLock) { if (interactionId == mInteractionId) { mNodesFromOriginalWindow = infos; - } else if (interactionId == mNodeWithReplacementActionsInteractionId) { - setNodeWithReplacementActionsFromList(infos); + } else if (interactionId == mInteractionId + 1) { + mNodesWithReplacementActions = infos; } else { Slog.e(LOG_TAG, "Callback with unexpected interactionId"); return; } + callbackForSingleNode = mSingleNodeCallbackHappened; + callbackForMultipleNodes = mMultiNodeCallbackHappened; + mMultiNodeCallbackHappened = true; } - replaceInfoActionsAndCallServiceIfReady(); - } - - @Override - public void setPrefetchAccessibilityNodeInfoResult(List infos, - int interactionId) - throws RemoteException { - synchronized (mLock) { - if (interactionId == mInteractionId) { - mPrefetchedNodesFromOriginalWindow = infos; - } else { - Slog.e(LOG_TAG, "Callback with unexpected interactionId"); - return; - } - } - replaceInfoActionsAndCallServiceIfReady(); - } - - private void replaceInfoActionsAndCallServiceIfReady() { - boolean originalAndReplacementCallsHaveHappened = false; - synchronized (mLock) { - originalAndReplacementCallsHaveHappened = mNodeWithReplacementActions != null - && (mNodeFromOriginalWindow != null - || mNodesFromOriginalWindow != null - || mPrefetchedNodesFromOriginalWindow != null); - originalAndReplacementCallsHaveHappened - |= mRequestForNodeWithReplacementActionFailed; - } - if (originalAndReplacementCallsHaveHappened) { + if (callbackForSingleNode) { replaceInfoActionsAndCallService(); - replaceInfosActionsAndCallService(); - replacePrefetchInfosActionsAndCallService(); } - } - - private void setNodeWithReplacementActionsFromList(List infos) { - for (int i = 0; i < infos.size(); i++) { - AccessibilityNodeInfo info = infos.get(i); - if (info.getSourceNodeId() == AccessibilityNodeInfo.ROOT_NODE_ID) { - mNodeWithReplacementActions = info; - } + if (callbackForMultipleNodes) { + replaceInfosActionsAndCallService(); } } @@ -165,10 +143,18 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection private void replaceInfoActionsAndCallService() { final AccessibilityNodeInfo nodeToReturn; synchronized (mLock) { + if (mDone) { + if (DEBUG) { + Slog.e(LOG_TAG, "Extra callback"); + } + return; + } if (mNodeFromOriginalWindow != null) { replaceActionsOnInfoLocked(mNodeFromOriginalWindow); } + recycleReplaceActionNodesLocked(); nodeToReturn = mNodeFromOriginalWindow; + mDone = true; } try { mServiceCallback.setFindAccessibilityNodeInfoResult(nodeToReturn, mInteractionId); @@ -182,7 +168,21 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection private void replaceInfosActionsAndCallService() { final List nodesToReturn; synchronized (mLock) { - nodesToReturn = replaceActionsLocked(mNodesFromOriginalWindow); + if (mDone) { + if (DEBUG) { + Slog.e(LOG_TAG, "Extra callback"); + } + 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; } try { mServiceCallback.setFindAccessibilityNodeInfosResult(nodesToReturn, mInteractionId); @@ -193,31 +193,6 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection } } - private void replacePrefetchInfosActionsAndCallService() { - final List nodesToReturn; - synchronized (mLock) { - nodesToReturn = replaceActionsLocked(mPrefetchedNodesFromOriginalWindow); - } - try { - mServiceCallback.setPrefetchAccessibilityNodeInfoResult(nodesToReturn, mInteractionId); - } catch (RemoteException re) { - if (DEBUG) { - Slog.e(LOG_TAG, "Failed to setFindAccessibilityNodeInfosResult"); - } - } - } - - @GuardedBy("mLock") - private List replaceActionsLocked(List 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) { info.removeAllActions(); @@ -229,22 +204,40 @@ public class ActionReplacingCallback extends IAccessibilityInteractionConnection info.setDismissable(false); // We currently only replace actions for the root node if ((info.getSourceNodeId() == AccessibilityNodeInfo.ROOT_NODE_ID) - && mNodeWithReplacementActions != null) { - List actions = mNodeWithReplacementActions.getActionList(); - if (actions != null) { - for (int j = 0; j < actions.size(); j++) { - info.addAction(actions.get(j)); + && 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 actions = nodeWithReplacementActions.getActionList(); + if (actions != null) { + for (int j = 0; j < actions.size(); j++) { + info.addAction(actions.get(j)); + } + // The PIP needs to be able to take accessibility focus + 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()); } - // The PIP needs to be able to take accessibility focus - info.addAction(AccessibilityAction.ACTION_ACCESSIBILITY_FOCUS); - info.addAction(AccessibilityAction.ACTION_CLEAR_ACCESSIBILITY_FOCUS); } - info.setClickable(mNodeWithReplacementActions.isClickable()); - info.setFocusable(mNodeWithReplacementActions.isFocusable()); - info.setContextClickable(mNodeWithReplacementActions.isContextClickable()); - info.setScrollable(mNodeWithReplacementActions.isScrollable()); - info.setLongClickable(mNodeWithReplacementActions.isLongClickable()); - info.setDismissable(mNodeWithReplacementActions.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(); + } + mNodesWithReplacementActions = null; + } }