From 0c56d28a197c53740c54442cb3855f36a347a975 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Mar 2020 11:39:45 +0800 Subject: [PATCH] Allow #disconnect to be called safely on connection timeout (2/n) Now #disconnect won't throw if the previous #connect failed due to timeout. Note we also introduce generation id so we won't receive notifications or modifications from the previous client to disrupt UiAutomation's status when making the next connection. Bug: 147785023 Test: m Change-Id: Idf77207124494bd78770b8ea5d9ac4b1fd1a490a Merged-In: Idf77207124494bd78770b8ea5d9ac4b1fd1a490a --- core/java/android/app/UiAutomation.java | 106 +++++++++++++++++------- 1 file changed, 75 insertions(+), 31 deletions(-) diff --git a/core/java/android/app/UiAutomation.java b/core/java/android/app/UiAutomation.java index 0db0ed9976185..e0951bf3f4d28 100644 --- a/core/java/android/app/UiAutomation.java +++ b/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) { - if (isConnectedLocked()) { - break; - } - final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis; - final long remainingTimeMillis = timeoutMillis - elapsedTimeMillis; - if (remainingTimeMillis <= 0) { - throw new TimeoutException("Timeout while connecting " + this); - } - try { - mLock.wait(remainingTimeMillis); - } catch (InterruptedException ie) { - /* ignore */ - } + while (true) { + if (mConnectionState == ConnectionState.CONNECTED) { + break; + } + final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis; + final long remainingTimeMillis = timeoutMillis - elapsedTimeMillis; + if (remainingTimeMillis <= 0) { + mConnectionState = ConnectionState.FAILED; + throw new TimeoutException("Timeout while connecting " + this); + } + try { + mLock.wait(remainingTimeMillis); + } catch (InterruptedException ie) { + /* 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) { - throw new IllegalStateException("UiAutomation not connected, " + this); + if (mConnectionState == ConnectionState.CONNECTED) { + 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));