From 5b8f812e612bb151e24fd8bd1b63ba2c2a9f834f Mon Sep 17 00:00:00 2001 From: Vinit Nayak Date: Wed, 16 Nov 2022 13:46:35 -0800 Subject: [PATCH] Use NavBarHelper to keep shared initalizing state between nav/taskbar * If we switch from taskbar to navbar and state updates, and then we switch back to taskbar, TaskbarDelegate would have old state since we stop listening for CommandQueue callbacks. * NavBarHelper outlives both so it can keep whatever state we need (and more can be added if necessary into CurrentSysuiState) and task/navbar can load from that on init Fixes: 246890611 Test: Couldn't repro bug through device interactions, but issue via adb (b/246890611#comment8) doesn't repro Change-Id: I55f888c475fc5d17bd44fb8df73ad5022af421e2 --- .../systemui/navigationbar/NavBarHelper.java | 44 +++++++++++++++-- .../systemui/navigationbar/NavigationBar.java | 10 ++++ .../navigationbar/TaskbarDelegate.java | 8 +++ .../navigationbar/NavBarHelperTest.java | 49 ++++++++++++++++++- .../navigationbar/NavigationBarTest.java | 8 ++- 5 files changed, 114 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java index 33021e3cde47e..2c0745bf0f0df 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavBarHelper.java @@ -16,6 +16,8 @@ package com.android.systemui.navigationbar; +import static android.app.StatusBarManager.WINDOW_NAVIGATION_BAR; +import static android.app.StatusBarManager.WindowVisibleState; import static android.provider.Settings.Secure.ACCESSIBILITY_BUTTON_MODE_FLOATING_MENU; import static android.view.WindowInsetsController.APPEARANCE_LOW_PROFILE_BARS; import static android.view.WindowInsetsController.APPEARANCE_OPAQUE_NAVIGATION_BARS; @@ -58,6 +60,7 @@ 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.CommandQueue; import com.android.systemui.statusbar.phone.BarTransitions.TransitionMode; import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -86,7 +89,7 @@ public final class NavBarHelper implements AccessibilityButtonModeObserver.ModeChangedListener, AccessibilityButtonTargetsObserver.TargetsChangedListener, OverviewProxyService.OverviewProxyListener, NavigationModeController.ModeChangedListener, - Dumpable { + Dumpable, CommandQueue.Callbacks { private final AccessibilityManager mAccessibilityManager; private final Lazy mAssistManagerLazy; private final Lazy> mCentralSurfacesOptionalLazy; @@ -97,13 +100,18 @@ public final class NavBarHelper implements private final AccessibilityButtonTargetsObserver mAccessibilityButtonTargetsObserver; private final List mA11yEventListeners = new ArrayList<>(); private final Context mContext; - private ContentResolver mContentResolver; + private final CommandQueue mCommandQueue; + private final ContentResolver mContentResolver; private boolean mAssistantAvailable; private boolean mLongPressHomeEnabled; private boolean mAssistantTouchGestureEnabled; private int mNavBarMode; private int mA11yButtonState; + // Attributes used in NavBarHelper.CurrentSysuiState + private int mWindowStateDisplayId; + private @WindowVisibleState int mWindowState; + private final ContentObserver mAssistContentObserver = new ContentObserver( new Handler(Looper.getMainLooper())) { @Override @@ -128,8 +136,10 @@ public final class NavBarHelper implements KeyguardStateController keyguardStateController, NavigationModeController navigationModeController, UserTracker userTracker, - DumpManager dumpManager) { + DumpManager dumpManager, + CommandQueue commandQueue) { mContext = context; + mCommandQueue = commandQueue; mContentResolver = mContext.getContentResolver(); mAccessibilityManager = accessibilityManager; mAssistManagerLazy = assistManagerLazy; @@ -160,10 +170,13 @@ public final class NavBarHelper implements false, mAssistContentObserver, UserHandle.USER_ALL); updateAssistantAvailability(); updateA11yState(); + mCommandQueue.addCallback(this); + } public void destroy() { mContentResolver.unregisterContentObserver(mAssistContentObserver); + mCommandQueue.removeCallback(this); } /** @@ -333,6 +346,20 @@ public final class NavBarHelper implements || (!isKeyguardShowing && (vis & InputMethodService.IME_VISIBLE) != 0); } + @Override + public void setWindowState(int displayId, int window, int state) { + CommandQueue.Callbacks.super.setWindowState(displayId, window, state); + if (window != WINDOW_NAVIGATION_BAR) { + return; + } + mWindowStateDisplayId = displayId; + mWindowState = state; + } + + public CurrentSysuiState getCurrentSysuiState() { + return new CurrentSysuiState(); + } + /** * Callbacks will get fired once immediately after registering via * {@link #registerNavTaskStateUpdater(NavbarTaskbarStateUpdater)} @@ -342,6 +369,17 @@ public final class NavBarHelper implements void updateAssistantAvailable(boolean available); } + /** Data class to help Taskbar/Navbar initiate state correctly when switching between the two.*/ + public class CurrentSysuiState { + public final int mWindowStateDisplayId; + public final @WindowVisibleState int mWindowState; + + public CurrentSysuiState() { + mWindowStateDisplayId = NavBarHelper.this.mWindowStateDisplayId; + mWindowState = NavBarHelper.this.mWindowState; + } + } + static @TransitionMode int transitionMode(boolean isTransient, int appearance) { final int lightsOutOpaque = APPEARANCE_LOW_PROFILE_BARS | APPEARANCE_OPAQUE_NAVIGATION_BARS; if (isTransient) { diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java index 85d15dca12cbd..b91039d8d928d 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java @@ -650,6 +650,9 @@ public class NavigationBar extends ViewController implements mDisplayId = mContext.getDisplayId(); mIsOnDefaultDisplay = mDisplayId == DEFAULT_DISPLAY; + // Ensure we try to get currentSysuiState from navBarHelper before command queue callbacks + // start firing, since the latter is source of truth + parseCurrentSysuiState(); mCommandQueue.addCallback(this); mLongPressHomeEnabled = mNavBarHelper.getLongPressHomeEnabled(); mNavBarHelper.init(); @@ -937,6 +940,13 @@ public class NavigationBar extends ViewController implements setOrientedHandleSamplingRegion(null); } + private void parseCurrentSysuiState() { + NavBarHelper.CurrentSysuiState state = mNavBarHelper.getCurrentSysuiState(); + if (state.mWindowStateDisplayId == mDisplayId) { + mNavigationBarWindowState = state.mWindowState; + } + } + private void reconfigureHomeLongClick() { if (mView.getHomeButton().getCurrentView() == null) { return; diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java b/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java index 73fc21ef928c1..0d9af31639f2e 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/TaskbarDelegate.java @@ -214,6 +214,7 @@ public class TaskbarDelegate implements CommandQueue.Callbacks, return; } mDisplayId = displayId; + parseCurrentSysuiState(); mCommandQueue.addCallback(this); mOverviewProxyService.addCallback(this); mEdgeBackGestureHandler.onNavigationModeChanged( @@ -271,6 +272,13 @@ public class TaskbarDelegate implements CommandQueue.Callbacks, return mInitialized; } + private void parseCurrentSysuiState() { + NavBarHelper.CurrentSysuiState state = mNavBarHelper.getCurrentSysuiState(); + if (state.mWindowStateDisplayId == mDisplayId) { + mTaskBarWindowState = state.mWindowState; + } + } + private void updateSysuiFlags() { int a11yFlags = mNavBarHelper.getA11yButtonState(); boolean clickable = (a11yFlags & SYSUI_STATE_A11Y_BUTTON_CLICKABLE) != 0; 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 6c03730e056e6..1865ef64c5d51 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavBarHelperTest.java @@ -16,6 +16,7 @@ package com.android.systemui.navigationbar; +import static android.app.StatusBarManager.WINDOW_NAVIGATION_BAR; import static android.provider.Settings.Secure.ACCESSIBILITY_BUTTON_MODE_FLOATING_MENU; import static android.provider.Settings.Secure.ACCESSIBILITY_BUTTON_MODE_NAVIGATION_BAR; @@ -47,6 +48,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.CommandQueue; import com.android.systemui.statusbar.phone.CentralSurfaces; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -69,6 +71,10 @@ import dagger.Lazy; @SmallTest public class NavBarHelperTest extends SysuiTestCase { + private static final int DISPLAY_ID = 0; + private static final int WINDOW = WINDOW_NAVIGATION_BAR; + private static final int STATE_ID = 0; + @Mock AccessibilityManager mAccessibilityManager; @Mock @@ -93,6 +99,8 @@ public class NavBarHelperTest extends SysuiTestCase { DumpManager mDumpManager; @Mock NavBarHelper.NavbarTaskbarStateUpdater mNavbarTaskbarStateUpdater; + @Mock + CommandQueue mCommandQueue; private AccessibilityManager.AccessibilityServicesStateChangeListener mAccessibilityServicesStateChangeListener; @@ -114,7 +122,7 @@ public class NavBarHelperTest extends SysuiTestCase { mAccessibilityButtonModeObserver, mAccessibilityButtonTargetObserver, mSystemActions, mOverviewProxyService, mAssistManagerLazy, () -> Optional.of(mock(CentralSurfaces.class)), mock(KeyguardStateController.class), - mNavigationModeController, mUserTracker, mDumpManager); + mNavigationModeController, mUserTracker, mDumpManager, mCommandQueue); } @@ -241,6 +249,45 @@ public class NavBarHelperTest extends SysuiTestCase { ACCESSIBILITY_BUTTON_CLICKABLE_STATE); } + @Test + public void registerCommandQueueCallbacks() { + mNavBarHelper.init(); + verify(mCommandQueue, times(1)).addCallback(any()); + } + + @Test + public void saveMostRecentSysuiState() { + mNavBarHelper.init(); + mNavBarHelper.setWindowState(DISPLAY_ID, WINDOW, STATE_ID); + NavBarHelper.CurrentSysuiState state1 = mNavBarHelper.getCurrentSysuiState(); + + // Update window state + int newState = STATE_ID + 1; + mNavBarHelper.setWindowState(DISPLAY_ID, WINDOW, newState); + NavBarHelper.CurrentSysuiState state2 = mNavBarHelper.getCurrentSysuiState(); + + // Ensure we get most recent state back + assertThat(state1.mWindowState).isNotEqualTo(state2.mWindowState); + assertThat(state1.mWindowStateDisplayId).isEqualTo(state2.mWindowStateDisplayId); + assertThat(state2.mWindowState).isEqualTo(newState); + } + + @Test + public void ignoreNonNavbarSysuiState() { + mNavBarHelper.init(); + mNavBarHelper.setWindowState(DISPLAY_ID, WINDOW, STATE_ID); + NavBarHelper.CurrentSysuiState state1 = mNavBarHelper.getCurrentSysuiState(); + + // Update window state for other window type + int newState = STATE_ID + 1; + mNavBarHelper.setWindowState(DISPLAY_ID, WINDOW + 1, newState); + NavBarHelper.CurrentSysuiState state2 = mNavBarHelper.getCurrentSysuiState(); + + // Ensure we get first state back + assertThat(state2.mWindowState).isEqualTo(state1.mWindowState); + assertThat(state2.mWindowState).isNotEqualTo(newState); + } + private List createFakeShortcutTargets() { return new ArrayList<>(List.of("a", "b", "c", "d")); } 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 c1fa9b39f50b1..f43a34f6e89b7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/navigationbar/NavigationBarTest.java @@ -247,7 +247,7 @@ public class NavigationBarTest extends SysuiTestCase { mSystemActions, mOverviewProxyService, () -> mock(AssistManager.class), () -> Optional.of(mCentralSurfaces), mKeyguardStateController, mock(NavigationModeController.class), - mock(UserTracker.class), mock(DumpManager.class))); + mock(UserTracker.class), mock(DumpManager.class), mock(CommandQueue.class))); mNavigationBar = createNavBar(mContext); mExternalDisplayNavigationBar = createNavBar(mSysuiTestableContextExternal); }); @@ -438,6 +438,12 @@ public class NavigationBarTest extends SysuiTestCase { verify(mNavigationBarView).setVisibility(View.INVISIBLE); } + @Test + public void testOnInit_readCurrentSysuiState() { + mNavigationBar.init(); + verify(mNavBarHelper, times(1)).getCurrentSysuiState(); + } + private NavigationBar createNavBar(Context context) { DeviceProvisionedController deviceProvisionedController = mock(DeviceProvisionedController.class);