fix(magnification button): delay showing magnification button to prevent button from detecting the touch expectedly
When magnification mode switch button is hiding, finger down event at the button area will first trigger MagnificationController#onUserInteractionStart to show the button, then the down event will be detected by the button too. This causes tapping on the hiding button area will show both the button and the settings panel. Therefore, when MagnificationController calls the systemui to show the button, we delay the showButton at systemui side, so the button will not show immediately then detects the touch event unexpectedly.
Bug: 338259519
Flag: ACONFIG com.android.systemui.delay_show_magnification_button DEVELOPMENT
Test: manually flip the flag
atest IMagnificationConnectionTest
Change-Id: I7852633914ab93126efc92199a904ef784d775af
diff --git a/packages/SystemUI/aconfig/accessibility.aconfig b/packages/SystemUI/aconfig/accessibility.aconfig
index 14ebc39..755fe2a 100644
--- a/packages/SystemUI/aconfig/accessibility.aconfig
+++ b/packages/SystemUI/aconfig/accessibility.aconfig
@@ -4,6 +4,16 @@
# NOTE: Keep alphabetized to help limit merge conflicts from multiple simultaneous editors.
flag {
+ name: "delay_show_magnification_button"
+ namespace: "accessibility"
+ description: "Delays the showing of magnification mode switch button."
+ bug: "338259519"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
+
+flag {
name: "floating_menu_animated_tuck"
namespace: "accessibility"
description: "Sets up animations for tucking/untucking and adjusts clipbounds."
diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/Magnification.java b/packages/SystemUI/src/com/android/systemui/accessibility/Magnification.java
index 177d933..35c2024 100644
--- a/packages/SystemUI/src/com/android/systemui/accessibility/Magnification.java
+++ b/packages/SystemUI/src/com/android/systemui/accessibility/Magnification.java
@@ -30,6 +30,8 @@
import android.graphics.Rect;
import android.hardware.display.DisplayManager;
import android.os.Handler;
+import android.os.Looper;
+import android.os.Message;
import android.util.SparseArray;
import android.view.Display;
import android.view.SurfaceControl;
@@ -41,6 +43,8 @@
import android.view.accessibility.IRemoteMagnificationAnimationCallback;
import android.window.InputTransferToken;
+import androidx.annotation.NonNull;
+
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.graphics.SfVsyncFrameCallbackProvider;
import com.android.systemui.CoreStartable;
@@ -69,6 +73,9 @@
public class Magnification implements CoreStartable, CommandQueue.Callbacks {
private static final String TAG = "Magnification";
+ @VisibleForTesting static final int DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS = 300;
+ private static final int MSG_SHOW_MAGNIFICATION_BUTTON_INTERNAL = 1;
+
private final ModeSwitchesController mModeSwitchesController;
private final Context mContext;
private final Handler mHandler;
@@ -209,8 +216,26 @@
SysUiState sysUiState, OverviewProxyService overviewProxyService,
SecureSettings secureSettings, DisplayTracker displayTracker,
DisplayManager displayManager, AccessibilityLogger a11yLogger) {
+ this(context, mainHandler.getLooper(), executor, commandQueue,
+ modeSwitchesController, sysUiState, overviewProxyService, secureSettings,
+ displayTracker, displayManager, a11yLogger);
+ }
+
+ @VisibleForTesting
+ public Magnification(Context context, Looper looper, @Main Executor executor,
+ CommandQueue commandQueue, ModeSwitchesController modeSwitchesController,
+ SysUiState sysUiState, OverviewProxyService overviewProxyService,
+ SecureSettings secureSettings, DisplayTracker displayTracker,
+ DisplayManager displayManager, AccessibilityLogger a11yLogger) {
mContext = context;
- mHandler = mainHandler;
+ mHandler = new Handler(looper) {
+ @Override
+ public void handleMessage(@NonNull Message msg) {
+ if (msg.what == MSG_SHOW_MAGNIFICATION_BUTTON_INTERNAL) {
+ showMagnificationButtonInternal(msg.arg1, msg.arg2);
+ }
+ }
+ };
mExecutor = executor;
mAccessibilityManager = mContext.getSystemService(AccessibilityManager.class);
mCommandQueue = commandQueue;
@@ -350,6 +375,21 @@
@MainThread
void showMagnificationButton(int displayId, int magnificationMode) {
+ if (Flags.delayShowMagnificationButton()) {
+ if (mHandler.hasMessages(MSG_SHOW_MAGNIFICATION_BUTTON_INTERNAL)) {
+ return;
+ }
+ mHandler.sendMessageDelayed(
+ mHandler.obtainMessage(
+ MSG_SHOW_MAGNIFICATION_BUTTON_INTERNAL, displayId, magnificationMode),
+ DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS);
+ } else {
+ showMagnificationButtonInternal(displayId, magnificationMode);
+ }
+ }
+
+ @MainThread
+ private void showMagnificationButtonInternal(int displayId, int magnificationMode) {
// not to show mode switch button if settings panel is already showing to
// prevent settings panel be covered by the button.
if (isMagnificationSettingsPanelShowing(displayId)) {
@@ -360,6 +400,9 @@
@MainThread
void removeMagnificationButton(int displayId) {
+ if (Flags.delayShowMagnificationButton()) {
+ mHandler.removeMessages(MSG_SHOW_MAGNIFICATION_BUTTON_INTERNAL);
+ }
mModeSwitchesController.removeButton(displayId);
}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/IMagnificationConnectionTest.java b/packages/SystemUI/tests/src/com/android/systemui/accessibility/IMagnificationConnectionTest.java
index 25e5470..3164f8e 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/accessibility/IMagnificationConnectionTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/IMagnificationConnectionTest.java
@@ -16,6 +16,8 @@
package com.android.systemui.accessibility;
+import static com.android.systemui.accessibility.Magnification.DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -23,11 +25,17 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import android.content.Context;
import android.hardware.display.DisplayManager;
import android.os.RemoteException;
+import android.platform.test.annotations.RequiresFlagsDisabled;
+import android.platform.test.annotations.RequiresFlagsEnabled;
+import android.platform.test.flag.junit.CheckFlagsRule;
+import android.platform.test.flag.junit.DeviceFlagsValueProvider;
import android.provider.Settings;
import android.testing.AndroidTestingRunner;
import android.testing.TestableLooper;
@@ -39,6 +47,7 @@
import androidx.test.filters.SmallTest;
+import com.android.systemui.Flags;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.model.SysUiState;
import com.android.systemui.recents.OverviewProxyService;
@@ -47,6 +56,7 @@
import com.android.systemui.util.settings.SecureSettings;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mock;
@@ -58,9 +68,12 @@
*/
@SmallTest
@RunWith(AndroidTestingRunner.class)
-@TestableLooper.RunWithLooper
+@TestableLooper.RunWithLooper(setAsMainLooper = true)
public class IMagnificationConnectionTest extends SysuiTestCase {
+ @Rule
+ public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();
+
private static final int TEST_DISPLAY = Display.DEFAULT_DISPLAY;
@Mock
private AccessibilityManager mAccessibilityManager;
@@ -90,6 +103,7 @@
private IMagnificationConnection mIMagnificationConnection;
private Magnification mMagnification;
private FakeDisplayTracker mDisplayTracker = new FakeDisplayTracker(mContext);
+ private TestableLooper mTestableLooper;
@Before
public void setUp() throws Exception {
@@ -100,8 +114,10 @@
return null;
}).when(mAccessibilityManager).setMagnificationConnection(
any(IMagnificationConnection.class));
+ mTestableLooper = TestableLooper.get(this);
+ assertNotNull(mTestableLooper);
mMagnification = new Magnification(getContext(),
- getContext().getMainThreadHandler(), getContext().getMainExecutor(), mCommandQueue,
+ mTestableLooper.getLooper(), getContext().getMainExecutor(), mCommandQueue,
mModeSwitchesController, mSysUiState, mOverviewProxyService, mSecureSettings,
mDisplayTracker, getContext().getSystemService(DisplayManager.class), mA11yLogger);
mMagnification.mWindowMagnificationControllerSupplier =
@@ -122,7 +138,7 @@
public void enableWindowMagnification_passThrough() throws RemoteException {
mIMagnificationConnection.enableWindowMagnification(TEST_DISPLAY, 3.0f, Float.NaN,
Float.NaN, 0f, 0f, mAnimationCallback);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mWindowMagnificationController).enableWindowMagnification(eq(3.0f),
eq(Float.NaN), eq(Float.NaN), eq(0f), eq(0f), eq(mAnimationCallback));
@@ -131,7 +147,7 @@
@Test
public void onFullscreenMagnificationActivationChanged_passThrough() throws RemoteException {
mIMagnificationConnection.onFullscreenMagnificationActivationChanged(TEST_DISPLAY, true);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mFullscreenMagnificationController)
.onFullscreenMagnificationActivationChanged(eq(true));
@@ -141,7 +157,7 @@
public void disableWindowMagnification_deleteWindowMagnification() throws RemoteException {
mIMagnificationConnection.disableWindowMagnification(TEST_DISPLAY,
mAnimationCallback);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mWindowMagnificationController).deleteWindowMagnification(
mAnimationCallback);
@@ -150,7 +166,7 @@
@Test
public void setScaleForWindowMagnification() throws RemoteException {
mIMagnificationConnection.setScaleForWindowMagnification(TEST_DISPLAY, 3.0f);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mWindowMagnificationController).setScale(3.0f);
}
@@ -158,7 +174,7 @@
@Test
public void moveWindowMagnifier() throws RemoteException {
mIMagnificationConnection.moveWindowMagnifier(TEST_DISPLAY, 100f, 200f);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mWindowMagnificationController).moveWindowMagnifier(100f, 200f);
}
@@ -167,37 +183,102 @@
public void moveWindowMagnifierToPosition() throws RemoteException {
mIMagnificationConnection.moveWindowMagnifierToPosition(TEST_DISPLAY,
100f, 200f, mAnimationCallback);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mWindowMagnificationController).moveWindowMagnifierToPosition(
eq(100f), eq(200f), any(IRemoteMagnificationAnimationCallback.class));
}
@Test
- public void showMagnificationButton() throws RemoteException {
+ @RequiresFlagsDisabled(Flags.FLAG_DELAY_SHOW_MAGNIFICATION_BUTTON)
+ public void showMagnificationButton_flagOff_directlyShowButton() throws RemoteException {
// magnification settings panel should not be showing
assertFalse(mMagnification.isMagnificationSettingsPanelShowing(TEST_DISPLAY));
mIMagnificationConnection.showMagnificationButton(TEST_DISPLAY,
Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mModeSwitchesController).showButton(TEST_DISPLAY,
Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
}
@Test
+ @RequiresFlagsEnabled(Flags.FLAG_DELAY_SHOW_MAGNIFICATION_BUTTON)
+ public void showMagnificationButton_flagOn_delayedShowButton() throws RemoteException {
+ // magnification settings panel should not be showing
+ assertFalse(mMagnification.isMagnificationSettingsPanelShowing(TEST_DISPLAY));
+
+ mIMagnificationConnection.showMagnificationButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ // This processAllPendingMessages lets the IMagnificationConnection to delegate the
+ // showMagnificationButton request to Magnification.
+ processAllPendingMessages();
+
+ // The delayed message would be processed after DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS.
+ // So call this processAllPendingMessages with a timeout to verify the showButton
+ // will be called.
+ int timeout = DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS + 100;
+ processAllPendingMessages(timeout);
+ verify(mModeSwitchesController).showButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ }
+
+ @Test
+ public void showMagnificationButton_settingsPanelShowing_doNotShowButton()
+ throws RemoteException {
+ when(mMagnificationSettingsController.isMagnificationSettingsShowing()).thenReturn(true);
+
+ mIMagnificationConnection.showMagnificationButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ // This processAllPendingMessages lets the IMagnificationConnection to delegate the
+ // showMagnificationButton request to Magnification.
+ processAllPendingMessages();
+
+ // If the flag is on, the isMagnificationSettingsShowing will be checked after timeout, so
+ // process all message after a timeout here to verify the showButton will not be called.
+ int timeout = Flags.delayShowMagnificationButton()
+ ? DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS + 100
+ : 0;
+ processAllPendingMessages(timeout);
+ verify(mModeSwitchesController, never()).showButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ }
+
+ @Test
public void removeMagnificationButton() throws RemoteException {
mIMagnificationConnection.removeMagnificationButton(TEST_DISPLAY);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mModeSwitchesController).removeButton(TEST_DISPLAY);
}
@Test
+ @RequiresFlagsEnabled(Flags.FLAG_DELAY_SHOW_MAGNIFICATION_BUTTON)
+ public void removeMagnificationButton_delayingShowButton_doNotShowButtonAfterTimeout()
+ throws RemoteException {
+ // magnification settings panel should not be showing
+ assertFalse(mMagnification.isMagnificationSettingsPanelShowing(TEST_DISPLAY));
+
+ mIMagnificationConnection.showMagnificationButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ mIMagnificationConnection.removeMagnificationButton(TEST_DISPLAY);
+ // This processAllPendingMessages lets the IMagnificationConnection to delegate the
+ // requests to Magnification.
+ processAllPendingMessages();
+
+ // Call this processAllPendingMessages with a timeout to ensure the delayed show button
+ // message should be removed and thus the showButton will not be called after timeout.
+ int timeout = DELAY_SHOW_MAGNIFICATION_TIMEOUT_MS + 100;
+ processAllPendingMessages(/* timeForwardMs= */ timeout);
+ verify(mModeSwitchesController, never()).showButton(TEST_DISPLAY,
+ Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN);
+ }
+
+ @Test
public void removeMagnificationSettingsPanel() throws RemoteException {
mIMagnificationConnection.removeMagnificationSettingsPanel(TEST_DISPLAY);
- waitForIdleSync();
+ processAllPendingMessages();
verify(mMagnificationSettingsController).closeMagnificationSettings();
}
@@ -208,7 +289,7 @@
final float testScale = 3.0f;
mIMagnificationConnection.onUserMagnificationScaleChanged(
testUserId, TEST_DISPLAY, testScale);
- waitForIdleSync();
+ processAllPendingMessages();
assertTrue(mMagnification.mUsersScales.contains(testUserId));
assertEquals(mMagnification.mUsersScales.get(testUserId).get(TEST_DISPLAY),
@@ -216,6 +297,17 @@
verify(mMagnificationSettingsController).setMagnificationScale(eq(testScale));
}
+ private void processAllPendingMessages() {
+ processAllPendingMessages(/* timeForwardMs=*/ 0);
+ }
+
+ private void processAllPendingMessages(int timeForwardMs) {
+ if (timeForwardMs > 0) {
+ mTestableLooper.moveTimeForward(timeForwardMs);
+ }
+ mTestableLooper.processAllMessages();
+ }
+
private class FakeWindowMagnificationControllerSupplier extends
DisplayIdIndexSupplier<WindowMagnificationController> {
@@ -229,7 +321,6 @@
}
}
-
private class FakeFullscreenMagnificationControllerSupplier extends
DisplayIdIndexSupplier<FullscreenMagnificationController> {