From a4002c8daf272f550787d94bed8e81749a891e66 Mon Sep 17 00:00:00 2001
From: Phil Weaver <pweaver@google.com>
Date: Tue, 17 Jul 2018 12:28:24 -0700
Subject: [PATCH] Fix a11y cache correctness bug

If two views change, their common ancestor can send
a subtree_changed event. But the cache is dropping
events that come from views that it hasn't cached.

Now clearing the cache entirely when a node not in
the cache sends a subtree changed event, since we
don't know which nodes are descendents and which
may have changed.

I'm not thrilled with this change, since it may
degrade performance, but I want to fix the correctness
problem quickly.

Bug: 111554539
Test: atest AccessibilityCacheTest
Change-Id: Ib32d3622cddd7001663943eff71e823d21f5e500
---
 .../accessibility/AccessibilityCache.java     | 14 ++++++++++---
 .../accessibility/AccessibilityCacheTest.java | 20 +++++++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/core/java/android/view/accessibility/AccessibilityCache.java b/core/java/android/view/accessibility/AccessibilityCache.java
index da5a1cd67922b..0e1e379d610a9 100644
--- a/core/java/android/view/accessibility/AccessibilityCache.java
+++ b/core/java/android/view/accessibility/AccessibilityCache.java
@@ -418,20 +418,28 @@ 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 void clearSubTreeRecursiveLocked(LongSparseArray<AccessibilityNodeInfo> nodes,
+    private boolean clearSubTreeRecursiveLocked(LongSparseArray<AccessibilityNodeInfo> nodes,
             long rootNodeId) {
         AccessibilityNodeInfo current = nodes.get(rootNodeId);
         if (current == null) {
-            return;
+            // The node isn't in the cache, but its descendents might be.
+            clear();
+            return true;
         }
         nodes.remove(rootNodeId);
         final int childCount = current.getChildCount();
         for (int i = 0; i < childCount; i++) {
             final long childNodeId = current.getChildId(i);
-            clearSubTreeRecursiveLocked(nodes, childNodeId);
+            if (clearSubTreeRecursiveLocked(nodes, childNodeId)) {
+                current.recycle();
+                return true;
+            }
         }
         current.recycle();
+        return false;
     }
 
     /**
diff --git a/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java b/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
index 4de8155663f5b..993378d8a4d04 100644
--- a/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
+++ b/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
@@ -299,6 +299,26 @@ 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