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

Commit dd577717 authored by Phil Weaver's avatar Phil Weaver
Browse files

Add synchronization to UiAutomationManager. 

The client change method needs to be called with a lock
held, and there's a one-off bug that looks like two threads
were shutting down UiAutomation simultaneously. Adding
a lock to prevent this from happening.

Bug: 111170405
Bug: 110845380
Test: A11y CTS and unit tests. I can't repro the issue, so
I can't write a test that specifically causes the problem
that this test makes pass.

Change-Id: Ia01603fcca5bcee70efd24e7af667d47d3057d61
parent 64c4523e

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

+2 −2
Original line number Diff line number Diff line
@@ -216,7 +216,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ 
         * Called back to notify system that the client has changed

         * @param serviceInfoChanged True if the service's AccessibilityServiceInfo changed.

         */

        void onClientChange(boolean serviceInfoChanged);

        void onClientChangeLocked(boolean serviceInfoChanged);



        int getCurrentUserIdLocked();
@@ -363,7 +363,7 @@ abstract class AbstractAccessibilityServiceConnection extends IAccessibilityServ 
                } else {

                    setDynamicallyConfigurableProperties(info);

                }

                mSystemSupport.onClientChange(true);

                mSystemSupport.onClientChangeLocked(true);

            }

        } finally {

            Binder.restoreCallingIdentity(identity);

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

+3 −3
Original line number Diff line number Diff line
@@ -254,7 +254,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 


    private final UserManager mUserManager;



    private final UiAutomationManager mUiAutomationManager = new UiAutomationManager();

    private final UiAutomationManager mUiAutomationManager = new UiAutomationManager(mLock);



    private int mCurrentUserId = UserHandle.USER_SYSTEM;
@@ -833,7 +833,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 


        synchronized (mLock) {

            mUiAutomationManager.registerUiTestAutomationServiceLocked(owner, serviceClient,

                    mContext, accessibilityServiceInfo, sIdCounter++, mMainHandler, mLock,

                    mContext, accessibilityServiceInfo, sIdCounter++, mMainHandler,

                    mSecurityPolicy, this, mWindowManagerService, mGlobalActionPerformer, flags);

            onUserStateChangedLocked(getCurrentUserStateLocked());

        }
@@ -2790,7 +2790,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub 
    }



    @Override

    public void onClientChange(boolean serviceInfoChanged) {

    public void onClientChangeLocked(boolean serviceInfoChanged) {

        AccessibilityManagerService.UserState userState = getUserStateLocked(mCurrentUserId);

        onUserStateChangedLocked(userState);

        if (serviceInfoChanged) {

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

+3 −5
Original line number Diff line number Diff line
@@ -16,8 +16,6 @@ 


package com.android.server.accessibility;



import static android.provider.Settings.Secure.SHOW_MODE_AUTO;



import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage;



import android.accessibilityservice.AccessibilityServiceInfo;
@@ -133,7 +131,7 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect 
                } finally {

                    Binder.restoreCallingIdentity(identity);

                }

                mSystemSupport.onClientChange(false);

                mSystemSupport.onClientChangeLocked(false);

            }

        }

    }
@@ -158,7 +156,7 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect 
            UserState userState = mUserStateWeakReference.get();

            if (userState == null) return;

            userState.addServiceLocked(this);

            mSystemSupport.onClientChange(false);

            mSystemSupport.onClientChangeLocked(false);

            // Initialize the service on the main handler after we're done setting up for

            // the new configuration (for example, initializing the input filter).

            mMainHandler.sendMessage(obtainMessage(
@@ -259,7 +257,7 @@ class AccessibilityServiceConnection extends AbstractAccessibilityServiceConnect 
                userState.serviceDisconnectedLocked(this);

            }

            mSystemSupport.getMagnificationController().resetIfNeeded(mId);

            mSystemSupport.onClientChange(false);

            mSystemSupport.onClientChangeLocked(false);

        }

    }

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

+80 −53
Original line number Diff line number Diff line
@@ -43,6 +43,8 @@ class UiAutomationManager { 
            new ComponentName("com.android.server.accessibility", "UiAutomation");

    private static final String LOG_TAG = "UiAutomationManager";



    private final Object mLock;



    private UiAutomationService mUiAutomationService;



    private AccessibilityServiceInfo mUiAutomationServiceInfo;
@@ -51,6 +53,10 @@ class UiAutomationManager { 


    private int mUiAutomationFlags;



    UiAutomationManager(Object lock) {

        mLock = lock;

    }



    private IBinder mUiAutomationServiceOwner;

    private final DeathRecipient mUiAutomationServiceOwnerDeathRecipient =

            new DeathRecipient() {
@@ -77,11 +83,12 @@ class UiAutomationManager { 
    void registerUiTestAutomationServiceLocked(IBinder owner,

            IAccessibilityServiceClient serviceClient,

            Context context, AccessibilityServiceInfo accessibilityServiceInfo,

            int id, Handler mainHandler, Object lock,

            int id, Handler mainHandler,

            AccessibilityManagerService.SecurityPolicy securityPolicy,

            AbstractAccessibilityServiceConnection.SystemSupport systemSupport,

            WindowManagerInternal windowManagerInternal,

            GlobalActionPerformer globalActionPerfomer, int flags) {

        synchronized (mLock) {

            accessibilityServiceInfo.setComponentName(COMPONENT_NAME);



            if (mUiAutomationService != null) {
@@ -92,13 +99,14 @@ class UiAutomationManager { 
            try {

                owner.linkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);

            } catch (RemoteException re) {

            Slog.e(LOG_TAG, "Couldn't register for the death of a UiTestAutomationService!", re);

                Slog.e(LOG_TAG, "Couldn't register for the death of a UiTestAutomationService!",

                        re);

                return;

            }



            mSystemSupport = systemSupport;

            mUiAutomationService = new UiAutomationService(context, accessibilityServiceInfo, id,

                mainHandler, lock, securityPolicy, systemSupport, windowManagerInternal,

                    mainHandler, mLock, securityPolicy, systemSupport, windowManagerInternal,

                    globalActionPerfomer);

            mUiAutomationServiceOwner = owner;

            mUiAutomationFlags = flags;
@@ -106,7 +114,8 @@ class UiAutomationManager { 
            mUiAutomationService.mServiceInterface = serviceClient;

            mUiAutomationService.onAdded();

            try {

            mUiAutomationService.mServiceInterface.asBinder().linkToDeath(mUiAutomationService, 0);

                mUiAutomationService.mServiceInterface.asBinder().linkToDeath(mUiAutomationService,

                        0);

            } catch (RemoteException re) {

                Slog.e(LOG_TAG, "Failed registering death link: " + re);

                destroyUiAutomationService();
@@ -115,8 +124,10 @@ class UiAutomationManager { 


            mUiAutomationService.connectServiceUnknownThread();

        }

    }



    void unregisterUiTestAutomationServiceLocked(IAccessibilityServiceClient serviceClient) {

        synchronized (mLock) {

            if ((mUiAutomationService == null)

                    || (serviceClient == null)

                    || (mUiAutomationService.mServiceInterface == null)
@@ -128,6 +139,7 @@ class UiAutomationManager { 


            destroyUiAutomationService();

        }

    }



    void sendAccessibilityEventLocked(AccessibilityEvent event) {

        if (mUiAutomationService != null) {
@@ -159,24 +171,38 @@ class UiAutomationManager { 
    }



    int getRelevantEventTypes() {

        if (mUiAutomationService == null) return 0;

        return mUiAutomationService.getRelevantEventTypes();

        UiAutomationService uiAutomationService;

        synchronized (mLock) {

            uiAutomationService = mUiAutomationService;

        }

        if (uiAutomationService == null) return 0;

        return uiAutomationService.getRelevantEventTypes();

    }



    @Nullable

    AccessibilityServiceInfo getServiceInfo() {

        if (mUiAutomationService == null) return null;

        return mUiAutomationService.getServiceInfo();

        UiAutomationService uiAutomationService;

        synchronized (mLock) {

            uiAutomationService = mUiAutomationService;

        }

        if (uiAutomationService == null) return null;

        return uiAutomationService.getServiceInfo();

    }



    void dumpUiAutomationService(FileDescriptor fd, final PrintWriter pw, String[] args) {

        if (mUiAutomationService != null) {

            mUiAutomationService.dump(fd, pw, args);

        UiAutomationService uiAutomationService;

        synchronized (mLock) {

            uiAutomationService = mUiAutomationService;

        }

        if (uiAutomationService != null) {

            uiAutomationService.dump(fd, pw, args);

        }

    }



    private void destroyUiAutomationService() {

        mUiAutomationService.mServiceInterface.asBinder().unlinkToDeath(mUiAutomationService, 0);

        synchronized (mLock) {

            mUiAutomationService.mServiceInterface.asBinder().unlinkToDeath(mUiAutomationService,

                    0);

            mUiAutomationService.onRemoved();

            mUiAutomationService.resetLocked();

            mUiAutomationService = null;
@@ -185,7 +211,8 @@ class UiAutomationManager { 
                mUiAutomationServiceOwner.unlinkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);

                mUiAutomationServiceOwner = null;

            }

        mSystemSupport.onClientChange(false);

            mSystemSupport.onClientChangeLocked(false);

        }

    }



    private class UiAutomationService extends AbstractAccessibilityServiceConnection {

services/tests/servicestests/src/com/android/server/accessibility/UiAutomationManagerTest.java

+3 −3
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ import org.mockito.MockitoAnnotations; 
public class UiAutomationManagerTest {

    static final int SERVICE_ID = 42;



    final UiAutomationManager mUiAutomationManager = new UiAutomationManager();

    final UiAutomationManager mUiAutomationManager = new UiAutomationManager(new Object());



    MessageCapturingHandler mMessageCapturingHandler;
@@ -158,13 +158,13 @@ public class UiAutomationManagerTest { 
        verify(mMockOwner).linkToDeath(captor.capture(), anyInt());

        captor.getValue().binderDied();

        mMessageCapturingHandler.sendAllMessages();

        verify(mMockSystemSupport).onClientChange(false);

        verify(mMockSystemSupport).onClientChangeLocked(false);

    }



    private void register(int flags) {

        mUiAutomationManager.registerUiTestAutomationServiceLocked(mMockOwner,

                mMockAccessibilityServiceClient, mMockContext, mMockServiceInfo, SERVICE_ID,

                mMessageCapturingHandler, new Object(), mMockSecurityPolicy, mMockSystemSupport,

                mMessageCapturingHandler, mMockSecurityPolicy, mMockSystemSupport,

                mMockWindowManagerInternal, mMockGlobalActionPerformer, flags);

    }