From c4f19733257e70df73043fd6ad43631948415cbe Mon Sep 17 00:00:00 2001 From: mincheli Date: Tue, 8 Feb 2022 21:52:46 +0800 Subject: [PATCH] Refactors FullScreenMagnificationController#onMagnificationChangedLocked Uses MagnificationInfoChangedCallback.onFullScreenMagnificationChanged callback method to notify magnification hange instead of accessing the public method of AccessibilityManagerService directly. It reduces the interaction between AccessibilityManagerService and FullScreenMagnificationControlller. So that MagnificationController would take responsible for handling notifyMagnificationChange(). Bug: 218540156 Test: atest FullScreenMagnificationControllerTest, atest FullScreenMagnificationGestureHandlerTest, atest MagnificationControllerTest, Change-Id: Ib9f31681e442f1700a76250a5206199bad458c84 (cherry picked from commit 3c91f067b11ad9b15f5ea81d73f02d81dbc0d766) Merged-In: Ib9f31681e442f1700a76250a5206199bad458c84 --- .../AccessibilityManagerService.java | 14 ++++++- .../FullScreenMagnificationController.java | 36 ++++++++-------- .../MagnificationController.java | 8 +++- ...FullScreenMagnificationControllerTest.java | 24 +++++------ ...ScreenMagnificationGestureHandlerTest.java | 2 - .../MagnificationControllerTest.java | 42 +++++++++++++++++-- 6 files changed, 86 insertions(+), 40 deletions(-) diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 62da981bd9a74..0e99265905119 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -1363,8 +1363,18 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub *

* * @param displayId The logical display id - * @param region the new magnified region, may be empty if - * magnification is not enabled (e.g. scale is 1) + * @param region The magnification region. + * If the config mode is + * {@link MagnificationConfig#MAGNIFICATION_MODE_FULLSCREEN}, + * it is the region of the screen currently active for magnification. + * the returned region will be empty if the magnification is not active + * (e.g. scale is 1. And the magnification is active if magnification + * gestures are enabled or if a service is running that can control + * magnification. + * If the config mode is + * {@link MagnificationConfig#MAGNIFICATION_MODE_WINDOW}, + * it is the region of screen projected on the magnification window. + * The region will be empty if magnification is not activated. * @param config The magnification config. That has magnification mode, the new scale and the * new screen-relative center position */ diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java b/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java index fe97a46e12f9e..a958209669264 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java @@ -50,7 +50,6 @@ import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.LocalServices; -import com.android.server.accessibility.AccessibilityManagerService; import com.android.server.accessibility.AccessibilityTraceManager; import com.android.server.wm.WindowManagerInternal; @@ -374,9 +373,8 @@ public class FullScreenMagnificationController implements .setScale(getScale()) .setCenterX(getCenterX()) .setCenterY(getCenterY()).build(); - mControllerCtx.getAms().notifyMagnificationChanged(mDisplayId, - mMagnificationRegion, - config); + mMagnificationInfoChangedCallback.onFullScreenMagnificationChanged(mDisplayId, + mMagnificationRegion, config); if (mUnregisterPending && !isMagnifying()) { unregister(mDeleteAfterUnregister); } @@ -665,10 +663,10 @@ public class FullScreenMagnificationController implements * FullScreenMagnificationController Constructor */ public FullScreenMagnificationController(@NonNull Context context, - @NonNull AccessibilityManagerService ams, @NonNull Object lock, + @NonNull AccessibilityTraceManager traceManager, @NonNull Object lock, @NonNull MagnificationInfoChangedCallback magnificationInfoChangedCallback, @NonNull MagnificationScaleProvider scaleProvider) { - this(new ControllerContext(context, ams, + this(new ControllerContext(context, traceManager, LocalServices.getService(WindowManagerInternal.class), new Handler(context.getMainLooper()), context.getResources().getInteger(R.integer.config_longAnimTime)), lock, @@ -1521,7 +1519,6 @@ public class FullScreenMagnificationController implements @VisibleForTesting public static class ControllerContext { private final Context mContext; - private final AccessibilityManagerService mAms; private final AccessibilityTraceManager mTrace; private final WindowManagerInternal mWindowManager; private final Handler mHandler; @@ -1531,13 +1528,12 @@ public class FullScreenMagnificationController implements * Constructor for ControllerContext. */ public ControllerContext(@NonNull Context context, - @NonNull AccessibilityManagerService ams, + @NonNull AccessibilityTraceManager traceManager, @NonNull WindowManagerInternal windowManager, @NonNull Handler handler, long animationDuration) { mContext = context; - mAms = ams; - mTrace = ams.getTraceManager(); + mTrace = traceManager; mWindowManager = windowManager; mHandler = handler; mAnimationDuration = animationDuration; @@ -1551,14 +1547,6 @@ public class FullScreenMagnificationController implements return mContext; } - /** - * @return AccessibilityManagerService - */ - @NonNull - public AccessibilityManagerService getAms() { - return mAms; - } - /** * @return AccessibilityTraceManager */ @@ -1632,5 +1620,17 @@ public class FullScreenMagnificationController implements * hidden. */ void onImeWindowVisibilityChanged(boolean shown); + + /** + * Called when the magnification spec changed. + * + * @param displayId The logical display id + * @param region The region of the screen currently active for magnification. + * The returned region will be empty if the magnification is not active. + * @param config The magnification config. That has magnification mode, the new scale and + * the new screen-relative center position + */ + void onFullScreenMagnificationChanged(int displayId, @NonNull Region region, + @NonNull MagnificationConfig config); } } 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 c376bf873dc86..09e82c787c904 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationController.java @@ -405,6 +405,12 @@ public class MagnificationController implements WindowMagnificationManager.Callb mAms.notifyMagnificationChanged(displayId, new Region(bounds), config); } + @Override + public void onFullScreenMagnificationChanged(int displayId, @NonNull Region region, + @NonNull MagnificationConfig config) { + mAms.notifyMagnificationChanged(displayId, region, config); + } + private void disableFullScreenMagnificationIfNeeded(int displayId) { final FullScreenMagnificationController fullScreenMagnificationController = getFullScreenMagnificationController(); @@ -590,7 +596,7 @@ public class MagnificationController implements WindowMagnificationManager.Callb synchronized (mLock) { if (mFullScreenMagnificationController == null) { mFullScreenMagnificationController = new FullScreenMagnificationController(mContext, - mAms, mLock, this, mScaleProvider); + mAms.getTraceManager(), mLock, this, mScaleProvider); } } return mFullScreenMagnificationController; diff --git a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java index a9b7cfb5e3388..3ce2ed84d3e8f 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java @@ -52,7 +52,6 @@ import android.view.accessibility.MagnificationAnimationCallback; import androidx.test.InstrumentationRegistry; import androidx.test.runner.AndroidJUnit4; -import com.android.server.accessibility.AccessibilityManagerService; import com.android.server.accessibility.AccessibilityTraceManager; import com.android.server.accessibility.test.MessageCapturingHandler; import com.android.server.wm.WindowManagerInternal; @@ -94,7 +93,6 @@ public class FullScreenMagnificationControllerTest { final FullScreenMagnificationController.ControllerContext mMockControllerCtx = mock(FullScreenMagnificationController.ControllerContext.class); final Context mMockContext = mock(Context.class); - final AccessibilityManagerService mMockAms = mock(AccessibilityManagerService.class); final AccessibilityTraceManager mMockTraceManager = mock(AccessibilityTraceManager.class); final WindowManagerInternal mMockWindowManager = mock(WindowManagerInternal.class); private final MagnificationAnimationCallback mAnimationCallback = mock( @@ -121,12 +119,10 @@ public class FullScreenMagnificationControllerTest { // Pretending ID of the Thread associated with looper as main thread ID in controller when(mMockContext.getMainLooper()).thenReturn(looper); when(mMockControllerCtx.getContext()).thenReturn(mMockContext); - when(mMockControllerCtx.getAms()).thenReturn(mMockAms); when(mMockControllerCtx.getTraceManager()).thenReturn(mMockTraceManager); when(mMockControllerCtx.getWindowManager()).thenReturn(mMockWindowManager); when(mMockControllerCtx.getHandler()).thenReturn(mMessageCapturingHandler); when(mMockControllerCtx.getAnimationDuration()).thenReturn(1000L); - when(mMockAms.getTraceManager()).thenReturn(mMockTraceManager); initMockWindowManager(); mFullScreenMagnificationController = new FullScreenMagnificationController( @@ -357,8 +353,8 @@ public class FullScreenMagnificationControllerTest { assertEquals(newCenter.x, mFullScreenMagnificationController.getCenterX(displayId), 0.5); assertEquals(newCenter.y, mFullScreenMagnificationController.getCenterY(displayId), 0.5); assertThat(getCurrentMagnificationSpec(displayId), closeTo(endSpec)); - verify(mMockAms).notifyMagnificationChanged(eq(displayId), eq(INITIAL_MAGNIFICATION_REGION), - mConfigCaptor.capture()); + verify(mRequestObserver).onFullScreenMagnificationChanged(eq(displayId), + eq(INITIAL_MAGNIFICATION_REGION), mConfigCaptor.capture()); assertConfigEquals(config, mConfigCaptor.getValue()); verify(mMockValueAnimator).start(); verify(mRequestObserver).onRequestMagnificationSpec(displayId, SERVICE_ID_1); @@ -501,7 +497,7 @@ public class FullScreenMagnificationControllerTest { mMessageCapturingHandler.sendAllMessages(); MagnificationConfig config = buildConfig(1.0f, OTHER_MAGNIFICATION_BOUNDS.centerX(), OTHER_MAGNIFICATION_BOUNDS.centerY()); - verify(mMockAms).notifyMagnificationChanged(eq(displayId), eq(OTHER_REGION), + verify(mRequestObserver).onFullScreenMagnificationChanged(eq(displayId), eq(OTHER_REGION), mConfigCaptor.capture()); assertConfigEquals(config, mConfigCaptor.getValue()); } @@ -655,9 +651,9 @@ public class FullScreenMagnificationControllerTest { register(displayId); zoomIn2xToMiddle(displayId); mMessageCapturingHandler.sendAllMessages(); - reset(mMockAms); + reset(mRequestObserver); assertTrue(mFullScreenMagnificationController.resetIfNeeded(displayId, false)); - verify(mMockAms).notifyMagnificationChanged(eq(displayId), + verify(mRequestObserver).onFullScreenMagnificationChanged(eq(displayId), eq(INITIAL_MAGNIFICATION_REGION), any(MagnificationConfig.class)); assertFalse(mFullScreenMagnificationController.isMagnifying(displayId)); assertFalse(mFullScreenMagnificationController.resetIfNeeded(displayId, false)); @@ -676,8 +672,8 @@ public class FullScreenMagnificationControllerTest { assertFalse(mFullScreenMagnificationController.reset(displayId, mAnimationCallback)); mMessageCapturingHandler.sendAllMessages(); - verify(mMockAms, never()).notifyMagnificationChanged(eq(displayId), any(Region.class), - any(MagnificationConfig.class)); + verify(mRequestObserver, never()).onFullScreenMagnificationChanged(eq(displayId), + any(Region.class), any(MagnificationConfig.class)); verify(mAnimationCallback).onResult(true); } @@ -1072,8 +1068,8 @@ public class FullScreenMagnificationControllerTest { when(mMockValueAnimator.getAnimatedFraction()).thenReturn(0.0f); mTargetAnimationListener.onAnimationUpdate(mMockValueAnimator); verify(mMockWindowManager).setMagnificationSpec(eq(displayId), eq(startSpec)); - verify(mMockAms).notifyMagnificationChanged(eq(displayId), eq(INITIAL_MAGNIFICATION_REGION), - mConfigCaptor.capture()); + verify(mRequestObserver).onFullScreenMagnificationChanged(eq(displayId), + eq(INITIAL_MAGNIFICATION_REGION), mConfigCaptor.capture()); assertConfigEquals(config, mConfigCaptor.getValue()); Mockito.reset(mMockWindowManager); @@ -1097,7 +1093,7 @@ public class FullScreenMagnificationControllerTest { // Animation should have been restarted verify(mMockValueAnimator, times(2)).start(); - verify(mMockAms, times(2)).notifyMagnificationChanged(eq(displayId), + verify(mRequestObserver, times(2)).onFullScreenMagnificationChanged(eq(displayId), eq(INITIAL_MAGNIFICATION_REGION), mConfigCaptor.capture()); assertConfigEquals(newConfig, mConfigCaptor.getValue()); diff --git a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationGestureHandlerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationGestureHandlerTest.java index 2060223f6f98e..0fed89b831d6d 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationGestureHandlerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationGestureHandlerTest.java @@ -151,8 +151,6 @@ public class FullScreenMagnificationGestureHandlerTest { mock(FullScreenMagnificationController.ControllerContext.class); final WindowManagerInternal mockWindowManager = mock(WindowManagerInternal.class); when(mockController.getContext()).thenReturn(mContext); - when(mockController.getAms()).thenReturn(mMockAccessibilityManagerService); - when(mMockAccessibilityManagerService.getTraceManager()).thenReturn(mMockTraceManager); when(mockController.getTraceManager()).thenReturn(mMockTraceManager); when(mockController.getWindowManager()).thenReturn(mockWindowManager); when(mockController.getHandler()).thenReturn(new Handler(mContext.getMainLooper())); 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 3fcce92fe1417..ec59090240f3c 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 @@ -145,9 +145,10 @@ public class MagnificationControllerTest { mCallbackDelegate, mTraceManager, mScaleProvider)); mMockConnection = new MockWindowMagnificationConnection(true); mWindowMagnificationManager.setConnection(mMockConnection.getConnection()); - new FullScreenMagnificationControllerStubber(mScreenMagnificationController); mMagnificationController = new MagnificationController(mService, globalLock, mContext, mScreenMagnificationController, mWindowMagnificationManager, mScaleProvider); + new FullScreenMagnificationControllerStubber(mScreenMagnificationController, + mMagnificationController); mMagnificationController.setMagnificationCapabilities( Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_ALL); @@ -450,6 +451,29 @@ public class MagnificationControllerTest { assertEquals(DEFAULT_SCALE, actualConfig.getScale(), 0); } + @Test + public void onFullScreenMagnificationChanged_fullScreenEnabled_notifyMagnificationChanged() + throws RemoteException { + setMagnificationEnabled(MODE_FULLSCREEN); + + final MagnificationConfig config = obtainMagnificationConfig(MODE_FULLSCREEN); + mScreenMagnificationController.setScaleAndCenter(TEST_DISPLAY, + config.getScale(), config.getCenterX(), config.getCenterY(), + true, TEST_SERVICE_ID); + + // The first time is triggered when setting magnification enabled. And the second time is + // triggered when calling setScaleAndCenter. + final ArgumentCaptor configCaptor = ArgumentCaptor.forClass( + MagnificationConfig.class); + verify(mService, times(2)).notifyMagnificationChanged(eq(TEST_DISPLAY), + eq(FullScreenMagnificationControllerStubber.MAGNIFICATION_REGION), + configCaptor.capture()); + final MagnificationConfig actualConfig = configCaptor.getValue(); + assertEquals(config.getCenterX(), actualConfig.getCenterX(), 0); + assertEquals(config.getCenterY(), actualConfig.getCenterY(), 0); + assertEquals(config.getScale(), actualConfig.getScale(), 0); + } + @Test public void onAccessibilityActionPerformed_magnifierEnabled_showMagnificationButton() throws RemoteException { @@ -679,7 +703,7 @@ public class MagnificationControllerTest { throws RemoteException { setMagnificationEnabled(MODE_FULLSCREEN); mScreenMagnificationController.setScaleAndCenter(TEST_DISPLAY, - /* scale= */1, MAGNIFIED_CENTER_X, MAGNIFIED_CENTER_Y, + /* scale= */ 1, MAGNIFIED_CENTER_X, MAGNIFIED_CENTER_Y, true, TEST_SERVICE_ID); mMagnificationController.onFullScreenMagnificationActivationState(TEST_DISPLAY, false); @@ -884,6 +908,8 @@ public class MagnificationControllerTest { private static class FullScreenMagnificationControllerStubber { private static final Region MAGNIFICATION_REGION = new Region(0, 0, 500, 600); private final FullScreenMagnificationController mScreenMagnificationController; + private final FullScreenMagnificationController.MagnificationInfoChangedCallback + mMagnificationChangedCallback; private boolean mIsMagnifying = false; private float mScale = 1.0f; private float mCenterX = MAGNIFICATION_REGION.getBounds().exactCenterX(); @@ -891,8 +917,10 @@ public class MagnificationControllerTest { private int mServiceId = -1; FullScreenMagnificationControllerStubber( - FullScreenMagnificationController screenMagnificationController) { + FullScreenMagnificationController screenMagnificationController, + FullScreenMagnificationController.MagnificationInfoChangedCallback callback) { mScreenMagnificationController = screenMagnificationController; + mMagnificationChangedCallback = callback; stubMethods(); } @@ -930,6 +958,14 @@ public class MagnificationControllerTest { } else { reset(); } + + + final MagnificationConfig config = new MagnificationConfig.Builder().setMode( + MODE_FULLSCREEN).setScale(mScale).setCenterX(mCenterX).setCenterY( + mCenterY).build(); + mMagnificationChangedCallback.onFullScreenMagnificationChanged(TEST_DISPLAY, + FullScreenMagnificationControllerStubber.MAGNIFICATION_REGION, + config); return true; }; doAnswer(setScaleAndCenterStubAnswer).when(