From f04786ea0e0378302fcd753e9fb4e366b693ad8a Mon Sep 17 00:00:00 2001 From: mincheli Date: Thu, 3 Feb 2022 02:15:08 +0800 Subject: [PATCH] Waits for windowMagnification connection set when requesting enableWindowMagnification It fixes the low quality CTS test, AccessibilityMagnificationTest. And this fix also makes enableWindowMagnification() a robust API that can be called frequently when the connection is toggled. CL comments and discussion are at ag/16785609 Root cause: 1. when the test cases run iteratively, the test a11y service would be enabled and disabled repeatedly so the windowMagnification connection would also be connected and disconnected repeatedly. And when the test a11y service is enabled in a short time, the connection may not be ready to be established. Solution: 1. So we add a wait lock and notify to unclock after the connection is established. And prevent the duplicated connection request. Bug: 2147470987 Test: atest AccessibilityMagnificationTest --iterations, atest com.android.server.accessibility Change-Id: I6ec22e66796aaf565c10b0fedcd4de359ce5fc7b --- .../AccessibilityManagerService.java | 3 + .../MagnificationController.java | 10 ++ .../magnification/MagnificationProcessor.java | 4 + .../WindowMagnificationManager.java | 99 +++++++++++++++++-- .../statusbar/StatusBarManagerInternal.java | 2 +- .../statusbar/StatusBarManagerService.java | 4 +- .../AccessibilityManagerServiceTest.java | 1 + .../MagnificationControllerTest.java | 4 + .../WindowMagnificationManagerTest.java | 46 +++++++-- 9 files changed, 154 insertions(+), 19 deletions(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 0e99265905119..0ea087d6de0fb 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -2749,6 +2749,9 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub } private void updateWindowMagnificationConnectionIfNeeded(AccessibilityUserState userState) { + if (!mMagnificationController.supportWindowMagnification()) { + return; + } final boolean connect = (userState.isShortcutMagnificationEnabledLocked() || userState.isDisplayMagnificationEnabledLocked()) && (userState.getMagnificationCapabilitiesLocked() diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java index 09e82c787c904..d57cc6b6ef850 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java @@ -17,6 +17,7 @@ package com.android.server.accessibility.magnification; import static android.accessibilityservice.MagnificationConfig.MAGNIFICATION_MODE_WINDOW; +import static android.content.pm.PackageManager.FEATURE_WINDOW_MAGNIFICATION; import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_ALL; import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN; import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_NONE; @@ -91,6 +92,8 @@ public class MagnificationController implements WindowMagnificationManager.Callb private FullScreenMagnificationController mFullScreenMagnificationController; private WindowMagnificationManager mWindowMagnificationMgr; private int mMagnificationCapabilities = ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN; + /** Whether the platform supports window magnification feature. */ + private final boolean mSupportWindowMagnification; @GuardedBy("mLock") private int mActivatedMode = ACCESSIBILITY_MAGNIFICATION_MODE_NONE; @@ -129,6 +132,8 @@ public class MagnificationController implements WindowMagnificationManager.Callb mScaleProvider = scaleProvider; LocalServices.getService(WindowManagerInternal.class) .getAccessibilityController().setUiChangesForAccessibilityCallbacks(this); + mSupportWindowMagnification = context.getPackageManager().hasSystemFeature( + FEATURE_WINDOW_MAGNIFICATION); } @VisibleForTesting @@ -185,6 +190,11 @@ public class MagnificationController implements WindowMagnificationManager.Callb } } + /** Returns {@code true} if the platform supports window magnification feature. */ + public boolean supportWindowMagnification() { + return mSupportWindowMagnification; + } + /** * Transitions to the target Magnification mode with current center of the magnification mode * if it is available. diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationProcessor.java b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationProcessor.java index 175182c7a3bb8..3e07b095fd29c 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationProcessor.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationProcessor.java @@ -396,6 +396,10 @@ public class MagnificationProcessor { dumpTrackingTypingFocusEnabledState(pw, displayId, config.getMode()); } + pw.append(" SupportWindowMagnification=" + + mController.supportWindowMagnification()).println(); + pw.append(" WindowMagnificationConnectionState=" + + mController.getWindowMagnificationMgr().getConnectionState()).println(); } private int getIdOfLastServiceToMagnify(int mode, int displayId) { diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/WindowMagnificationManager.java b/services/accessibility/java/com/android/server/accessibility/magnification/WindowMagnificationManager.java index 89910eac06c50..0844d126f7632 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/WindowMagnificationManager.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/WindowMagnificationManager.java @@ -36,6 +36,7 @@ import android.graphics.Region; import android.os.Binder; import android.os.IBinder; import android.os.RemoteException; +import android.os.SystemClock; import android.util.Slog; import android.util.SparseArray; import android.view.MotionEvent; @@ -87,6 +88,30 @@ public class WindowMagnificationManager implements }) public @interface WindowPosition {} + /** Window magnification connection is connecting. */ + private static final int CONNECTING = 0; + /** Window magnification connection is connected. */ + private static final int CONNECTED = 1; + /** Window magnification connection is disconnecting. */ + private static final int DISCONNECTING = 2; + /** Window magnification connection is disconnected. */ + private static final int DISCONNECTED = 3; + + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = {"CONNECTION_STATE"}, value = { + CONNECTING, + CONNECTED, + DISCONNECTING, + DISCONNECTED + }) + private @interface ConnectionState { + } + + @ConnectionState + private int mConnectionState = DISCONNECTED; + + private static final int WAIT_CONNECTION_TIMEOUT_MILLIS = 100; + private final Object mLock; private final Context mContext; @VisibleForTesting @@ -178,7 +203,7 @@ public class WindowMagnificationManager implements */ public void setConnection(@Nullable IWindowMagnificationConnection connection) { if (DBG) { - Slog.d(TAG, "setConnection :" + connection); + Slog.d(TAG, "setConnection :" + connection + " ,mConnectionState=" + mConnectionState); } synchronized (mLock) { // Reset connectionWrapper. @@ -189,6 +214,13 @@ public class WindowMagnificationManager implements } mConnectionWrapper.unlinkToDeath(mConnectionCallback); mConnectionWrapper = null; + // The connection is still connecting so it is no need to reset the + // connection state to disconnected. + // TODO b/220086369 will reset the connection immediately when requestConnection + // is called + if (mConnectionState != CONNECTING) { + setConnectionState(DISCONNECTED); + } } if (connection != null) { mConnectionWrapper = new WindowMagnificationConnectionWrapper(connection, mTrace); @@ -199,9 +231,13 @@ public class WindowMagnificationManager implements mConnectionCallback = new ConnectionCallback(); mConnectionWrapper.linkToDeath(mConnectionCallback); mConnectionWrapper.setConnectionCallback(mConnectionCallback); + setConnectionState(CONNECTED); } catch (RemoteException e) { Slog.e(TAG, "setConnection failed", e); mConnectionWrapper = null; + setConnectionState(DISCONNECTED); + } finally { + mLock.notify(); } } } @@ -229,10 +265,20 @@ public class WindowMagnificationManager implements if (DBG) { Slog.d(TAG, "requestConnection :" + connect); } + if (mTrace.isA11yTracingEnabledForTypes(FLAGS_WINDOW_MAGNIFICATION_CONNECTION)) { + mTrace.logTrace(TAG + ".requestWindowMagnificationConnection", + FLAGS_WINDOW_MAGNIFICATION_CONNECTION, "connect=" + connect); + } synchronized (mLock) { - if (connect == isConnected()) { + if ((connect && (mConnectionState == CONNECTED || mConnectionState == CONNECTING)) + || (!connect && (mConnectionState == DISCONNECTED + || mConnectionState == DISCONNECTING))) { + Slog.w(TAG, + "requestConnection duplicated request: connect=" + connect + + " ,mConnectionState=" + mConnectionState); return false; } + if (connect) { final IntentFilter intentFilter = new IntentFilter(Intent.ACTION_SCREEN_OFF); if (!mReceiverRegistered) { @@ -247,19 +293,42 @@ public class WindowMagnificationManager implements } } } - if (mTrace.isA11yTracingEnabledForTypes(FLAGS_WINDOW_MAGNIFICATION_CONNECTION)) { - mTrace.logTrace(TAG + ".requestWindowMagnificationConnection", - FLAGS_WINDOW_MAGNIFICATION_CONNECTION, "connect=" + connect); + if (requestConnectionInternal(connect)) { + setConnectionState(connect ? CONNECTING : DISCONNECTING); + return true; + } else { + setConnectionState(DISCONNECTED); + return false; } + } + + private boolean requestConnectionInternal(boolean connect) { final long identity = Binder.clearCallingIdentity(); try { final StatusBarManagerInternal service = LocalServices.getService( StatusBarManagerInternal.class); - service.requestWindowMagnificationConnection(connect); + if (service != null) { + return service.requestWindowMagnificationConnection(connect); + } } finally { Binder.restoreCallingIdentity(identity); } - return true; + return false; + } + + /** + * Returns window magnification connection state. + */ + public int getConnectionState() { + return mConnectionState; + } + + private void setConnectionState(@ConnectionState int state) { + if (DBG) { + Slog.d(TAG, "setConnectionState : state=" + state + " ,mConnectionState=" + + mConnectionState); + } + mConnectionState = state; } /** @@ -849,6 +918,7 @@ public class WindowMagnificationManager implements mConnectionWrapper.unlinkToDeath(this); mConnectionWrapper = null; mConnectionCallback = null; + setConnectionState(DISCONNECTED); resetWindowMagnifiers(); } } @@ -1025,11 +1095,22 @@ public class WindowMagnificationManager implements float centerY, float magnificationFrameOffsetRatioX, float magnificationFrameOffsetRatioY, MagnificationAnimationCallback animationCallback) { + // Wait for the connection with a timeout. + final long endMillis = SystemClock.uptimeMillis() + WAIT_CONNECTION_TIMEOUT_MILLIS; + while (mConnectionState == CONNECTING && (SystemClock.uptimeMillis() < endMillis)) { + try { + mLock.wait(endMillis - SystemClock.uptimeMillis()); + } catch (InterruptedException ie) { + /* ignore */ + } + } if (mConnectionWrapper == null) { - Slog.w(TAG, "enableWindowMagnificationInternal mConnectionWrapper is null"); + Slog.w(TAG, + "enableWindowMagnificationInternal mConnectionWrapper is null. " + + "mConnectionState=" + mConnectionState); return false; } - return mConnectionWrapper.enableWindowMagnification( + return mConnectionWrapper.enableWindowMagnification( displayId, scale, centerX, centerY, magnificationFrameOffsetRatioX, magnificationFrameOffsetRatioY, animationCallback); diff --git a/services/core/java/com/android/server/statusbar/StatusBarManagerInternal.java b/services/core/java/com/android/server/statusbar/StatusBarManagerInternal.java index 411f3dcc1eb6e..11fd99cf5b687 100644 --- a/services/core/java/com/android/server/statusbar/StatusBarManagerInternal.java +++ b/services/core/java/com/android/server/statusbar/StatusBarManagerInternal.java @@ -157,7 +157,7 @@ public interface StatusBarManagerInternal { * @see com.android.internal.statusbar.IStatusBar#requestWindowMagnificationConnection(boolean * request) */ - void requestWindowMagnificationConnection(boolean request); + boolean requestWindowMagnificationConnection(boolean request); /** * Handles a logging command from the WM shell command. diff --git a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java index 66904b1069525..ef5c7f3d5aad1 100644 --- a/services/core/java/com/android/server/statusbar/StatusBarManagerService.java +++ b/services/core/java/com/android/server/statusbar/StatusBarManagerService.java @@ -637,12 +637,14 @@ public class StatusBarManagerService extends IStatusBarService.Stub implements D } @Override - public void requestWindowMagnificationConnection(boolean request) { + public boolean requestWindowMagnificationConnection(boolean request) { if (mBar != null) { try { mBar.requestWindowMagnificationConnection(request); + return true; } catch (RemoteException ex) { } } + return false; } @Override diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java index 953b5368c86fd..1f016fb6f017d 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java @@ -154,6 +154,7 @@ public class AccessibilityManagerServiceTest { mMockWindowMagnificationMgr); when(mMockMagnificationController.getFullScreenMagnificationController()).thenReturn( mMockFullScreenMagnificationController); + when(mMockMagnificationController.supportWindowMagnification()).thenReturn(true); when(mMockWindowManagerService.getAccessibilityController()).thenReturn( mMockA11yController); when(mMockA11yController.isAccessibilityTracingEnabled()).thenReturn(false); diff --git a/services/tests/servicestests/src/com/android/server/accessibility/magnification/MagnificationControllerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/magnification/MagnificationControllerTest.java index ec59090240f3c..4824f046ebf4f 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/magnification/MagnificationControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/magnification/MagnificationControllerTest.java @@ -41,6 +41,7 @@ import static org.mockito.Mockito.when; import android.accessibilityservice.MagnificationConfig; import android.content.Context; +import android.content.pm.PackageManager; import android.graphics.PointF; import android.graphics.Rect; import android.graphics.Region; @@ -100,6 +101,8 @@ public class MagnificationControllerTest { @Mock private Context mContext; @Mock + PackageManager mPackageManager; + @Mock private FullScreenMagnificationController mScreenMagnificationController; private MagnificationScaleProvider mScaleProvider; @Captor @@ -136,6 +139,7 @@ public class MagnificationControllerTest { mMockResolver = new MockContentResolver(); mMockResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider()); when(mContext.getContentResolver()).thenReturn(mMockResolver); + when(mContext.getPackageManager()).thenReturn(mPackageManager); Settings.Secure.putFloatForUser(mMockResolver, Settings.Secure.ACCESSIBILITY_DISPLAY_MAGNIFICATION_SCALE, DEFAULT_SCALE, CURRENT_USER_ID); diff --git a/services/tests/servicestests/src/com/android/server/accessibility/magnification/WindowMagnificationManagerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/magnification/WindowMagnificationManagerTest.java index 0742c09492f2a..68595c54f606f 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/magnification/WindowMagnificationManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/magnification/WindowMagnificationManagerTest.java @@ -54,6 +54,8 @@ import android.view.accessibility.IRemoteMagnificationAnimationCallback; import android.view.accessibility.IWindowMagnificationConnectionCallback; import android.view.accessibility.MagnificationAnimationCallback; +import androidx.test.core.app.ApplicationProvider; + import com.android.internal.util.test.FakeSettingsProvider; import com.android.server.LocalServices; import com.android.server.accessibility.AccessibilityTraceManager; @@ -99,12 +101,7 @@ public class WindowMagnificationManagerTest { mMockCallback, mMockTrace, new MagnificationScaleProvider(mContext)); when(mContext.getContentResolver()).thenReturn(mResolver); - doAnswer((InvocationOnMock invocation) -> { - final boolean connect = (Boolean) invocation.getArguments()[0]; - mWindowMagnificationManager.setConnection( - connect ? mMockConnection.getConnection() : null); - return null; - }).when(mMockStatusBarManagerInternal).requestWindowMagnificationConnection(anyBoolean()); + stubSetConnection(false); mResolver.addProvider(Settings.AUTHORITY, new FakeSettingsProvider()); Settings.Secure.putFloatForUser(mResolver, @@ -112,6 +109,25 @@ public class WindowMagnificationManagerTest { CURRENT_USER_ID); } + private void stubSetConnection(boolean needDelay) { + doAnswer((InvocationOnMock invocation) -> { + final boolean connect = (Boolean) invocation.getArguments()[0]; + // Simulates setConnection() called by another process. + if (needDelay) { + final Context context = ApplicationProvider.getApplicationContext(); + context.getMainThreadHandler().postDelayed( + () -> { + mWindowMagnificationManager.setConnection( + connect ? mMockConnection.getConnection() : null); + }, 10); + } else { + mWindowMagnificationManager.setConnection( + connect ? mMockConnection.getConnection() : null); + } + return true; + }).when(mMockStatusBarManagerInternal).requestWindowMagnificationConnection(anyBoolean()); + } + @Test public void setConnection_connectionIsNull_wrapperIsNullAndLinkToDeath() { mWindowMagnificationManager.setConnection(mMockConnection.getConnection()); @@ -464,7 +480,7 @@ public class WindowMagnificationManagerTest { public void requestConnectionToNull_disableAllMagnifiersAndRequestWindowMagnificationConnection() throws RemoteException { - mWindowMagnificationManager.setConnection(mMockConnection.getConnection()); + assertTrue(mWindowMagnificationManager.requestConnection(true)); mWindowMagnificationManager.enableWindowMagnification(TEST_DISPLAY, 3f, NaN, NaN); assertTrue(mWindowMagnificationManager.requestConnection(false)); @@ -499,7 +515,7 @@ public class WindowMagnificationManagerTest { @Test public void requestConnectionToNull_expectedGetterResults() { - mWindowMagnificationManager.setConnection(mMockConnection.getConnection()); + mWindowMagnificationManager.requestConnection(true); mWindowMagnificationManager.enableWindowMagnification(TEST_DISPLAY, 3f, 1, 1); mWindowMagnificationManager.requestConnection(false); @@ -512,6 +528,20 @@ public class WindowMagnificationManagerTest { assertTrue(bounds.isEmpty()); } + @Test + public void enableWindowMagnification_connecting_invokeConnectionMethodAfterConnected() + throws RemoteException { + stubSetConnection(true); + mWindowMagnificationManager.requestConnection(true); + + assertTrue(mWindowMagnificationManager.enableWindowMagnification(TEST_DISPLAY, 3f, 1, 1)); + + // Invoke enableWindowMagnification if the connection is connected. + verify(mMockConnection.getConnection()).enableWindowMagnification( + eq(TEST_DISPLAY), eq(3f), + eq(1f), eq(1f), eq(0f), eq(0f), notNull()); + } + @Test public void resetAllMagnification_enabledBySameId_windowMagnifiersDisabled() { mWindowMagnificationManager.setConnection(mMockConnection.getConnection());