Eliminate potential deadlock in AccessibilityCache
The cache calls out with its lock to the app main thread when refreshing
nodes. This opens the door for a deadlock, and causes a slowdown for
non-main threads.
Bug: 180957109, 189786298
Test: atest CtsAccessibilityServiceTestCases CtsAccessibilityTestCases
CtsUiAutomationTestCases
FrameworksServicesTests:com.android.server.accessibility
FrameworksCoreTests:com.android.internal.accessibility
FrameworksCoreTests:android.view.accessibility
Merged-In: I27e4e64f778ae1c2a348e16cf8739f7a5596e0fc
Change-Id: I27e4e64f778ae1c2a348e16cf8739f7a5596e0fc
diff --git a/core/java/android/view/accessibility/AccessibilityCache.java b/core/java/android/view/accessibility/AccessibilityCache.java
index 9ab2c2b..d14dc6e 100644
--- a/core/java/android/view/accessibility/AccessibilityCache.java
+++ b/core/java/android/view/accessibility/AccessibilityCache.java
@@ -158,6 +158,7 @@
* @param event An event.
*/
public void onAccessibilityEvent(AccessibilityEvent event) {
+ AccessibilityNodeInfo nodeToRefresh = null;
synchronized (mLock) {
if (DEBUG) {
Log.i(LOG_TAG, "onAccessibilityEvent(" + event + ")");
@@ -166,17 +167,19 @@
switch (eventType) {
case AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED: {
if (mAccessibilityFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID) {
- refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
+ removeCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
}
mAccessibilityFocus = event.getSourceNodeId();
mAccessibilityFocusedWindow = event.getWindowId();
- refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
+ nodeToRefresh = removeCachedNodeLocked(mAccessibilityFocusedWindow,
+ mAccessibilityFocus);
} break;
case AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED: {
if (mAccessibilityFocus == event.getSourceNodeId()
&& mAccessibilityFocusedWindow == event.getWindowId()) {
- refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
+ nodeToRefresh = removeCachedNodeLocked(mAccessibilityFocusedWindow,
+ mAccessibilityFocus);
mAccessibilityFocus = AccessibilityNodeInfo.UNDEFINED_ITEM_ID;
mAccessibilityFocusedWindow = AccessibilityWindowInfo.UNDEFINED_WINDOW_ID;
}
@@ -184,17 +187,18 @@
case AccessibilityEvent.TYPE_VIEW_FOCUSED: {
if (mInputFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID) {
- refreshCachedNodeLocked(event.getWindowId(), mInputFocus);
+ removeCachedNodeLocked(event.getWindowId(), mInputFocus);
}
mInputFocus = event.getSourceNodeId();
- refreshCachedNodeLocked(event.getWindowId(), mInputFocus);
+ nodeToRefresh = removeCachedNodeLocked(event.getWindowId(), mInputFocus);
} break;
case AccessibilityEvent.TYPE_VIEW_SELECTED:
case AccessibilityEvent.TYPE_VIEW_TEXT_CHANGED:
case AccessibilityEvent.TYPE_VIEW_CLICKED:
case AccessibilityEvent.TYPE_VIEW_TEXT_SELECTION_CHANGED: {
- refreshCachedNodeLocked(event.getWindowId(), event.getSourceNodeId());
+ nodeToRefresh = removeCachedNodeLocked(event.getWindowId(),
+ event.getSourceNodeId());
} break;
case AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED: {
@@ -205,7 +209,7 @@
& AccessibilityEvent.CONTENT_CHANGE_TYPE_SUBTREE) != 0) {
clearSubTreeLocked(windowId, sourceId);
} else {
- refreshCachedNodeLocked(windowId, sourceId);
+ nodeToRefresh = removeCachedNodeLocked(windowId, sourceId);
}
}
} break;
@@ -218,8 +222,8 @@
if (event.getWindowChanges()
== AccessibilityEvent.WINDOWS_CHANGE_ACCESSIBILITY_FOCUSED) {
// Don't need to clear all cache. Unless the changes are related to
- // content, we won't clear all cache here.
- refreshCachedWindowLocked(event.getWindowId());
+ // content, we won't clear all cache here with clear().
+ clearWindowCacheLocked();
break;
}
case AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED: {
@@ -228,59 +232,34 @@
}
}
+ if (nodeToRefresh != null) {
+ if (DEBUG) {
+ Log.i(LOG_TAG, "Refreshing and re-adding cached node.");
+ }
+ if (mAccessibilityNodeRefresher.refreshNode(nodeToRefresh, true)) {
+ add(nodeToRefresh);
+ }
+ }
if (CHECK_INTEGRITY) {
checkIntegrity();
}
}
- private void refreshCachedNodeLocked(int windowId, long sourceId) {
+ private AccessibilityNodeInfo removeCachedNodeLocked(int windowId, long sourceId) {
if (DEBUG) {
- Log.i(LOG_TAG, "Refreshing cached node.");
+ Log.i(LOG_TAG, "Removing cached node.");
}
-
LongSparseArray<AccessibilityNodeInfo> nodes = mNodeCache.get(windowId);
if (nodes == null) {
- return;
+ return null;
}
AccessibilityNodeInfo cachedInfo = nodes.get(sourceId);
// If the source is not in the cache - nothing to do.
if (cachedInfo == null) {
- return;
+ return null;
}
- // The node changed so we will just refresh it right now.
- if (mAccessibilityNodeRefresher.refreshNode(cachedInfo, true)) {
- return;
- }
- // Weird, we could not refresh. Just evict the entire sub-tree.
- clearSubTreeLocked(windowId, sourceId);
- }
-
- private void refreshCachedWindowLocked(int windowId) {
- if (DEBUG) {
- Log.i(LOG_TAG, "Refreshing cached window.");
- }
-
- if (windowId == AccessibilityWindowInfo.UNDEFINED_WINDOW_ID) {
- return;
- }
-
- final int displayCounts = mWindowCacheByDisplay.size();
- for (int i = 0; i < displayCounts; i++) {
- final SparseArray<AccessibilityWindowInfo> windowsOfDisplay =
- mWindowCacheByDisplay.valueAt(i);
- if (windowsOfDisplay == null) {
- continue;
- }
- final AccessibilityWindowInfo window = windowsOfDisplay.get(windowId);
- if (window == null) {
- continue;
- }
- if (!mAccessibilityNodeRefresher.refreshWindow(window)) {
- // If we fail to refresh the window, clear all windows.
- clearWindowCacheLocked();
- }
- return;
- }
+ nodes.remove(sourceId);
+ return cachedInfo;
}
/**
@@ -450,7 +429,7 @@
if (clone.isAccessibilityFocused()) {
if (mAccessibilityFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID
&& mAccessibilityFocus != sourceId) {
- refreshCachedNodeLocked(windowId, mAccessibilityFocus);
+ removeCachedNodeLocked(windowId, mAccessibilityFocus);
}
mAccessibilityFocus = sourceId;
mAccessibilityFocusedWindow = windowId;
diff --git a/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java b/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
index 0d5db6d..d2b52ba 100644
--- a/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
+++ b/core/tests/coretests/src/android/view/accessibility/AccessibilityCacheTest.java
@@ -563,7 +563,7 @@
}
@Test
- public void nodeWithA11yFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
+ public void nodeWithA11yFocusWhenAnotherNodeGetsFocus_getsRemoved() {
AccessibilityNodeInfo nodeInfo = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
nodeInfo.setAccessibilityFocused(true);
mAccessibilityCache.add(nodeInfo);
@@ -573,7 +573,7 @@
mAccessibilityCache.onAccessibilityEvent(event);
event.recycle();
try {
- verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo, true);
+ assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
} finally {
nodeInfo.recycle();
}
@@ -614,7 +614,7 @@
}
@Test
- public void nodeWithInputFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
+ public void nodeWithInputFocusWhenAnotherNodeGetsFocus_getsRemoved() {
AccessibilityNodeInfo nodeInfo = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
nodeInfo.setFocused(true);
mAccessibilityCache.add(nodeInfo);
@@ -624,7 +624,7 @@
mAccessibilityCache.onAccessibilityEvent(event);
event.recycle();
try {
- verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo, true);
+ assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
} finally {
nodeInfo.recycle();
}
@@ -733,20 +733,15 @@
}
@Test
- public void addA11yFocusNodeBeforeFocusClearedEvent_previousA11yFocusNodeGetsRefreshed() {
+ public void addA11yFocusNodeBeforeFocusClearedEvent_previousA11yFocusNodeGetsRemoved() {
AccessibilityNodeInfo nodeInfo1 = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
nodeInfo1.setAccessibilityFocused(true);
mAccessibilityCache.add(nodeInfo1);
AccessibilityNodeInfo nodeInfo2 = getNodeWithA11yAndWindowId(OTHER_VIEW_ID, WINDOW_ID_1);
nodeInfo2.setAccessibilityFocused(true);
mAccessibilityCache.add(nodeInfo2);
- AccessibilityEvent event = AccessibilityEvent.obtain(
- AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
- event.setSource(getMockViewWithA11yAndWindowIds(SINGLE_VIEW_ID, WINDOW_ID_1));
- mAccessibilityCache.onAccessibilityEvent(event);
- event.recycle();
try {
- verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo1, true);
+ assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
} finally {
nodeInfo1.recycle();
nodeInfo2.recycle();