Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ec42a60b authored by Daniel Norman's avatar Daniel Norman
Browse files

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
parent 1af65858

services/accessibility/accessibility.aconfig

+9 −2
Original line number Diff line number Diff line
@@ -27,3 +27,10 @@ flag { 
    description: "Sends accessibility events in TouchExplorer#onAccessibilityEvent based on internal state to keep it consistent. This reduces test flakiness."

    bug: "295575684"

}



flag {

    name: "add_window_token_without_lock"

    namespace: "accessibility"

    description: "Calls WMS.addWindowToken without holding A11yManagerService#mLock"

    bug: "297972548"

}

services/accessibility/java/com/android/server/accessibility/AbstractAccessibilityServiceConnection.java

+13 −3
Original line number Diff line number Diff line
@@ -1506,11 +1506,17 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ 
        }

    }



    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 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ 
     * 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();

services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java

+51 −5
Original line number Diff line number Diff line
@@ -154,6 +154,7 @@ import com.android.internal.os.BackgroundThread; 
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 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 
        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 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 


        @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 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 
                    mInputFilter.onDisplayAdded(display);

                }

                AccessibilityUserState userState = getCurrentUserStateLocked();

                if (Flags.addWindowTokenWithoutLock()) {

                    services = new ArrayList<>(userState.mBoundServices);

                } else {

                    services = userState.mBoundServices;

                    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);

                            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;

services/accessibility/java/com/android/server/accessibility/AccessibilityServiceConnection.java

+4 −2
Original line number Diff line number Diff line
@@ -44,7 +44,6 @@ import android.util.Slog; 
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 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect 


    @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 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect 
                }

            }

            mServiceInterface = IAccessibilityServiceClient.Stub.asInterface(service);

            AccessibilityUserState userState = mUserStateWeakReference.get();

            if (userState == null) return;

            userState.addServiceLocked(this);

            mSystemSupport.onClientChangeLocked(false);

services/accessibility/java/com/android/server/accessibility/AccessibilityUserState.java

+3 −1
Original line number Diff line number Diff line
@@ -224,7 +224,9 @@ class AccessibilityUserState { 


    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);