Skip to content

Commit 30f36b1

Browse files
SallyGenkzsz11
authored andcommitted
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
1 parent fd6b13c commit 30f36b1

2 files changed

Lines changed: 34 additions & 60 deletions

File tree

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

Lines changed: 28 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ private void addWindowByDisplayLocked(int displayId, AccessibilityWindowInfo win
158158
* @param event An event.
159159
*/
160160
public void onAccessibilityEvent(AccessibilityEvent event) {
161+
AccessibilityNodeInfo nodeToRefresh = null;
161162
synchronized (mLock) {
162163
if (DEBUG) {
163164
Log.i(LOG_TAG, "onAccessibilityEvent(" + event + ")");
@@ -166,35 +167,38 @@ public void onAccessibilityEvent(AccessibilityEvent event) {
166167
switch (eventType) {
167168
case AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED: {
168169
if (mAccessibilityFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID) {
169-
refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
170+
removeCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
170171
}
171172
mAccessibilityFocus = event.getSourceNodeId();
172173
mAccessibilityFocusedWindow = event.getWindowId();
173-
refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
174+
nodeToRefresh = removeCachedNodeLocked(mAccessibilityFocusedWindow,
175+
mAccessibilityFocus);
174176
} break;
175177

176178
case AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED: {
177179
if (mAccessibilityFocus == event.getSourceNodeId()
178180
&& mAccessibilityFocusedWindow == event.getWindowId()) {
179-
refreshCachedNodeLocked(mAccessibilityFocusedWindow, mAccessibilityFocus);
181+
nodeToRefresh = removeCachedNodeLocked(mAccessibilityFocusedWindow,
182+
mAccessibilityFocus);
180183
mAccessibilityFocus = AccessibilityNodeInfo.UNDEFINED_ITEM_ID;
181184
mAccessibilityFocusedWindow = AccessibilityWindowInfo.UNDEFINED_WINDOW_ID;
182185
}
183186
} break;
184187

185188
case AccessibilityEvent.TYPE_VIEW_FOCUSED: {
186189
if (mInputFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID) {
187-
refreshCachedNodeLocked(event.getWindowId(), mInputFocus);
190+
removeCachedNodeLocked(event.getWindowId(), mInputFocus);
188191
}
189192
mInputFocus = event.getSourceNodeId();
190-
refreshCachedNodeLocked(event.getWindowId(), mInputFocus);
193+
nodeToRefresh = removeCachedNodeLocked(event.getWindowId(), mInputFocus);
191194
} break;
192195

193196
case AccessibilityEvent.TYPE_VIEW_SELECTED:
194197
case AccessibilityEvent.TYPE_VIEW_TEXT_CHANGED:
195198
case AccessibilityEvent.TYPE_VIEW_CLICKED:
196199
case AccessibilityEvent.TYPE_VIEW_TEXT_SELECTION_CHANGED: {
197-
refreshCachedNodeLocked(event.getWindowId(), event.getSourceNodeId());
200+
nodeToRefresh = removeCachedNodeLocked(event.getWindowId(),
201+
event.getSourceNodeId());
198202
} break;
199203

200204
case AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED: {
@@ -205,7 +209,7 @@ public void onAccessibilityEvent(AccessibilityEvent event) {
205209
& AccessibilityEvent.CONTENT_CHANGE_TYPE_SUBTREE) != 0) {
206210
clearSubTreeLocked(windowId, sourceId);
207211
} else {
208-
refreshCachedNodeLocked(windowId, sourceId);
212+
nodeToRefresh = removeCachedNodeLocked(windowId, sourceId);
209213
}
210214
}
211215
} break;
@@ -218,8 +222,8 @@ public void onAccessibilityEvent(AccessibilityEvent event) {
218222
if (event.getWindowChanges()
219223
== AccessibilityEvent.WINDOWS_CHANGE_ACCESSIBILITY_FOCUSED) {
220224
// Don't need to clear all cache. Unless the changes are related to
221-
// content, we won't clear all cache here.
222-
refreshCachedWindowLocked(event.getWindowId());
225+
// content, we won't clear all cache here with clear().
226+
clearWindowCacheLocked();
223227
break;
224228
}
225229
case AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED: {
@@ -228,59 +232,34 @@ public void onAccessibilityEvent(AccessibilityEvent event) {
228232
}
229233
}
230234

235+
if (nodeToRefresh != null) {
236+
if (DEBUG) {
237+
Log.i(LOG_TAG, "Refreshing and re-adding cached node.");
238+
}
239+
if (mAccessibilityNodeRefresher.refreshNode(nodeToRefresh, true)) {
240+
add(nodeToRefresh);
241+
}
242+
}
231243
if (CHECK_INTEGRITY) {
232244
checkIntegrity();
233245
}
234246
}
235247

236-
private void refreshCachedNodeLocked(int windowId, long sourceId) {
248+
private AccessibilityNodeInfo removeCachedNodeLocked(int windowId, long sourceId) {
237249
if (DEBUG) {
238-
Log.i(LOG_TAG, "Refreshing cached node.");
250+
Log.i(LOG_TAG, "Removing cached node.");
239251
}
240-
241252
LongSparseArray<AccessibilityNodeInfo> nodes = mNodeCache.get(windowId);
242253
if (nodes == null) {
243-
return;
254+
return null;
244255
}
245256
AccessibilityNodeInfo cachedInfo = nodes.get(sourceId);
246257
// If the source is not in the cache - nothing to do.
247258
if (cachedInfo == null) {
248-
return;
249-
}
250-
// The node changed so we will just refresh it right now.
251-
if (mAccessibilityNodeRefresher.refreshNode(cachedInfo, true)) {
252-
return;
253-
}
254-
// Weird, we could not refresh. Just evict the entire sub-tree.
255-
clearSubTreeLocked(windowId, sourceId);
256-
}
257-
258-
private void refreshCachedWindowLocked(int windowId) {
259-
if (DEBUG) {
260-
Log.i(LOG_TAG, "Refreshing cached window.");
261-
}
262-
263-
if (windowId == AccessibilityWindowInfo.UNDEFINED_WINDOW_ID) {
264-
return;
265-
}
266-
267-
final int displayCounts = mWindowCacheByDisplay.size();
268-
for (int i = 0; i < displayCounts; i++) {
269-
final SparseArray<AccessibilityWindowInfo> windowsOfDisplay =
270-
mWindowCacheByDisplay.valueAt(i);
271-
if (windowsOfDisplay == null) {
272-
continue;
273-
}
274-
final AccessibilityWindowInfo window = windowsOfDisplay.get(windowId);
275-
if (window == null) {
276-
continue;
277-
}
278-
if (!mAccessibilityNodeRefresher.refreshWindow(window)) {
279-
// If we fail to refresh the window, clear all windows.
280-
clearWindowCacheLocked();
281-
}
282-
return;
259+
return null;
283260
}
261+
nodes.remove(sourceId);
262+
return cachedInfo;
284263
}
285264

286265
/**
@@ -450,7 +429,7 @@ public void add(AccessibilityNodeInfo info) {
450429
if (clone.isAccessibilityFocused()) {
451430
if (mAccessibilityFocus != AccessibilityNodeInfo.UNDEFINED_ITEM_ID
452431
&& mAccessibilityFocus != sourceId) {
453-
refreshCachedNodeLocked(windowId, mAccessibilityFocus);
432+
removeCachedNodeLocked(windowId, mAccessibilityFocus);
454433
}
455434
mAccessibilityFocus = sourceId;
456435
mAccessibilityFocusedWindow = windowId;

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ public void nodeSourceOfA11yFocusEvent_getsRefreshed() {
563563
}
564564

565565
@Test
566-
public void nodeWithA11yFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
566+
public void nodeWithA11yFocusWhenAnotherNodeGetsFocus_getsRemoved() {
567567
AccessibilityNodeInfo nodeInfo = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
568568
nodeInfo.setAccessibilityFocused(true);
569569
mAccessibilityCache.add(nodeInfo);
@@ -573,7 +573,7 @@ public void nodeWithA11yFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
573573
mAccessibilityCache.onAccessibilityEvent(event);
574574
event.recycle();
575575
try {
576-
verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo, true);
576+
assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
577577
} finally {
578578
nodeInfo.recycle();
579579
}
@@ -614,7 +614,7 @@ public void nodeSourceOfInputFocusEvent_getsRefreshed() {
614614
}
615615

616616
@Test
617-
public void nodeWithInputFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
617+
public void nodeWithInputFocusWhenAnotherNodeGetsFocus_getsRemoved() {
618618
AccessibilityNodeInfo nodeInfo = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
619619
nodeInfo.setFocused(true);
620620
mAccessibilityCache.add(nodeInfo);
@@ -624,7 +624,7 @@ public void nodeWithInputFocusWhenAnotherNodeGetsFocus_getsRefreshed() {
624624
mAccessibilityCache.onAccessibilityEvent(event);
625625
event.recycle();
626626
try {
627-
verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo, true);
627+
assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
628628
} finally {
629629
nodeInfo.recycle();
630630
}
@@ -733,20 +733,15 @@ public void testCacheCriticalEventList_doesntLackEvents() {
733733
}
734734

735735
@Test
736-
public void addA11yFocusNodeBeforeFocusClearedEvent_previousA11yFocusNodeGetsRefreshed() {
736+
public void addA11yFocusNodeBeforeFocusClearedEvent_previousA11yFocusNodeGetsRemoved() {
737737
AccessibilityNodeInfo nodeInfo1 = getNodeWithA11yAndWindowId(SINGLE_VIEW_ID, WINDOW_ID_1);
738738
nodeInfo1.setAccessibilityFocused(true);
739739
mAccessibilityCache.add(nodeInfo1);
740740
AccessibilityNodeInfo nodeInfo2 = getNodeWithA11yAndWindowId(OTHER_VIEW_ID, WINDOW_ID_1);
741741
nodeInfo2.setAccessibilityFocused(true);
742742
mAccessibilityCache.add(nodeInfo2);
743-
AccessibilityEvent event = AccessibilityEvent.obtain(
744-
AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
745-
event.setSource(getMockViewWithA11yAndWindowIds(SINGLE_VIEW_ID, WINDOW_ID_1));
746-
mAccessibilityCache.onAccessibilityEvent(event);
747-
event.recycle();
748743
try {
749-
verify(mAccessibilityNodeRefresher).refreshNode(nodeInfo1, true);
744+
assertNull(mAccessibilityCache.getNode(WINDOW_ID_1, SINGLE_VIEW_ID));
750745
} finally {
751746
nodeInfo1.recycle();
752747
nodeInfo2.recycle();

0 commit comments

Comments
 (0)