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

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

Protect a11y cache against loops in node hierarchy 

Plugging a hole where we would recycle a node that is its
own ancestor twice if it was a descendant of the last
child we iterated over.

Bug: 112549653
Test: Adding new test case that reproduced the issue before
and now passes.

Change-Id: I6afc83655d4d7ca1c01f21703c8f9f9ac23e4ac6
parent 8f80b51b

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

+5 −5
Original line number Diff line number Diff line
@@ -306,6 +306,11 @@ public class AccessibilityCache { 


                final int oldChildCount = oldInfo.getChildCount();

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

                    final long oldChildId = oldInfo.getChildId(i);

                    // If the child is no longer present, remove the sub-tree.

                    if (newChildrenIds == null || newChildrenIds.indexOf(oldChildId) < 0) {

                        clearSubTreeLocked(windowId, oldChildId);

                    }

                    if (nodes.get(sourceId) == null) {

                        // We've removed (and thus recycled) this node because it was its own

                        // ancestor (the app gave us bad data), we can't continue using it.
@@ -313,11 +318,6 @@ public class AccessibilityCache { 
                        clearNodesForWindowLocked(windowId);

                        return;

                    }

                    final long oldChildId = oldInfo.getChildId(i);

                    // If the child is no longer present, remove the sub-tree.

                    if (newChildrenIds == null || newChildrenIds.indexOf(oldChildId) < 0) {

                        clearSubTreeLocked(windowId, oldChildId);

                    }

                }



                // Also be careful if the parent has changed since the new

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

+24 −1
Original line number Diff line number Diff line
@@ -501,7 +501,7 @@ public class AccessibilityCacheTest { 
    }



    @Test

    public void addNode_whenNodeBeingReplacedIsOwnGrandparent_doesntCrash() {

    public void addNode_whenNodeBeingReplacedIsOwnGrandparentWithTwoChildren_doesntCrash() {

        AccessibilityNodeInfo parentNodeInfo =

                getNodeWithA11yAndWindowId(PARENT_VIEW_ID, WINDOW_ID_1);

        parentNodeInfo.addChild(getMockViewWithA11yAndWindowIds(CHILD_VIEW_ID, WINDOW_ID_1));
@@ -524,6 +524,29 @@ public class AccessibilityCacheTest { 
        }

    }



    @Test

    public void addNode_whenNodeBeingReplacedIsOwnGrandparentWithOneChild_doesntCrash() {

        AccessibilityNodeInfo parentNodeInfo =

                getNodeWithA11yAndWindowId(PARENT_VIEW_ID, WINDOW_ID_1);

        parentNodeInfo.addChild(getMockViewWithA11yAndWindowIds(CHILD_VIEW_ID, WINDOW_ID_1));

        AccessibilityNodeInfo childNodeInfo =

                getNodeWithA11yAndWindowId(CHILD_VIEW_ID, WINDOW_ID_1);

        childNodeInfo.setParent(getMockViewWithA11yAndWindowIds(PARENT_VIEW_ID, WINDOW_ID_1));

        childNodeInfo.addChild(getMockViewWithA11yAndWindowIds(PARENT_VIEW_ID, WINDOW_ID_1));



        AccessibilityNodeInfo replacementParentNodeInfo =

                getNodeWithA11yAndWindowId(PARENT_VIEW_ID, WINDOW_ID_1);

        try {

            mAccessibilityCache.add(parentNodeInfo);

            mAccessibilityCache.add(childNodeInfo);

            mAccessibilityCache.add(replacementParentNodeInfo);

        } finally {

            parentNodeInfo.recycle();

            childNodeInfo.recycle();

            replacementParentNodeInfo.recycle();

        }

    }



    @Test

    public void testCacheCriticalEventList_doesntLackEvents() {

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