Handle UiAutomation dying before being connected

There was an opportunity for the UiAutomation's binder
to die before it was connected, which caused an
NPE when it tried to connect.

I also moved the message handling to a different handler
to make it easier to test.

Bug: 65474486
Test: UiAutomation CTS passes. I also added a unit test
that failed in the same way as the bug. That test now
passes.

Change-Id: I31036ace114b21fed64227b62212b95267039d9e

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

@@ -47,7 +47,7 @@ class UiAutomationManager {
     private int mUiAutomationFlags;
 
     private IBinder mUiAutomationServiceOwner;
-    private final DeathRecipient mUiAutomationSerivceOwnerDeathRecipient =
+    private final DeathRecipient mUiAutomationServiceOwnerDeathRecipient =
             new DeathRecipient() {
                 @Override
                 public void binderDied() {
@@ -85,7 +85,7 @@ class UiAutomationManager {
         }
 
         try {
-            owner.linkToDeath(mUiAutomationSerivceOwnerDeathRecipient, 0);
+            owner.linkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);
         } catch (RemoteException re) {
             Slog.e(LOG_TAG, "Couldn't register for the death of a UiTestAutomationService!", re);
             return;
@@ -165,12 +165,14 @@ class UiAutomationManager {
         mUiAutomationService = null;
         mUiAutomationFlags = 0;
         if (mUiAutomationServiceOwner != null) {
-            mUiAutomationServiceOwner.unlinkToDeath(mUiAutomationSerivceOwnerDeathRecipient, 0);
+            mUiAutomationServiceOwner.unlinkToDeath(mUiAutomationServiceOwnerDeathRecipient, 0);
             mUiAutomationServiceOwner = null;
         }
     }
 
     private class UiAutomationService extends AccessibilityClientConnection {
+        private final Handler mMainHandler;
+
         UiAutomationService(Context context, AccessibilityServiceInfo accessibilityServiceInfo,
                 int id, Handler mainHandler, Object lock,
                 AccessibilityManagerService.SecurityPolicy securityPolicy,
@@ -178,15 +180,26 @@ class UiAutomationManager {
                 GlobalActionPerformer globalActionPerfomer) {
             super(context, COMPONENT_NAME, accessibilityServiceInfo, id, mainHandler, lock,
                     securityPolicy, systemSupport, windowManagerInternal, globalActionPerfomer);
+            mMainHandler = mainHandler;
         }
 
         void connectServiceUnknownThread() {
             // This needs to be done on the main thread
-            mEventDispatchHandler.post(() -> {
+            mMainHandler.post(() -> {
                 try {
-                    mService = mServiceInterface.asBinder();
-                    mService.linkToDeath(this, 0);
-                    mServiceInterface.init(this, mId, mOverlayWindowToken);
+                    final IAccessibilityServiceClient serviceInterface;
+                    final IBinder service;
+                    synchronized (mLock) {
+                        serviceInterface = mServiceInterface;
+                        mService = (serviceInterface == null) ? null : mServiceInterface.asBinder();
+                        service = mService;
+                    }
+                    // If the serviceInterface is null, the UiAutomation has been shut down on
+                    // another thread.
+                    if (serviceInterface != null) {
+                        service.linkToDeath(this, 0);
+                        serviceInterface.init(this, mId, mOverlayWindowToken);
+                    }
                 } catch (RemoteException re) {
                     Slog.w(LOG_TAG, "Error initialized connection", re);
                     destroyUiAutomationService();

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

@@ -16,6 +16,7 @@
 
 package com.android.server.accessibility;
 
+import android.app.Notification;
 import android.os.Handler;
 import android.os.Message;
 import android.util.Pair;
@@ -49,7 +50,7 @@ public class MessageCapturingHandler extends Handler {
     public void sendOneMessage() {
         Message message = timedMessages.remove(0).first;
         removeMessages(message.what, message.obj);
-        mCallback.handleMessage(message);
+        dispatchMessage(message);
         removeStaleMessages();
     }
 
@@ -62,7 +63,7 @@ public class MessageCapturingHandler extends Handler {
     public void sendLastMessage() {
         Message message = timedMessages.remove(timedMessages.size() - 1).first;
         removeMessages(message.what, message.obj);
-        mCallback.handleMessage(message);
+        dispatchMessage(message);
         removeStaleMessages();
     }
 
@@ -79,4 +80,12 @@ public class MessageCapturingHandler extends Handler {
             }
         }
     }
+
+    public void dispatchMessage(Message m) {
+        if (mCallback != null) {
+            mCallback.handleMessage(m);
+            return;
+        }
+        super.dispatchMessage(m);
+    }
 }

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

@@ -19,7 +19,9 @@ package com.android.server.accessibility;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import android.accessibilityservice.AccessibilityServiceInfo;
@@ -29,7 +31,6 @@ import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.ResolveInfo;
 import android.content.pm.ServiceInfo;
-import android.os.Handler;
 import android.os.IBinder;
 import android.os.Looper;
 import android.view.WindowManagerInternal;
@@ -38,6 +39,7 @@ import android.view.accessibility.AccessibilityEvent;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
@@ -49,6 +51,8 @@ public class UiAutomationManagerTest {
 
     final UiAutomationManager mUiAutomationManager = new UiAutomationManager();
 
+    MessageCapturingHandler mMessageCapturingHandler;
+
     @Mock AccessibilityManagerService.UserState mMockUserState;
     @Mock Context mMockContext;
     @Mock AccessibilityServiceInfo mMockServiceInfo;
@@ -68,7 +72,6 @@ public class UiAutomationManagerTest {
         }
     }
 
-
     @Before
     public void setup() {
         MockitoAnnotations.initMocks(this);
@@ -80,6 +83,8 @@ public class UiAutomationManagerTest {
         mMockResolveInfo.serviceInfo.applicationInfo = mock(ApplicationInfo.class);
 
         when(mMockAccessibilityServiceClient.asBinder()).thenReturn(mMockServiceAsBinder);
+
+        mMessageCapturingHandler = new MessageCapturingHandler(null);
     }
 
     @Test
@@ -146,10 +151,20 @@ public class UiAutomationManagerTest {
         assertEquals(0, mUiAutomationManager.getRequestedEventMaskLocked());
     }
 
+    @Test
+    public void uiAutomationBinderDiesBeforeConnecting_shouldNotCrash() throws Exception {
+        register(0);
+        ArgumentCaptor<IBinder.DeathRecipient> captor = ArgumentCaptor.forClass(
+                IBinder.DeathRecipient.class);
+        verify(mMockOwner).linkToDeath(captor.capture(), anyInt());
+        captor.getValue().binderDied();
+        mMessageCapturingHandler.sendAllMessages();
+    }
+
     private void register(int flags) {
         mUiAutomationManager.registerUiTestAutomationServiceLocked(mMockOwner,
                 mMockAccessibilityServiceClient, mMockContext, mMockServiceInfo, SERVICE_ID,
-                new Handler(), new Object(), mMockSecurityPolicy, mMockSystemSupport,
+                mMessageCapturingHandler, new Object(), mMockSecurityPolicy, mMockSystemSupport,
                 mMockWindowManagerInternal, mMockGlobalActionPerformer, flags);
     }