Release the A11yManagerService#mLock before calling WMS#addWindowToken.
To enable the flag:
adb shell device_config put accessibility \
com.android.server.accessibility.add_window_token_without_lock true
Bug: 297972548
Test: Perform the below tests with flag disabled & enabled...
Test: Use a service with an overlay; observe behavior is unchanged
Test: atest CtsAccessibilityServiceTestCases
Test: atest FrameworksServicesTests:com.android.server.accessibility
Change-Id: I8599bc9bf26ba86c9aef35cf027408868cfece53
diff --git a/services/accessibility/accessibility.aconfig b/services/accessibility/accessibility.aconfig
index 0480c22..11189cf 100644
--- a/services/accessibility/accessibility.aconfig
+++ b/services/accessibility/accessibility.aconfig
@@ -25,5 +25,12 @@
name: "send_a11y_events_based_on_state"
namespace: "accessibility"
description: "Sends accessibility events in TouchExplorer#onAccessibilityEvent based on internal state to keep it consistent. This reduces test flakiness."
-bug: "295575684"
-}
\ No newline at end of file
+ bug: "295575684"
+}
+
+flag {
+ name: "add_window_token_without_lock"
+ namespace: "accessibility"
+ description: "Calls WMS.addWindowToken without holding A11yManagerService#mLock"
+ bug: "297972548"
+}
diff --git a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
index 05b6eb4..fa73cff 100644
--- a/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
+++ b/services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java
@@ -1506,11 +1506,17 @@
}
}
- public void onAdded() {
+ /**
+ * Called when the connection is first created. Add a window token for all known displays.
+ * <p>
+ * <strong>Note:</strong> Should not be called while holding the AccessibilityManagerService
+ * lock because this calls out to WindowManagerService.
+ */
+ void addWindowTokensForAllDisplays() {
final Display[] displays = mDisplayManager.getDisplays();
for (int i = 0; i < displays.length; i++) {
final int displayId = displays[i].getDisplayId();
- onDisplayAdded(displayId);
+ addWindowTokenForDisplay(displayId);
}
}
@@ -1518,9 +1524,13 @@
* Called whenever a logical display has been added to the system. Add a window token for adding
* an accessibility overlay.
*
+ * <p>
+ * <strong>Note:</strong> Should not be called while holding the AccessibilityManagerService
+ * lock because this calls out to WindowManagerService.
+ *
* @param displayId The id of the logical display that was added.
*/
- public void onDisplayAdded(int displayId) {
+ void addWindowTokenForDisplay(int displayId) {
final long identity = Binder.clearCallingIdentity();
try {
final IBinder overlayWindowToken = new Binder();
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
index 93ba362..60d4ee6 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java
@@ -154,6 +154,7 @@
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.IntPair;
+import com.android.internal.util.Preconditions;
import com.android.server.AccessibilityManagerInternal;
import com.android.server.LocalServices;
import com.android.server.SystemService;
@@ -4500,6 +4501,20 @@
private int mSystemUiUid = 0;
AccessibilityDisplayListener(Context context, Handler handler) {
+ if (Flags.addWindowTokenWithoutLock()) {
+ // Avoid concerns about one thread adding displays while another thread removes
+ // them by ensuring the looper is the main looper and the DisplayListener
+ // callbacks are always executed on the one main thread.
+ final boolean isMainHandler = handler.getLooper() == Looper.getMainLooper();
+ final String errorMessage =
+ "AccessibilityDisplayListener must use the main handler";
+ if (Build.IS_USERDEBUG || Build.IS_ENG) {
+ Preconditions.checkArgument(isMainHandler, errorMessage);
+ } else if (!isMainHandler) {
+ Slog.e(LOG_TAG, errorMessage);
+ }
+ }
+
mDisplayManager = (DisplayManager) context.getSystemService(Context.DISPLAY_SERVICE);
mDisplayManager.registerDisplayListener(this, handler);
initializeDisplayList();
@@ -4541,11 +4556,21 @@
@Override
public void onDisplayAdded(int displayId) {
+ if (Flags.addWindowTokenWithoutLock()) {
+ final boolean isMainThread = Looper.getMainLooper().isCurrentThread();
+ final String errorMessage = "onDisplayAdded must be called from the main thread";
+ if (Build.IS_USERDEBUG || Build.IS_ENG) {
+ Preconditions.checkArgument(isMainThread, errorMessage);
+ } else if (!isMainThread) {
+ Slog.e(LOG_TAG, errorMessage);
+ }
+ }
final Display display = mDisplayManager.getDisplay(displayId);
if (!isValidDisplay(display)) {
return;
}
+ final List<AccessibilityServiceConnection> services;
synchronized (mLock) {
mDisplaysList.add(display);
mA11yOverlayLayers.put(
@@ -4554,21 +4579,42 @@
mInputFilter.onDisplayAdded(display);
}
AccessibilityUserState userState = getCurrentUserStateLocked();
- if (displayId != Display.DEFAULT_DISPLAY) {
- final List<AccessibilityServiceConnection> services = userState.mBoundServices;
- for (int i = 0; i < services.size(); i++) {
- AccessibilityServiceConnection boundClient = services.get(i);
- boundClient.onDisplayAdded(displayId);
+ if (Flags.addWindowTokenWithoutLock()) {
+ services = new ArrayList<>(userState.mBoundServices);
+ } else {
+ services = userState.mBoundServices;
+ if (displayId != Display.DEFAULT_DISPLAY) {
+ for (int i = 0; i < services.size(); i++) {
+ AccessibilityServiceConnection boundClient = services.get(i);
+ boundClient.addWindowTokenForDisplay(displayId);
+ }
}
}
updateMagnificationLocked(userState);
updateWindowsForAccessibilityCallbackLocked(userState);
notifyClearAccessibilityCacheLocked();
}
+ if (Flags.addWindowTokenWithoutLock()) {
+ if (displayId != Display.DEFAULT_DISPLAY) {
+ for (int i = 0; i < services.size(); i++) {
+ AccessibilityServiceConnection boundClient = services.get(i);
+ boundClient.addWindowTokenForDisplay(displayId);
+ }
+ }
+ }
}
@Override
public void onDisplayRemoved(int displayId) {
+ if (Flags.addWindowTokenWithoutLock()) {
+ final boolean isMainThread = Looper.getMainLooper().isCurrentThread();
+ final String errorMessage = "onDisplayRemoved must be called from the main thread";
+ if (Build.IS_USERDEBUG || Build.IS_ENG) {
+ Preconditions.checkArgument(isMainThread, errorMessage);
+ } else if (!isMainThread) {
+ Slog.e(LOG_TAG, errorMessage);
+ }
+ }
synchronized (mLock) {
if (!removeDisplayFromList(displayId)) {
return;
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
index 9e70073..7a2a602 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java
@@ -44,7 +44,6 @@
import android.view.Display;
import android.view.MotionEvent;
-
import com.android.server.inputmethod.InputMethodManagerInternal;
import com.android.server.wm.ActivityTaskManagerInternal;
import com.android.server.wm.WindowManagerInternal;
@@ -169,6 +168,10 @@
@Override
public void onServiceConnected(ComponentName componentName, IBinder service) {
+ AccessibilityUserState userState = mUserStateWeakReference.get();
+ if (userState != null && Flags.addWindowTokenWithoutLock()) {
+ addWindowTokensForAllDisplays();
+ }
synchronized (mLock) {
if (mService != service) {
if (mService != null) {
@@ -184,7 +187,6 @@
}
}
mServiceInterface = IAccessibilityServiceClient.Stub.asInterface(service);
- AccessibilityUserState userState = mUserStateWeakReference.get();
if (userState == null) return;
userState.addServiceLocked(this);
mSystemSupport.onClientChangeLocked(false);
diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityUserState.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityUserState.java
index ab6cc71..693526a 100644
--- a/services/accessibility/java/com/android/server/accessibility/AccessibilityUserState.java
+++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityUserState.java
@@ -224,7 +224,9 @@
void addServiceLocked(AccessibilityServiceConnection serviceConnection) {
if (!mBoundServices.contains(serviceConnection)) {
- serviceConnection.onAdded();
+ if (!Flags.addWindowTokenWithoutLock()) {
+ serviceConnection.addWindowTokensForAllDisplays();
+ }
mBoundServices.add(serviceConnection);
mComponentNameToServiceMap.put(serviceConnection.getComponentName(), serviceConnection);
mServiceInfoChangeListener.onServiceInfoChangedLocked(this);
diff --git a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java
index 208acdf..53c629a 100644
--- a/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java
+++ b/services/accessibility/java/com/android/server/accessibility/UiAutomationManager.java
@@ -25,9 +25,11 @@
import android.content.ComponentName;
import android.content.Context;
import android.os.Binder;
+import android.os.Build;
import android.os.Handler;
import android.os.IBinder;
import android.os.IBinder.DeathRecipient;
+import android.os.Looper;
import android.os.RemoteCallback;
import android.os.RemoteException;
import android.util.Slog;
@@ -35,6 +37,7 @@
import android.view.accessibility.AccessibilityEvent;
import com.android.internal.util.DumpUtils;
+import com.android.internal.util.Preconditions;
import com.android.server.utils.Slogf;
import com.android.server.wm.WindowManagerInternal;
@@ -98,46 +101,47 @@
accessibilityServiceInfo.setComponentName(COMPONENT_NAME);
Slogf.i(LOG_TAG, "Registering UiTestAutomationService (id=%s) when called by user %d",
accessibilityServiceInfo.getId(), Binder.getCallingUserHandle().getIdentifier());
- synchronized (mLock) {
- if (mUiAutomationService != null) {
- throw new IllegalStateException(
- "UiAutomationService " + mUiAutomationService.mServiceInterface
- + "already registered!");
- }
-
- try {
- owner.linkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);
- } catch (RemoteException re) {
- Slog.e(LOG_TAG, "Couldn't register for the death of a UiTestAutomationService!",
- re);
- return;
- }
-
- mUiAutomationFlags = flags;
- mSystemSupport = systemSupport;
- // Ignore registering UiAutomation if it is not allowed to use the accessibility
- // subsystem.
- if (!useAccessibility()) {
- return;
- }
- mUiAutomationService = new UiAutomationService(context, accessibilityServiceInfo, id,
- mainHandler, mLock, securityPolicy, systemSupport, trace, windowManagerInternal,
- systemActionPerformer, awm);
- mUiAutomationServiceOwner = owner;
- mUiAutomationService.mServiceInterface = serviceClient;
- try {
- mUiAutomationService.mServiceInterface.asBinder().linkToDeath(mUiAutomationService,
- 0);
- } catch (RemoteException re) {
- Slog.e(LOG_TAG, "Failed registering death link: " + re);
- destroyUiAutomationService();
- return;
- }
-
- mUiAutomationService.onAdded();
-
- mUiAutomationService.connectServiceUnknownThread();
+ if (mUiAutomationService != null) {
+ throw new IllegalStateException(
+ "UiAutomationService " + mUiAutomationService.mServiceInterface
+ + "already registered!");
}
+
+ try {
+ owner.linkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);
+ } catch (RemoteException re) {
+ Slog.e(LOG_TAG, "Couldn't register for the death of a UiTestAutomationService!",
+ re);
+ return;
+ }
+
+ mUiAutomationFlags = flags;
+ mSystemSupport = systemSupport;
+ // Ignore registering UiAutomation if it is not allowed to use the accessibility
+ // subsystem.
+ if (!useAccessibility()) {
+ return;
+ }
+ mUiAutomationService = new UiAutomationService(context, accessibilityServiceInfo, id,
+ mainHandler, mLock, securityPolicy, systemSupport, trace, windowManagerInternal,
+ systemActionPerformer, awm);
+ mUiAutomationServiceOwner = owner;
+ mUiAutomationService.mServiceInterface = serviceClient;
+ try {
+ mUiAutomationService.mServiceInterface.asBinder().linkToDeath(mUiAutomationService,
+ 0);
+ } catch (RemoteException re) {
+ Slog.e(LOG_TAG, "Failed registering death link: " + re);
+ destroyUiAutomationService();
+ return;
+ }
+
+ if (!Flags.addWindowTokenWithoutLock()) {
+ mUiAutomationService.addWindowTokensForAllDisplays();
+ }
+ // UiAutomationService#connectServiceUnknownThread posts to a handler
+ // so this call should return immediately.
+ mUiAutomationService.connectServiceUnknownThread();
}
void unregisterUiTestAutomationServiceLocked(IAccessibilityServiceClient serviceClient) {
@@ -253,6 +257,13 @@
super(context, COMPONENT_NAME, accessibilityServiceInfo, id, mainHandler, lock,
securityPolicy, systemSupport, trace, windowManagerInternal,
systemActionPerformer, awm);
+ final boolean isMainHandler = mainHandler.getLooper() == Looper.getMainLooper();
+ final String errorMessage = "UiAutomationService must use the main handler";
+ if (Build.IS_USERDEBUG || Build.IS_ENG) {
+ Preconditions.checkArgument(isMainHandler, errorMessage);
+ } else if (!isMainHandler) {
+ Slog.e(LOG_TAG, errorMessage);
+ }
mMainHandler = mainHandler;
setDisplayTypes(DISPLAY_TYPE_DEFAULT | DISPLAY_TYPE_PROXY);
}
@@ -274,6 +285,9 @@
// If the serviceInterface is null, the UiAutomation has been shut down on
// another thread.
if (serviceInterface != null) {
+ if (Flags.addWindowTokenWithoutLock()) {
+ mUiAutomationService.addWindowTokensForAllDisplays();
+ }
if (mTrace.isA11yTracingEnabledForTypes(
AccessibilityTrace.FLAGS_ACCESSIBILITY_SERVICE_CLIENT)) {
mTrace.logTrace("UiAutomationService.connectServiceUnknownThread",
@@ -286,7 +300,7 @@
mOverlayWindowTokens.get(Display.DEFAULT_DISPLAY));
}
} catch (RemoteException re) {
- Slog.w(LOG_TAG, "Error initialized connection", re);
+ Slog.w(LOG_TAG, "Error initializing connection", re);
destroyUiAutomationService();
}
});