Allow #disconnect to be called safely on connection timeout (2/n) am: 0c56d28a19 am: 68569ea618 am: 125259951c

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1470701

Change-Id: I45169e0e371bd0f110b3da9542dac9b618b91f21

core/java/android/app/UiAutomation.java

@@ -22,6 +22,7 @@ import android.accessibilityservice.AccessibilityService.IAccessibilityServiceCl
 import android.accessibilityservice.AccessibilityServiceInfo;
 import android.accessibilityservice.IAccessibilityServiceClient;
 import android.accessibilityservice.IAccessibilityServiceConnection;
+import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.TestApi;
@@ -60,6 +61,8 @@ import com.android.internal.util.function.pooled.PooledLambda;
 import libcore.io.IoUtils;
 
 import java.io.IOException;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeoutException;
@@ -116,6 +119,28 @@ public final class UiAutomation {
     /** Rotation constant: Freeze rotation to 270 degrees . */
     public static final int ROTATION_FREEZE_270 = Surface.ROTATION_270;
 
+    @Retention(RetentionPolicy.SOURCE)
+    @IntDef(value = {
+            ConnectionState.DISCONNECTED,
+            ConnectionState.CONNECTING,
+            ConnectionState.CONNECTED,
+            ConnectionState.FAILED
+    })
+    private @interface ConnectionState {
+        /** The initial state before {@link #connect} or after {@link #disconnect} is called. */
+        int DISCONNECTED = 0;
+        /**
+         * The temporary state after {@link #connect} is called. Will transition to
+         * {@link #CONNECTED} or {@link #FAILED} depending on whether {@link #connect} succeeds or
+         * not.
+         */
+        int CONNECTING = 1;
+        /** The state when {@link #connect} has succeeded. */
+        int CONNECTED = 2;
+        /** The state when {@link #connect} has failed. */
+        int FAILED = 3;
+    }
+
     /**
      * UiAutomation supresses accessibility services by default. This flag specifies that
      * existing accessibility services should continue to run, and that new ones may start.
@@ -144,12 +169,14 @@ public final class UiAutomation {
 
     private long mLastEventTimeMillis;
 
-    private boolean mIsConnecting;
+    private @ConnectionState int mConnectionState = ConnectionState.DISCONNECTED;
 
     private boolean mIsDestroyed;
 
     private int mFlags;
 
+    private int mGenerationId = 0;
+
     /**
      * Listener for observing the {@link AccessibilityEvent} stream.
      */
@@ -250,13 +277,15 @@ public final class UiAutomation {
     public void connectWithTimeout(int flags, long timeoutMillis) throws TimeoutException {
         synchronized (mLock) {
             throwIfConnectedLocked();
-            if (mIsConnecting) {
+            if (mConnectionState == ConnectionState.CONNECTING) {
                 return;
             }
-            mIsConnecting = true;
+            mConnectionState = ConnectionState.CONNECTING;
             mRemoteCallbackThread = new HandlerThread("UiAutomation");
             mRemoteCallbackThread.start();
-            mClient = new IAccessibilityServiceClientImpl(mRemoteCallbackThread.getLooper());
+            // Increment the generation since we are about to interact with a new client
+            mClient = new IAccessibilityServiceClientImpl(
+                    mRemoteCallbackThread.getLooper(), ++mGenerationId);
         }
 
         try {
@@ -269,24 +298,21 @@ public final class UiAutomation {
 
         synchronized (mLock) {
             final long startTimeMillis = SystemClock.uptimeMillis();
-            try {
+            while (true) {
-                while (true) {
+                if (mConnectionState == ConnectionState.CONNECTED) {
-                    if (isConnectedLocked()) {
+                    break;
-                        break;
+                }
-                    }
+                final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis;
-                    final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis;
+                final long remainingTimeMillis = timeoutMillis - elapsedTimeMillis;
-                    final long remainingTimeMillis = timeoutMillis - elapsedTimeMillis;
+                if (remainingTimeMillis <= 0) {
-                    if (remainingTimeMillis <= 0) {
+                    mConnectionState = ConnectionState.FAILED;
-                        throw new TimeoutException("Timeout while connecting " + this);
+                    throw new TimeoutException("Timeout while connecting " + this);
-                    }
+                }
-                    try {
+                try {
-                        mLock.wait(remainingTimeMillis);
+                    mLock.wait(remainingTimeMillis);
-                    } catch (InterruptedException ie) {
+                } catch (InterruptedException ie) {
-                        /* ignore */
+                    /* ignore */
-                    }
                 }
-            } finally {
-                mIsConnecting = false;
             }
         }
     }
@@ -310,12 +336,17 @@ public final class UiAutomation {
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     public void disconnect() {
         synchronized (mLock) {
-            if (mIsConnecting) {
+            if (mConnectionState == ConnectionState.CONNECTING) {
                 throw new IllegalStateException(
                         "Cannot call disconnect() while connecting " + this);
             }
-            throwIfNotConnectedLocked();
+            if (mConnectionState == ConnectionState.DISCONNECTED) {
+                return;
+            }
+            mConnectionState = ConnectionState.DISCONNECTED;
             mConnectionId = CONNECTION_ID_UNDEFINED;
+            // Increment the generation so we no longer interact with the existing client
+            ++mGenerationId;
         }
         try {
             // Calling out without a lock held.
@@ -1245,18 +1276,14 @@ public final class UiAutomation {
         return stringBuilder.toString();
     }
 
-    private boolean isConnectedLocked() {
-        return mConnectionId != CONNECTION_ID_UNDEFINED;
-    }
-
     private void throwIfConnectedLocked() {
-        if (mConnectionId != CONNECTION_ID_UNDEFINED) {
+        if (mConnectionState == ConnectionState.CONNECTED) {
-            throw new IllegalStateException("UiAutomation not connected, " + this);
+            throw new IllegalStateException("UiAutomation connected, " + this);
         }
     }
 
     private void throwIfNotConnectedLocked() {
-        if (!isConnectedLocked()) {
+        if (mConnectionState != ConnectionState.CONNECTED) {
             throw new IllegalStateException("UiAutomation not connected, " + this);
         }
     }
@@ -1273,11 +1300,25 @@ public final class UiAutomation {
 
     private class IAccessibilityServiceClientImpl extends IAccessibilityServiceClientWrapper {
 
-        public IAccessibilityServiceClientImpl(Looper looper) {
+        public IAccessibilityServiceClientImpl(Looper looper, int generationId) {
             super(null, looper, new Callbacks() {
+                private final int mGenerationId = generationId;
+                /**
+                 * True if UiAutomation doesn't interact with this client anymore.
+                 * Used by methods below to stop sending notifications or changing members
+                 * of {@link UiAutomation}.
+                 */
+                private boolean isGenerationChangedLocked() {
+                    return mGenerationId != UiAutomation.this.mGenerationId;
+                }
+
                 @Override
                 public void init(int connectionId, IBinder windowToken) {
                     synchronized (mLock) {
+                        if (isGenerationChangedLocked()) {
+                            return;
+                        }
+                        mConnectionState = ConnectionState.CONNECTED;
                         mConnectionId = connectionId;
                         mLock.notifyAll();
                     }
@@ -1311,6 +1352,9 @@ public final class UiAutomation {
                 public void onAccessibilityEvent(AccessibilityEvent event) {
                     final OnAccessibilityEventListener listener;
                     synchronized (mLock) {
+                        if (isGenerationChangedLocked()) {
+                            return;
+                        }
                         mLastEventTimeMillis = event.getEventTime();
                         if (mWaitingForEventDelivery) {
                             mEventQueue.add(AccessibilityEvent.obtain(event));