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

Commit c140fdc3 authored by Phil Weaver's avatar Phil Weaver
Browse files

Bypass a11y cache when requested 

When refreshing a node from the cache, we would still ask
the cache to update itself in the middle of the refresh,
which led to nodes being put in inconsistent states.

Also rolling back ag/3051037, which was a temporary fix for
one crashing bug.

I'm also doing a bit of cleanup. Several methods in
AccessibilityInteractionClient should really be static,
since they only manipulate global state.

Also, since I was already adding tests, I'm moving the
tests for android.view.accessibility to their proper
place in the source tree.

Fixes: 68987775
Fixes: 67735115
Test: Adding a test to verify that we don't touch the cache
when refreshing a node. This test fails without the change
to AccessibilityInteractionClient, and passes now.

Change-Id: If642ca79bc414ef9846b7b2a2277870f4143cb98
parent a8d534bf

core/java/android/view/accessibility/AccessibilityCache.java

+3 −4
Original line number Diff line number Diff line
@@ -23,8 +23,6 @@ import android.util.LongArray; 
import android.util.LongSparseArray;

import android.util.SparseArray;



import com.android.internal.annotations.VisibleForTesting;



import java.util.ArrayList;

import java.util.List;
@@ -33,8 +31,7 @@ import java.util.List; 
 * It is updated when windows change or nodes change.

 * @hide

 */

@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

public final class AccessibilityCache {

public class AccessibilityCache {



    private static final String LOG_TAG = "AccessibilityCache";
@@ -329,6 +326,8 @@ public final class AccessibilityCache { 
                final long oldParentId = oldInfo.getParentNodeId();

                if (info.getParentNodeId() != oldParentId) {

                    clearSubTreeLocked(windowId, oldParentId);

                } else {

                    oldInfo.recycle();

                }

           }

core/java/android/view/accessibility/AccessibilityInteractionClient.java

+67 −50
Original line number Diff line number Diff line
@@ -28,6 +28,8 @@ import android.util.Log; 
import android.util.LongSparseArray;

import android.util.SparseArray;



import com.android.internal.annotations.VisibleForTesting;



import java.util.ArrayList;

import java.util.Collections;

import java.util.HashSet;
@@ -86,6 +88,12 @@ public final class AccessibilityInteractionClient 
    private static final LongSparseArray<AccessibilityInteractionClient> sClients =

        new LongSparseArray<>();



    private static final SparseArray<IAccessibilityServiceConnection> sConnectionCache =

            new SparseArray<>();



    private static AccessibilityCache sAccessibilityCache =

            new AccessibilityCache(new AccessibilityCache.AccessibilityNodeRefresher());



    private final AtomicInteger mInteractionIdCounter = new AtomicInteger();



    private final Object mInstanceLock = new Object();
@@ -100,12 +108,6 @@ public final class AccessibilityInteractionClient 


    private Message mSameThreadMessage;



    private static final SparseArray<IAccessibilityServiceConnection> sConnectionCache =

        new SparseArray<>();



    private static final AccessibilityCache sAccessibilityCache =

        new AccessibilityCache(new AccessibilityCache.AccessibilityNodeRefresher());



    /**

     * @return The client for the current thread.

     */
@@ -133,6 +135,50 @@ public final class AccessibilityInteractionClient 
        }

    }



    /**

     * Gets a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     * @return The cached connection if such.

     */

    public static IAccessibilityServiceConnection getConnection(int connectionId) {

        synchronized (sConnectionCache) {

            return sConnectionCache.get(connectionId);

        }

    }



    /**

     * Adds a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     * @param connection The connection.

     */

    public static void addConnection(int connectionId, IAccessibilityServiceConnection connection) {

        synchronized (sConnectionCache) {

            sConnectionCache.put(connectionId, connection);

        }

    }



    /**

     * Removes a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     */

    public static void removeConnection(int connectionId) {

        synchronized (sConnectionCache) {

            sConnectionCache.remove(connectionId);

        }

    }



    /**

     * This method is only for testing. Replacing the cache is a generally terrible idea, but

     * tests need to be able to verify this class's interactions with the cache

     */

    @VisibleForTesting

    public static void setCache(AccessibilityCache cache) {

        sAccessibilityCache = cache;

    }



    private AccessibilityInteractionClient() {

        /* reducing constructor visibility */

    }
@@ -300,7 +346,7 @@ public final class AccessibilityInteractionClient 
                if (success) {

                    List<AccessibilityNodeInfo> infos = getFindAccessibilityNodeInfosResultAndClear(

                            interactionId);

                    finalizeAndCacheAccessibilityNodeInfos(infos, connectionId);

                    finalizeAndCacheAccessibilityNodeInfos(infos, connectionId, bypassCache);

                    if (infos != null && !infos.isEmpty()) {

                        for (int i = 1; i < infos.size(); i++) {

                            infos.get(i).recycle();
@@ -356,7 +402,7 @@ public final class AccessibilityInteractionClient 
                    List<AccessibilityNodeInfo> infos = getFindAccessibilityNodeInfosResultAndClear(

                            interactionId);

                    if (infos != null) {

                        finalizeAndCacheAccessibilityNodeInfos(infos, connectionId);

                        finalizeAndCacheAccessibilityNodeInfos(infos, connectionId, false);

                        return infos;

                    }

                }
@@ -409,7 +455,7 @@ public final class AccessibilityInteractionClient 
                    List<AccessibilityNodeInfo> infos = getFindAccessibilityNodeInfosResultAndClear(

                            interactionId);

                    if (infos != null) {

                        finalizeAndCacheAccessibilityNodeInfos(infos, connectionId);

                        finalizeAndCacheAccessibilityNodeInfos(infos, connectionId, false);

                        return infos;

                    }

                }
@@ -460,7 +506,7 @@ public final class AccessibilityInteractionClient 
                if (success) {

                    AccessibilityNodeInfo info = getFindAccessibilityNodeInfoResultAndClear(

                            interactionId);

                    finalizeAndCacheAccessibilityNodeInfo(info, connectionId);

                    finalizeAndCacheAccessibilityNodeInfo(info, connectionId, false);

                    return info;

                }

            } else {
@@ -509,7 +555,7 @@ public final class AccessibilityInteractionClient 
                if (success) {

                    AccessibilityNodeInfo info = getFindAccessibilityNodeInfoResultAndClear(

                            interactionId);

                    finalizeAndCacheAccessibilityNodeInfo(info, connectionId);

                    finalizeAndCacheAccessibilityNodeInfo(info, connectionId, false);

                    return info;

                }

            } else {
@@ -731,29 +777,35 @@ public final class AccessibilityInteractionClient 
     *

     * @param info The info.

     * @param connectionId The id of the connection to the system.

     * @param bypassCache Whether or not to bypass the cache. The node is added to the cache if

     *                    this value is {@code false}

     */

    private void finalizeAndCacheAccessibilityNodeInfo(AccessibilityNodeInfo info,

            int connectionId) {

            int connectionId, boolean bypassCache) {

        if (info != null) {

            info.setConnectionId(connectionId);

            info.setSealed(true);

            if (!bypassCache) {

                sAccessibilityCache.add(info);

            }

        }

    }



    /**

     * Finalize {@link AccessibilityNodeInfo}s before passing them to the client.

     *

     * @param infos The {@link AccessibilityNodeInfo}s.

     * @param connectionId The id of the connection to the system.

     * @param bypassCache Whether or not to bypass the cache. The nodes are added to the cache if

     *                    this value is {@code false}

     */

    private void finalizeAndCacheAccessibilityNodeInfos(List<AccessibilityNodeInfo> infos,

            int connectionId) {

            int connectionId, boolean bypassCache) {

        if (infos != null) {

            final int infosCount = infos.size();

            for (int i = 0; i < infosCount; i++) {

                AccessibilityNodeInfo info = infos.get(i);

                finalizeAndCacheAccessibilityNodeInfo(info, connectionId);

                finalizeAndCacheAccessibilityNodeInfo(info, connectionId, bypassCache);

            }

        }

    }
@@ -772,41 +824,6 @@ public final class AccessibilityInteractionClient 
        }

    }



    /**

     * Gets a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     * @return The cached connection if such.

     */

    public IAccessibilityServiceConnection getConnection(int connectionId) {

        synchronized (sConnectionCache) {

            return sConnectionCache.get(connectionId);

        }

    }



    /**

     * Adds a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     * @param connection The connection.

     */

    public void addConnection(int connectionId, IAccessibilityServiceConnection connection) {

        synchronized (sConnectionCache) {

            sConnectionCache.put(connectionId, connection);

        }

    }



    /**

     * Removes a cached accessibility service connection.

     *

     * @param connectionId The connection id.

     */

    public void removeConnection(int connectionId) {

        synchronized (sConnectionCache) {

            sConnectionCache.remove(connectionId);

        }

    }



    /**

     * Checks whether the infos are a fully connected tree with no duplicates.

     *

services/tests/servicestests/src/com/android/server/accessibility/AccessibilityCacheTest.java→core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java

+19 −21
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2016 The Android Open Source Project

 * Copyright 2016 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.
@@ -14,26 +14,23 @@ 
 * limitations under the License.

 */



package com.android.server.accessibility;

package android.view.accessibility;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertNotNull;

import static junit.framework.Assert.assertNull;



import static org.mockito.Matchers.anyBoolean;

import static org.mockito.Matchers.anyObject;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;



import android.support.test.runner.AndroidJUnit4;

import android.view.accessibility.AccessibilityCache;

import android.view.accessibility.AccessibilityEvent;

import android.view.accessibility.AccessibilityInteractionClient;

import android.view.accessibility.AccessibilityNodeInfo;

import android.view.accessibility.AccessibilityWindowInfo;

import android.view.View;



import org.junit.After;

import org.junit.Before;

import org.junit.Test;
@@ -48,27 +45,27 @@ import java.util.concurrent.atomic.AtomicInteger; 


@RunWith(AndroidJUnit4.class)

public class AccessibilityCacheTest {

    private static int WINDOW_ID_1 = 0xBEEF;

    private static int WINDOW_ID_2 = 0xFACE;

    private static int SINGLE_VIEW_ID = 0xCAFE;

    private static int OTHER_VIEW_ID = 0xCAB2;

    private static int PARENT_VIEW_ID = 0xFED4;

    private static int CHILD_VIEW_ID = 0xFEED;

    private static int OTHER_CHILD_VIEW_ID = 0xACE2;

    private static int MOCK_CONNECTION_ID = 1;

    private static final int WINDOW_ID_1 = 0xBEEF;

    private static final int WINDOW_ID_2 = 0xFACE;

    private static final int SINGLE_VIEW_ID = 0xCAFE;

    private static final int OTHER_VIEW_ID = 0xCAB2;

    private static final int PARENT_VIEW_ID = 0xFED4;

    private static final int CHILD_VIEW_ID = 0xFEED;

    private static final int OTHER_CHILD_VIEW_ID = 0xACE2;

    private static final int MOCK_CONNECTION_ID = 1;



    AccessibilityCache mAccessibilityCache;

    AccessibilityCache.AccessibilityNodeRefresher mAccessibilityNodeRefresher;

    AtomicInteger numA11yNodeInfosInUse = new AtomicInteger(0);

    AtomicInteger numA11yWinInfosInUse = new AtomicInteger(0);

    AtomicInteger mNumA11yNodeInfosInUse = new AtomicInteger(0);

    AtomicInteger mNumA11yWinInfosInUse = new AtomicInteger(0);



    @Before

    public void setUp() {

        mAccessibilityNodeRefresher = mock(AccessibilityCache.AccessibilityNodeRefresher.class);

        when(mAccessibilityNodeRefresher.refreshNode(anyObject(), anyBoolean())).thenReturn(true);

        mAccessibilityCache = new AccessibilityCache(mAccessibilityNodeRefresher);

        AccessibilityNodeInfo.setNumInstancesInUseCounter(numA11yNodeInfosInUse);

        AccessibilityWindowInfo.setNumInstancesInUseCounter(numA11yWinInfosInUse);

        AccessibilityNodeInfo.setNumInstancesInUseCounter(mNumA11yNodeInfosInUse);

        AccessibilityWindowInfo.setNumInstancesInUseCounter(mNumA11yWinInfosInUse);

    }



    @After
@@ -76,7 +73,8 @@ public class AccessibilityCacheTest { 
        // Make sure we're recycling all of our window and node infos

        mAccessibilityCache.clear();

        AccessibilityInteractionClient.getInstance().clearCache();

        assertEquals(0, numA11yWinInfosInUse.get());

        assertEquals(0, mNumA11yWinInfosInUse.get());

        assertEquals(0, mNumA11yNodeInfosInUse.get());

    }



    @Test

core/tests/coretests/src/android/view/accessibility/AccessibilityInteractionClientTest.java

0 → 100644
+88 −0
Original line number Diff line number Diff line 
/*

 * Copyright 2017 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package android.view.accessibility;



import static org.junit.Assert.assertEquals;

import static org.mockito.Mockito.verifyZeroInteractions;

import static org.mockito.MockitoAnnotations.initMocks;



import android.os.Bundle;

import android.os.RemoteException;

import android.support.test.runner.AndroidJUnit4;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;

import org.mockito.Mock;



import java.util.Arrays;

import java.util.List;



/**

 * Tests for AccessibilityInteractionClient

 */

@RunWith(AndroidJUnit4.class)

public class AccessibilityInteractionClientTest {

    private static final int MOCK_CONNECTION_ID = 0xabcd;



    private MockConnection mMockConnection = new MockConnection();

    @Mock private AccessibilityCache mMockCache;



    @Before

    public void setUp() {

        initMocks(this);

        AccessibilityInteractionClient.setCache(mMockCache);

        AccessibilityInteractionClient.addConnection(MOCK_CONNECTION_ID, mMockConnection);

    }



    /**

     * When the AccessibilityCache refreshes the nodes it contains, it gets very confused if

     * it is called to update itself during the refresh. It tries to update the node that it's

     * in the process of refreshing, which leads to AccessibilityNodeInfos in inconsistent states.

     */

    @Test

    public void findA11yNodeInfoByA11yId_whenBypassingCache_doesntTouchCache() {

        final int windowId = 0x1234;

        final long accessibilityNodeId = 0x4321L;

        AccessibilityNodeInfo nodeFromConnection = AccessibilityNodeInfo.obtain();

        nodeFromConnection.setSourceNodeId(accessibilityNodeId, windowId);

        mMockConnection.mInfosToReturn = Arrays.asList(nodeFromConnection);

        AccessibilityInteractionClient client = AccessibilityInteractionClient.getInstance();

        AccessibilityNodeInfo node = client.findAccessibilityNodeInfoByAccessibilityId(

                MOCK_CONNECTION_ID, windowId, accessibilityNodeId, true, 0, null);

        assertEquals("Node got lost along the way", nodeFromConnection, node);



        verifyZeroInteractions(mMockCache);

    }



    private static class MockConnection extends AccessibilityServiceConnectionImpl {

        List<AccessibilityNodeInfo> mInfosToReturn;



        @Override

        public boolean findAccessibilityNodeInfoByAccessibilityId(int accessibilityWindowId,

                long accessibilityNodeId, int interactionId,

                IAccessibilityInteractionConnectionCallback callback, int flags, long threadId,

                Bundle arguments) {

            try {

                callback.setFindAccessibilityNodeInfosResult(mInfosToReturn, interactionId);

            } catch (RemoteException e) {

                throw new RuntimeException(e);

            }

            return true;

        }

    }

}

services/tests/servicestests/src/com/android/server/accessibility/AccessibilityNodeInfoTest.java→core/tests/coretests/src/android/view/accessibility/AccessibilityNodeInfoTest.java

+17 −2
Original line number Diff line number Diff line 
package com.android.server.accessibility;

/*

 * Copyright 2017 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package android.view.accessibility;



import static org.junit.Assert.fail;



import android.support.test.runner.AndroidJUnit4;

import android.util.ArraySet;

import android.view.accessibility.AccessibilityNodeInfo;

import android.view.accessibility.AccessibilityNodeInfo.AccessibilityAction;



import com.android.internal.util.CollectionUtils;