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

Commit 4d70022b authored by Qasid Ahmad Sadiq's avatar Qasid Ahmad Sadiq
Browse files

Revert "Fix a11y cache correctness bug" 

This fix introduced a painful crash that ends up disabling accessibility
services for certain users.
This happens when a client of AccessibilityCache tries to add a node, with the same id as a node previously in the cache, but fewer children, where the removed child is not in the cache.
This is because, when children are removed, and a the node is updated, the cache tries to clear the child trees. But if the child is not in the cache, the cache clears the whole tree. Every node is recycled.
Then the original node being replaced is attempted to be recycled again, and voila crash.

The fix also didn't fix the original issue based on the discussion in
b/114133438.

The risk for this is pretty low, since nothing was built on top of this.

This reverts commit 2f69c16c.
Bug: 124676705
Test: Tested to see if above usecase still happens.

Change-Id: I8a39698c4532a1613ba47e1c6ca70201cd496212
parent ec9d9834

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

+3 −11
Original line number Diff line number Diff line
@@ -418,28 +418,20 @@ public class AccessibilityCache { 
     *

     * @param nodes The nodes in the hosting window.

     * @param rootNodeId The id of the root to evict.

     *

     * @return {@code true} if the cache was cleared

     */

    private boolean clearSubTreeRecursiveLocked(LongSparseArray<AccessibilityNodeInfo> nodes,

    private void clearSubTreeRecursiveLocked(LongSparseArray<AccessibilityNodeInfo> nodes,

            long rootNodeId) {

        AccessibilityNodeInfo current = nodes.get(rootNodeId);

        if (current == null) {

            // The node isn't in the cache, but its descendents might be.

            clear();

            return true;

            return;

        }

        nodes.remove(rootNodeId);

        final int childCount = current.getChildCount();

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

            final long childNodeId = current.getChildId(i);

            if (clearSubTreeRecursiveLocked(nodes, childNodeId)) {

                current.recycle();

                return true;

            }

            clearSubTreeRecursiveLocked(nodes, childNodeId);

        }

        current.recycle();

        return false;

    }



    /**

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

+0 −20
Original line number Diff line number Diff line
@@ -299,26 +299,6 @@ public class AccessibilityCacheTest { 
        }

    }



    @Test

    public void subTreeChangeEventFromUncachedNode_clearsNodeInCache() {

        AccessibilityNodeInfo nodeInfo = getNodeWithA11yAndWindowId(CHILD_VIEW_ID, WINDOW_ID_1);

        long id = nodeInfo.getSourceNodeId();

        mAccessibilityCache.add(nodeInfo);

        nodeInfo.recycle();



        AccessibilityEvent event = AccessibilityEvent

                .obtain(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);

        event.setContentChangeTypes(AccessibilityEvent.CONTENT_CHANGE_TYPE_SUBTREE);

        event.setSource(getMockViewWithA11yAndWindowIds(PARENT_VIEW_ID, WINDOW_ID_1));



        mAccessibilityCache.onAccessibilityEvent(event);

        AccessibilityNodeInfo shouldBeNull = mAccessibilityCache.getNode(WINDOW_ID_1, id);

        if (shouldBeNull != null) {

            shouldBeNull.recycle();

        }

        assertNull(shouldBeNull);

    }



    @Test

    public void scrollEvent_clearsNodeAndChild() {

        AccessibilityEvent event = AccessibilityEvent