From d7219b7d60b6905c1941bc55f92b0228a7955aab Mon Sep 17 00:00:00 2001 From: ryanlwlin Date: Tue, 15 Mar 2022 09:05:16 +0800 Subject: [PATCH] Fix the flaky AccessibilityMagnificationTest#testListener The listner might invoked twice because we use vsync to change the magnification spec. When the animation is running, the posted tasks might executed after animation ended twice. To fix, we update the source bounds only when it is indeed changed, which also could reduce the redudant mirroing content operation when the request is sent consecutively in a short time. Bug: 214785717 Test: atest AccesibilityMagnificationTest \ --retry-strategy ITERATIONS --max-testcase-run-count 20 Change-Id: I68962b189d8f39f587b717e8125a224e0f67a7e7 --- .../WindowMagnificationController.java | 14 ++++++++++---- .../WindowMagnificationControllerTest.java | 19 ++++++++++++++----- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationController.java b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationController.java index 7e1a02626dc9f..807ff21bf47a8 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationController.java +++ b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnificationController.java @@ -254,8 +254,8 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold mMirrorViewGeometryVsyncCallback = l -> { - if (isWindowVisible() && mMirrorSurface != null) { - calculateSourceBounds(mMagnificationFrame, mScale); + if (isWindowVisible() && mMirrorSurface != null && calculateSourceBounds( + mMagnificationFrame, mScale)) { // The final destination for the magnification surface should be at 0,0 // since the ViewRootImpl's position will change mTmpRect.set(0, 0, mMagnificationFrame.width(), @@ -350,6 +350,7 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold mMirrorWindowControl.destroyControl(); } mMirrorViewBounds.setEmpty(); + mSourceBounds.setEmpty(); updateSystemUIStateIfNeeded(); mContext.unregisterComponentCallbacks(this); } @@ -728,8 +729,12 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold /** * Calculates the desired source bounds. This will be the area under from the center of the * displayFrame, factoring in scale. + * + * @return {@code true} if the source bounds is changed. */ - private void calculateSourceBounds(Rect displayFrame, float scale) { + private boolean calculateSourceBounds(Rect displayFrame, float scale) { + final Rect oldSourceBounds = mTmpRect; + oldSourceBounds.set(mSourceBounds); int halfWidth = displayFrame.width() / 2; int halfHeight = displayFrame.height() / 2; int left = displayFrame.left + (halfWidth - (int) (halfWidth / scale)); @@ -757,6 +762,7 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold mSourceBounds.offsetTo(mSourceBounds.left, mWindowBounds.height() - mSourceBounds.height()); } + return !mSourceBounds.equals(oldSourceBounds); } private void calculateMagnificationFrameBoundary() { @@ -1079,7 +1085,7 @@ class WindowMagnificationController implements View.OnTouchListener, SurfaceHold pw.println(" mMagnificationFrame:" + (isWindowVisible() ? mMagnificationFrame : "empty")); pw.println(" mSourceBounds:" - + (isWindowVisible() ? mSourceBounds : "empty")); + + (mSourceBounds.isEmpty() ? "empty" : mSourceBounds)); pw.println(" mSystemGestureTop:" + mSystemGestureTop); pw.println(" mMagnificationFrameOffsetX:" + mMagnificationFrameOffsetX); pw.println(" mMagnificationFrameOffsetY:" + mMagnificationFrameOffsetY); diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationControllerTest.java index a49c4d76445b9..c684b66d2892d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationControllerTest.java @@ -172,17 +172,26 @@ public class WindowMagnificationControllerTest extends SysuiTestCase { @Test public void enableWindowMagnification_notifySourceBoundsChanged() { - mInstrumentation.runOnMainSync(() -> { - mWindowMagnificationController.enableWindowMagnification(Float.NaN, Float.NaN, - Float.NaN, /* magnificationFrameOffsetRatioX= */ 0, - /* magnificationFrameOffsetRatioY= */ 0, null); - }); + mInstrumentation.runOnMainSync( + () -> mWindowMagnificationController.enableWindowMagnification(Float.NaN, Float.NaN, + Float.NaN, /* magnificationFrameOffsetRatioX= */ 0, + /* magnificationFrameOffsetRatioY= */ 0, null)); // Waits for the surface created verify(mWindowMagnifierCallback, timeout(LAYOUT_CHANGE_TIMEOUT_MS)).onSourceBoundsChanged( (eq(mContext.getDisplayId())), any()); } + @Test + public void enableWindowMagnification_disabled_notifySourceBoundsChanged() { + enableWindowMagnification_notifySourceBoundsChanged(); + mInstrumentation.runOnMainSync( + () -> mWindowMagnificationController.deleteWindowMagnification(null)); + Mockito.reset(mWindowMagnifierCallback); + + enableWindowMagnification_notifySourceBoundsChanged(); + } + @Test public void enableWindowMagnification_withAnimation_schedulesFrame() { mInstrumentation.runOnMainSync(() -> {