From 1e5c277f7bb63b125e77ea3ef76cf58071d6ef86 Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Fri, 12 Nov 2021 13:44:34 +0800 Subject: [PATCH] Remote mCurClientInKeyguard dependency in IMMS The above field introduced from CL[1] for quick-fixing unexpected back icon indicator shows up when the lockscreen is shown. However, from IMMS points of view, seems IMMS is not necessary to be the decision maker to tell systemUI whether the IME is behind keyguard or not. SystemUI can properly leverage WindowInsets APIs to know the IME insets state when the keyguard requests IME visible. so that in other non-keyguard cases should be enough to deliver the IME visiblity/active information without setting "vis=0" for the special cases. [1]: I0de01ec29cb544e902305b0f9d9fb94a73835e7b Change-Id: I24c511d068ab371d6984201327a561322fc26bd8 Fix: 202218289 Test: manual as steps: 1) Set passcode for lockscreen to require IME. 2) Try to unlock screen 3) See if the back key icon altered when the IME shows up on the lockscreen Test: atest NavigationBarTest#\ testSetImeWindowStatusWhenKeyguardLockingAndImeInsetsChange --- .../systemui/navigationbar/NavigationBar.java | 13 ++++- .../navigationbar/NavigationBarTest.java | 48 ++++++++++++++++++- .../InputMethodManagerService.java | 12 ----- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java index 0e6e8a4337993..bdacc4118442e 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java @@ -97,6 +97,7 @@ import android.view.MotionEvent; import android.view.Surface; import android.view.View; import android.view.ViewTreeObserver; +import android.view.WindowInsets; import android.view.WindowInsetsController.Appearance; import android.view.WindowInsetsController.Behavior; import android.view.WindowManager; @@ -901,7 +902,17 @@ public class NavigationBar implements View.OnAttachStateChangeListener, if (displayId != mDisplayId) { return; } - boolean imeShown = (vis & InputMethodService.IME_VISIBLE) != 0; + boolean imeVisibleOnShade = mStatusBarOptionalLazy.get().map(statusBar -> { + View shadeWindowView = statusBar.getNotificationShadeWindowView(); + return shadeWindowView.isAttachedToWindow() + && shadeWindowView.getRootWindowInsets().isVisible(WindowInsets.Type.ime()); + }).orElse(false); + boolean isKeyguardShowing = mStatusBarOptionalLazy.get().map( + StatusBar::isKeyguardShowing).orElse(false); + boolean imeShown = imeVisibleOnShade + || (!isKeyguardShowing && (vis & InputMethodService.IME_VISIBLE) != 0); + showImeSwitcher = imeShown && showImeSwitcher; + int hints = Utilities.calculateBackDispositionHints(mNavigationIconHints, backDisposition, imeShown, showImeSwitcher); if (hints == mNavigationIconHints) return; diff --git a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java index 223ffbd7bba5e..50b717181dc26 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java @@ -23,17 +23,20 @@ import static android.inputmethodservice.InputMethodService.IME_INVISIBLE; import static android.inputmethodservice.InputMethodService.IME_VISIBLE; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.DisplayAdjustments.DEFAULT_DISPLAY_ADJUSTMENTS; +import static android.view.WindowInsets.Type.ime; import static com.android.internal.config.sysui.SystemUiDeviceConfigFlags.HOME_BUTTON_LONG_PRESS_DURATION_MS; import static com.android.systemui.navigationbar.NavigationBar.NavBarActionEvent.NAVBAR_ASSIST_LONGPRESS; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -57,6 +60,7 @@ import android.view.Display; import android.view.DisplayInfo; import android.view.MotionEvent; import android.view.View; +import android.view.WindowInsets; import android.view.WindowManager; import android.view.WindowMetrics; import android.view.accessibility.AccessibilityManager; @@ -83,6 +87,7 @@ import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.NotificationShadeDepthController; import com.android.systemui.statusbar.phone.AutoHideController; import com.android.systemui.statusbar.phone.LightBarController; +import com.android.systemui.statusbar.phone.NotificationShadeWindowView; import com.android.systemui.statusbar.phone.ShadeController; import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.policy.DeviceProvisionedController; @@ -148,6 +153,8 @@ public class NavigationBarTest extends SysuiTestCase { private InputMethodManager mInputMethodManager; @Mock private AssistManager mAssistManager; + @Mock + private StatusBar mStatusBar; @Rule public final LeakCheckedTest.SysuiLeakCheck mLeakCheck = new LeakCheckedTest.SysuiLeakCheck(); @@ -255,6 +262,11 @@ public class NavigationBarTest extends SysuiTestCase { // Create default & external NavBar fragment. NavigationBar defaultNavBar = mNavigationBar; NavigationBar externalNavBar = mExternalDisplayNavigationBar; + NotificationShadeWindowView mockShadeWindowView = mock(NotificationShadeWindowView.class); + WindowInsets windowInsets = new WindowInsets.Builder().setVisible(ime(), false).build(); + doReturn(windowInsets).when(mockShadeWindowView).getRootWindowInsets(); + doReturn(mockShadeWindowView).when(mStatusBar).getNotificationShadeWindowView(); + doReturn(true).when(mockShadeWindowView).isAttachedToWindow(); doNothing().when(defaultNavBar).checkNavBarModes(); doNothing().when(externalNavBar).checkNavBarModes(); defaultNavBar.createView(null); @@ -280,6 +292,40 @@ public class NavigationBarTest extends SysuiTestCase { assertFalse((defaultNavBar.getNavigationIconHints() & NAVIGATION_HINT_IME_SHOWN) != 0); } + @Test + public void testSetImeWindowStatusWhenKeyguardLockingAndImeInsetsChange() { + NotificationShadeWindowView mockShadeWindowView = mock(NotificationShadeWindowView.class); + doReturn(mockShadeWindowView).when(mStatusBar).getNotificationShadeWindowView(); + doReturn(true).when(mockShadeWindowView).isAttachedToWindow(); + doNothing().when(mNavigationBar).checkNavBarModes(); + mNavigationBar.createView(null); + WindowInsets windowInsets = new WindowInsets.Builder().setVisible(ime(), false).build(); + doReturn(windowInsets).when(mockShadeWindowView).getRootWindowInsets(); + + // Verify navbar altered back icon when an app is showing IME + mNavigationBar.setImeWindowStatus(DEFAULT_DISPLAY, null, IME_VISIBLE, + BACK_DISPOSITION_DEFAULT, true); + assertTrue((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_BACK_ALT) != 0); + assertTrue((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_IME_SHOWN) != 0); + + // Verify navbar didn't alter and showing back icon when the keyguard is showing without + // requesting IME insets visible. + doReturn(true).when(mStatusBar).isKeyguardShowing(); + mNavigationBar.setImeWindowStatus(DEFAULT_DISPLAY, null, IME_VISIBLE, + BACK_DISPOSITION_DEFAULT, true); + assertFalse((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_BACK_ALT) != 0); + assertFalse((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_IME_SHOWN) != 0); + + // Verify navbar altered and showing back icon when the keyguard is showing and + // requesting IME insets visible. + windowInsets = new WindowInsets.Builder().setVisible(ime(), true).build(); + doReturn(windowInsets).when(mockShadeWindowView).getRootWindowInsets(); + mNavigationBar.setImeWindowStatus(DEFAULT_DISPLAY, null, IME_VISIBLE, + BACK_DISPOSITION_DEFAULT, true); + assertTrue((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_BACK_ALT) != 0); + assertTrue((mNavigationBar.getNavigationIconHints() & NAVIGATION_HINT_IME_SHOWN) != 0); + } + @Test public void testA11yEventAfterDetach() { View v = mNavigationBar.createView(null); @@ -313,7 +359,7 @@ public class NavigationBarTest extends SysuiTestCase { Optional.of(mock(Pip.class)), Optional.of(mock(LegacySplitScreen.class)), Optional.of(mock(Recents.class)), - () -> Optional.of(mock(StatusBar.class)), + () -> Optional.of(mStatusBar), mock(ShadeController.class), mock(NotificationRemoteInputManager.class), mock(NotificationShadeDepthController.class), diff --git a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java index d74b3d79d6aab..cb76d8325c4b9 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -548,9 +548,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub */ private InputMethodSubtype mCurrentSubtype; - // Was the keyguard locked when this client became current? - private boolean mCurClientInKeyguard; - /** * {@code true} if the IME has not been mostly hidden via {@link android.view.InsetsController} */ @@ -2390,14 +2387,9 @@ public class InputMethodManagerService extends IInputMethodManager.Stub mImeHiddenByDisplayPolicy = false; if (mCurClient != cs) { - // Was the keyguard locked when switching over to the new client? - mCurClientInKeyguard = isKeyguardLocked(); // If the client is changing, we need to switch over to the new // one. unbindCurrentClientLocked(UnbindReason.SWITCH_CLIENT); - if (DEBUG) Slog.v(TAG, "switching to client: client=" - + cs.client.asBinder() + " keyguard=" + mCurClientInKeyguard); - // If the screen is on, inform the new client it is active if (mIsInteractive) { scheduleSetActiveToClient(cs, true /* active */, false /* fullscreen */, @@ -2905,10 +2897,6 @@ public class InputMethodManagerService extends IInputMethodManager.Stub // all updateSystemUi happens on system previlege. final long ident = Binder.clearCallingIdentity(); try { - // apply policy for binder calls - if (vis != 0 && isKeyguardLocked() && !mCurClientInKeyguard) { - vis = 0; - } if (!mCurPerceptible) { vis &= ~InputMethodService.IME_VISIBLE; }