From 67c91c405c04ed9cf10e1d2a23bded50ce94e7a0 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Mar 2020 10:05:42 +0800 Subject: [PATCH 1/3] Add #connectWithTimeout (1/n) Add #connectWithTimeout which allows us to specify the timeout before giving up the connection. The method throws a TimeoutException so the caller can catch it and retry connection again. Note we don't change the exception spec. of #connect in order to be source and binary compatible. Bug: 147785023 Test: m Change-Id: I5ac61ed0aef107f4e38166c0b95bc3a3fb419387 Merged-In: I5ac61ed0aef107f4e38166c0b95bc3a3fb419387 --- core/java/android/app/UiAutomation.java | 31 +++++++++++++++++++++---- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/core/java/android/app/UiAutomation.java b/core/java/android/app/UiAutomation.java index a9a06dabc0491..0db0ed9976185 100644 --- a/core/java/android/app/UiAutomation.java +++ b/core/java/android/app/UiAutomation.java @@ -210,23 +210,44 @@ public final class UiAutomation { } /** - * Connects this UiAutomation to the accessibility introspection APIs with default flags. + * Connects this UiAutomation to the accessibility introspection APIs with default flags + * and default timeout. * * @hide */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) public void connect() { - connect(0); + try { + connectWithTimeout(0, CONNECT_TIMEOUT_MILLIS); + } catch (TimeoutException e) { + throw new RuntimeException(e); + } + } + + /** + * Connects this UiAutomation to the accessibility introspection APIs with default timeout. + * + * @hide + */ + public void connect(int flags) { + try { + connectWithTimeout(flags, CONNECT_TIMEOUT_MILLIS); + } catch (TimeoutException e) { + throw new RuntimeException(e); + } } /** * Connects this UiAutomation to the accessibility introspection APIs. * * @param flags Any flags to apply to the automation as it gets connected + * @param timeoutMillis The wait timeout in milliseconds + * + * @throws TimeoutException If not connected within the timeout * * @hide */ - public void connect(int flags) { + public void connectWithTimeout(int flags, long timeoutMillis) throws TimeoutException { synchronized (mLock) { throwIfConnectedLocked(); if (mIsConnecting) { @@ -254,9 +275,9 @@ public final class UiAutomation { break; } final long elapsedTimeMillis = SystemClock.uptimeMillis() - startTimeMillis; - final long remainingTimeMillis = CONNECT_TIMEOUT_MILLIS - elapsedTimeMillis; + final long remainingTimeMillis = timeoutMillis - elapsedTimeMillis; if (remainingTimeMillis <= 0) { - throw new RuntimeException("Error while connecting " + this); + throw new TimeoutException("Timeout while connecting " + this); } try { mLock.wait(remainingTimeMillis); From 0c56d28a197c53740c54442cb3855f36a347a975 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Mar 2020 11:39:45 +0800 Subject: [PATCH 2/3] 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)); From b5f26d897d16c6533b956e320425139ccdfe19bb Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Mar 2020 17:14:26 +0800 Subject: [PATCH 3/3] Let #getUiAutomation return null if UiAutomation fails to connect (3/n) Now #getUiAutomation returns null if the underlying UiAutomation fails to connect for targetSdkVersion > R. This gives the caller a chance to retry and decide when to give up. Bug: 147785023 Test: presubmit Change-Id: Ibf201fa042b80c067f2a0c3b0bde23adf9c60546 Merged-In: Ibf201fa042b80c067f2a0c3b0bde23adf9c60546 --- core/java/android/app/Instrumentation.java | 29 ++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/core/java/android/app/Instrumentation.java b/core/java/android/app/Instrumentation.java index 721525d9af9d7..2ef147b3e903e 100644 --- a/core/java/android/app/Instrumentation.java +++ b/core/java/android/app/Instrumentation.java @@ -62,6 +62,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeoutException; /** * Base class for implementing application instrumentation code. When running @@ -90,6 +91,8 @@ public class Instrumentation { private static final String TAG = "Instrumentation"; + private static final long CONNECT_TIMEOUT_MILLIS = 5000; + /** * @hide */ @@ -2125,6 +2128,13 @@ public class Instrumentation { * Equivalent to {@code getUiAutomation(0)}. If a {@link UiAutomation} exists with different * flags, the flags on that instance will be changed, and then it will be returned. *

+ *

+ * Compatibility mode: This method is infallible for apps targeted for + * {@link Build.VERSION_CODES#R} and earlier versions; for apps targeted for later versions, it + * will return null if {@link UiAutomation} fails to connect. The caller can check the return + * value and retry on error. + *

+ * * @return The UI automation instance. * * @see UiAutomation @@ -2152,6 +2162,12 @@ public class Instrumentation { * If a {@link UiAutomation} exists with different flags, the flags on that instance will be * changed, and then it will be returned. *

+ *

+ * Compatibility mode: This method is infallible for apps targeted for + * {@link Build.VERSION_CODES#R} and earlier versions; for apps targeted for later versions, it + * will return null if {@link UiAutomation} fails to connect. The caller can check the return + * value and retry on error. + *

* * @param flags The flags to be passed to the UiAutomation, for example * {@link UiAutomation#FLAG_DONT_SUPPRESS_ACCESSIBILITY_SERVICES}. @@ -2173,8 +2189,17 @@ public class Instrumentation { } else { mUiAutomation.disconnect(); } - mUiAutomation.connect(flags); - return mUiAutomation; + if (getTargetContext().getApplicationInfo().targetSdkVersion <= Build.VERSION_CODES.R) { + mUiAutomation.connect(flags); + return mUiAutomation; + } + try { + mUiAutomation.connectWithTimeout(flags, CONNECT_TIMEOUT_MILLIS); + return mUiAutomation; + } catch (TimeoutException e) { + mUiAutomation.destroy(); + mUiAutomation = null; + } } return null; }