From 355c04e0a778cada3db55c6c3a0e059b2add49e9 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 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 (cherry picked from commit 1e5c277f7bb63b125e77ea3ef76cf58071d6ef86) Merged-In: I24c511d068ab371d6984201327a561322fc26bd8 Change-Id: I24c511d068ab371d6984201327a561322fc26bd8 --- .../systemui/navigationbar/NavBarHelper.java | 21 +++++++ .../systemui/navigationbar/NavigationBar.java | 6 +- .../navigationbar/TaskbarDelegate.java | 4 +- .../navigationbar/NavBarHelperTest.java | 8 ++- .../navigationbar/NavigationBarTest.java | 58 ++++++++++++++++++- .../InputMethodManagerService.java | 12 ---- 6 files changed, 88 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java index 23062d805fd6f..25337b6d52a1c 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java @@ -24,12 +24,15 @@ import static com.android.systemui.shared.system.QuickStepContract.SYSUI_STATE_A import android.content.ContentResolver; import android.content.Context; import android.database.ContentObserver; +import android.inputmethodservice.InputMethodService; import android.net.Uri; import android.os.Bundle; import android.os.Handler; import android.os.Looper; import android.os.UserHandle; import android.provider.Settings; +import android.view.View; +import android.view.WindowInsets; import android.view.accessibility.AccessibilityManager; import androidx.annotation.NonNull; @@ -42,12 +45,14 @@ import com.android.systemui.dump.DumpManager; import com.android.systemui.recents.OverviewProxyService; import com.android.systemui.settings.UserTracker; import com.android.systemui.shared.system.QuickStepContract; +import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.policy.AccessibilityManagerWrapper; import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import javax.inject.Inject; @@ -69,6 +74,7 @@ public final class NavBarHelper implements Dumpable { private final AccessibilityManager mAccessibilityManager; private final Lazy mAssistManagerLazy; + private final Lazy> mStatusBarOptionalLazy; private final UserTracker mUserTracker; private final AccessibilityButtonModeObserver mAccessibilityButtonModeObserver; private final List mA11yEventListeners = new ArrayList<>(); @@ -98,12 +104,14 @@ public final class NavBarHelper implements AccessibilityButtonModeObserver accessibilityButtonModeObserver, OverviewProxyService overviewProxyService, Lazy assistManagerLazy, + Lazy> statusBarOptionalLazy, NavigationModeController navigationModeController, UserTracker userTracker, DumpManager dumpManager) { mContext = context; mAccessibilityManager = accessibilityManager; mAssistManagerLazy = assistManagerLazy; + mStatusBarOptionalLazy = statusBarOptionalLazy; mUserTracker = userTracker; accessibilityManagerWrapper.addCallback( accessibilityManager1 -> NavBarHelper.this.dispatchA11yEventUpdate()); @@ -231,6 +239,19 @@ public final class NavBarHelper implements updateAssitantAvailability(); } + /** + * @return Whether the IME is shown on top of the screen given the {@code vis} flag of + * {@link InputMethodService} and the keyguard states. + */ + public boolean isImeShown(int vis) { + View shadeWindowView = mStatusBarOptionalLazy.get().get().getNotificationShadeWindowView(); + boolean isKeyguardShowing = mStatusBarOptionalLazy.get().get().isKeyguardShowing(); + boolean imeVisibleOnShade = shadeWindowView != null && shadeWindowView.isAttachedToWindow() + && shadeWindowView.getRootWindowInsets().isVisible(WindowInsets.Type.ime()); + return imeVisibleOnShade + || (!isKeyguardShowing && (vis & InputMethodService.IME_VISIBLE) != 0); + } + /** * Callbacks will get fired once immediately after registering via * {@link #registerNavTaskStateUpdater(NavbarTaskbarStateUpdater)} diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java index 1219c7a343a62..03bceacdfbdcc 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java @@ -72,7 +72,6 @@ import android.graphics.Insets; import android.graphics.PixelFormat; import android.graphics.Rect; import android.graphics.RectF; -import android.inputmethodservice.InputMethodService; import android.os.Binder; import android.os.Bundle; import android.os.Handler; @@ -884,7 +883,8 @@ public class NavigationBar implements View.OnAttachStateChangeListener, if (displayId != mDisplayId) { return; } - boolean imeShown = (vis & InputMethodService.IME_VISIBLE) != 0; + boolean imeShown = mNavBarHelper.isImeShown(vis); + showImeSwitcher = imeShown && showImeSwitcher; int hints = Utilities.calculateBackDispositionHints(mNavigationIconHints, backDisposition, imeShown, showImeSwitcher); if (hints == mNavigationIconHints) return; @@ -1757,4 +1757,4 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mInputMethodManager); } } -} \ No newline at end of file +} diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java b/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java index fb9b8eb23de10..8fb394c06ba11 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java @@ -41,7 +41,6 @@ import android.content.ComponentCallbacks; import android.content.Context; import android.content.res.Configuration; import android.hardware.display.DisplayManager; -import android.inputmethodservice.InputMethodService; import android.os.IBinder; import android.os.RemoteException; import android.util.Log; @@ -265,7 +264,8 @@ public class TaskbarDelegate implements CommandQueue.Callbacks, @Override public void setImeWindowStatus(int displayId, IBinder token, int vis, int backDisposition, boolean showImeSwitcher) { - boolean imeShown = (vis & InputMethodService.IME_VISIBLE) != 0; + boolean imeShown = mNavBarHelper.isImeShown(vis); + showImeSwitcher = imeShown && showImeSwitcher; int hints = Utilities.calculateBackDispositionHints(mNavigationIconHints, backDisposition, imeShown, showImeSwitcher); if (hints != mNavigationIconHints) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java index ed5b8cb1a7fe0..a445d6f4e5c5b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java @@ -18,6 +18,7 @@ package com.android.systemui.navigationbar; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -34,6 +35,7 @@ import com.android.systemui.assist.AssistManager; import com.android.systemui.dump.DumpManager; import com.android.systemui.recents.OverviewProxyService; import com.android.systemui.settings.UserTracker; +import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.policy.AccessibilityManagerWrapper; import org.junit.Before; @@ -42,6 +44,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Optional; + import dagger.Lazy; @RunWith(AndroidJUnit4.class) @@ -82,8 +86,8 @@ public class NavBarHelperTest extends SysuiTestCase { mNavBarHelper = new NavBarHelper(mContext, mAccessibilityManager, mAccessibilityManagerWrapper, mAccessibilityButtonModeObserver, - mOverviewProxyService, mAssistManagerLazy, mNavigationModeController, - mUserTracker, mDumpManager); + mOverviewProxyService, mAssistManagerLazy, () -> Optional.of(mock(StatusBar.class)), + mNavigationModeController, mUserTracker, mDumpManager); } 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 776b6aa2ef380..5003013358be1 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; @@ -72,6 +76,7 @@ import com.android.systemui.accessibility.AccessibilityButtonModeObserver; import com.android.systemui.accessibility.SystemActions; import com.android.systemui.assist.AssistManager; import com.android.systemui.broadcast.BroadcastDispatcher; +import com.android.systemui.dump.DumpManager; import com.android.systemui.model.SysUiState; import com.android.systemui.navigationbar.gestural.EdgeBackGestureHandler; import com.android.systemui.plugins.statusbar.StatusBarStateController; @@ -83,8 +88,10 @@ 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.AccessibilityManagerWrapper; import com.android.systemui.statusbar.policy.DeviceProvisionedController; import com.android.systemui.statusbar.policy.KeyguardStateController; import com.android.systemui.utils.leaks.LeakCheckedTest; @@ -98,6 +105,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.Spy; import java.util.Optional; @@ -130,7 +138,6 @@ public class NavigationBarTest extends SysuiTestCase { EdgeBackGestureHandler.Factory mEdgeBackGestureHandlerFactory; @Mock EdgeBackGestureHandler mEdgeBackGestureHandler; - @Mock NavBarHelper mNavBarHelper; @Mock private LightBarController mLightBarController; @@ -148,6 +155,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(); @@ -172,6 +181,12 @@ public class NavigationBarTest extends SysuiTestCase { mDependency.injectTestDependency(OverviewProxyService.class, mOverviewProxyService); mDependency.injectTestDependency(NavigationModeController.class, mNavigationModeController); TestableLooper.get(this).runWithLooper(() -> { + mNavBarHelper = spy(new NavBarHelper(mContext, mock(AccessibilityManager.class), + mock(AccessibilityManagerWrapper.class), + mock(AccessibilityButtonModeObserver.class), mOverviewProxyService, + () -> mock(AssistManager.class), () -> Optional.of(mStatusBar), + mock(NavigationModeController.class), mock(UserTracker.class), + mock(DumpManager.class))); mNavigationBar = createNavBar(mContext); mExternalDisplayNavigationBar = createNavBar(mSysuiTestableContextExternal); }); @@ -256,6 +271,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); @@ -281,6 +301,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); @@ -314,7 +368,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 9d66780535335..51da107af2837 100644 --- a/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java +++ b/services/core/java/com/android/server/inputmethod/InputMethodManagerService.java @@ -542,9 +542,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} */ @@ -2363,14 +2360,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 */, @@ -2875,10 +2867,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; }