Unlocking getDisplayByUserIdAndWindowId

The above function calls a synchronized function in a different service, which can cause an ANR when that service is slow to respond.
This change restructures the function to only apply a lock to the portion that does not call out to the other service.
Logic that should be synchronized will remain so, but this should prevent needlessly holding onto the lock.

Bug: 327710829
Test: atest AbstractAccessibilityServiceConnectionTest
Flag: N/A
Change-Id: I68f005e48660a70a9272bbf873020118ac898e82

4 files changed, 18 insertions, 16 deletions

diff --git a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
index af47ed28e3b0..73584154df3a 100644
--- a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
+++ b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
@@ -611,12 +611,12 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ
         if (svcConnTracingEnabled()) {
             logTraceSvcConn("getWindow", "windowId=" + windowId);
         }
+        int displayId = Display.INVALID_DISPLAY;
+        if (windowId != AccessibilityWindowInfo.UNDEFINED_WINDOW_ID) {
+            displayId = mA11yWindowManager.getDisplayIdByUserIdAndWindowId(
+                    mSystemSupport.getCurrentUserIdLocked(), windowId);
+        }
         synchronized (mLock) {
-            int displayId = Display.INVALID_DISPLAY;
-            if (windowId != AccessibilityWindowInfo.UNDEFINED_WINDOW_ID) {
-                displayId = mA11yWindowManager.getDisplayIdByUserIdAndWindowIdLocked(
-                        mSystemSupport.getCurrentUserIdLocked(), windowId);
-            }
             ensureWindowsAvailableTimedLocked(displayId);
 
             if (!hasRightsToCurrentUserLocked()) {
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
index cbb66dc18f28..ffc92aad4fae 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
@@ -1303,15 +1303,14 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub
             // the computation for performance reasons.
             boolean shouldComputeWindows = false;
             int displayId = event.getDisplayId();
+            final int windowId = event.getWindowId();
+            if (windowId != AccessibilityWindowInfo.UNDEFINED_WINDOW_ID
+                    && displayId == Display.INVALID_DISPLAY) {
+                displayId = mA11yWindowManager.getDisplayIdByUserIdAndWindowId(
+                        resolvedUserId, windowId);
+                event.setDisplayId(displayId);
+            }
             synchronized (mLock) {
-                final int windowId = event.getWindowId();
-                if (windowId != AccessibilityWindowInfo.UNDEFINED_WINDOW_ID
-                        && displayId == Display.INVALID_DISPLAY) {
-                    displayId = mA11yWindowManager.getDisplayIdByUserIdAndWindowIdLocked(
-                            resolvedUserId, windowId);
-                    event.setDisplayId(displayId);
-                }
-
                 if (event.getEventType() == AccessibilityEvent.TYPE_WINDOW_STATE_CHANGED
                         && displayId != Display.INVALID_DISPLAY
                         && mA11yWindowManager.isTrackingWindowsLocked(displayId)) {
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java
index b8181505b9c4..8c06bc8f4607 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityWindowManager.java
@@ -2038,8 +2038,11 @@ public class AccessibilityWindowManager {
      * @param windowId The windowId
      * @return The display ID
      */
-    public int getDisplayIdByUserIdAndWindowIdLocked(int userId, int windowId) {
-        final IBinder windowToken = getWindowTokenForUserAndWindowIdLocked(userId, windowId);
+    public int getDisplayIdByUserIdAndWindowId(int userId, int windowId) {
+        final IBinder windowToken;
+        synchronized (mLock) {
+            windowToken = getWindowTokenForUserAndWindowIdLocked(userId, windowId);
+        }
         if (traceWMEnabled()) {
             logTraceWM("getDisplayIdForWindow", "token=" + windowToken);
         }
diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java
index e168596b8eb2..16d05b157727 100644
--- a/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java
+++ b/services/tests/servicestests/src/com/android/server/accessibility/AbstractAccessibilityServiceConnectionTest.java
@@ -214,7 +214,7 @@ public class AbstractAccessibilityServiceConnectionTest {
                 .thenReturn(mA11yWindowInfos.get(0));
         when(mMockA11yWindowManager.findA11yWindowInfoByIdLocked(PIP_WINDOWID))
                 .thenReturn(mA11yWindowInfos.get(1));
-        when(mMockA11yWindowManager.getDisplayIdByUserIdAndWindowIdLocked(USER_ID,
+        when(mMockA11yWindowManager.getDisplayIdByUserIdAndWindowId(USER_ID,
             WINDOWID_ONSECONDDISPLAY)).thenReturn(SECONDARY_DISPLAY_ID);
         when(mMockA11yWindowManager.getWindowListLocked(SECONDARY_DISPLAY_ID))
             .thenReturn(mA11yWindowInfosOnSecondDisplay);