From 35fd336aa4c1a8fb751967a764b973676cff4bc2 Mon Sep 17 00:00:00 2001 From: Justin Weir Date: Thu, 1 Jun 2023 14:20:03 -0400 Subject: [PATCH] Remove CentralSurfaces.isPanelExpanded CSI.mPanelExpanded was redundant, because the only code path that updated its value also updated NPVC.mPanelExpanded to that same value. Additionally, whenever CS updated its value, it would then tell NPVC to update the sysui state flags using its copy of the data. This change eliminates CSI's copy and getter, and it moves the flag update request into NPVC to eliminate some spaghetti code. There was a tiny chance that there was another listener for NPVC.mPanelExpanded changes that received the event before CSI and needed the sysui state flags to not have been updated yet, so I verified that that was not the case. Bug: 249277686 Test: manual Change-Id: I05a038d2be66cfb4d84662d0f18465e161564653 --- .../systemui/accessibility/SystemActions.java | 4 ++- .../NotificationPanelViewController.java | 5 +-- .../systemui/shade/ShadeViewController.kt | 2 +- .../statusbar/phone/CentralSurfaces.java | 2 -- .../CentralSurfacesCommandQueueCallbacks.java | 2 +- .../statusbar/phone/CentralSurfacesImpl.java | 31 ++++++------------- .../phone/CentralSurfacesImplTest.java | 10 +++--- 7 files changed, 22 insertions(+), 34 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/SystemActions.java b/packages/SystemUI/src/com/android/systemui/accessibility/SystemActions.java index eebc1f06c5162..2f6a68c3ff8dd 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/SystemActions.java +++ b/packages/SystemUI/src/com/android/systemui/accessibility/SystemActions.java @@ -329,7 +329,9 @@ public class SystemActions implements CoreStartable { // binder calls final Optional centralSurfacesOptional = mCentralSurfacesOptionalLazy.get(); - if (centralSurfacesOptional.map(CentralSurfaces::isPanelExpanded).orElse(false) + if (centralSurfacesOptional.isPresent() + && centralSurfacesOptional.get().getShadeViewController() != null + && centralSurfacesOptional.get().getShadeViewController().isPanelExpanded() && !centralSurfacesOptional.get().isKeyguardShowing()) { if (!mDismissNotificationShadeActionRegistered) { mA11yManager.registerSystemAction( diff --git a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java index 452fc3904c32c..1bac2aa94e32c 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/shade/NotificationPanelViewController.java @@ -2440,6 +2440,7 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump boolean isExpanded = !isFullyCollapsed() || mExpectingSynthesizedDown; if (mPanelExpanded != isExpanded) { mPanelExpanded = isExpanded; + updateSystemUiStateFlags(); mShadeExpansionStateManager.onShadeExpansionFullyChanged(isExpanded); if (!isExpanded) { mQsController.closeQsCustomizer(); @@ -2447,6 +2448,7 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump } } + @Override public boolean isPanelExpanded() { return mPanelExpanded; } @@ -3440,9 +3442,8 @@ public final class NotificationPanelViewController implements ShadeSurface, Dump Log.d(TAG, "Updating panel sysui state flags: fullyExpanded=" + isFullyExpanded() + " inQs=" + mQsController.getExpanded()); } - boolean isPanelVisible = mCentralSurfaces != null && mCentralSurfaces.isPanelExpanded(); mSysUiState - .setFlag(SYSUI_STATE_NOTIFICATION_PANEL_VISIBLE, isPanelVisible) + .setFlag(SYSUI_STATE_NOTIFICATION_PANEL_VISIBLE, mPanelExpanded) .setFlag(SYSUI_STATE_NOTIFICATION_PANEL_EXPANDED, isFullyExpanded() && !mQsController.getExpanded()) .setFlag(SYSUI_STATE_QUICK_SETTINGS_EXPANDED, diff --git a/packages/SystemUI/src/com/android/systemui/shade/ShadeViewController.kt b/packages/SystemUI/src/com/android/systemui/shade/ShadeViewController.kt index 0548180807aba..9c80e0250ee91 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/ShadeViewController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/ShadeViewController.kt @@ -60,7 +60,7 @@ interface ShadeViewController { * Returns whether the shade height is greater than zero or the shade is expecting a synthesized * down event. */ - @get:Deprecated("use {@link #isExpanded()} instead") val isPanelExpanded: Boolean + val isPanelExpanded: Boolean /** Returns whether the shade is fully expanded in either QS or QQS. */ val isShadeFullyExpanded: Boolean diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfaces.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfaces.java index 0929a4c906130..f0b82f477c4ad 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfaces.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfaces.java @@ -224,8 +224,6 @@ public interface CentralSurfaces extends Dumpable, LifecycleOwner { NotificationPresenter getPresenter(); - boolean isPanelExpanded(); - /** * Used to dispatch initial touch events before crossing the threshold to pull down the * notification shade. After that, since the launcher window is set to slippery, input diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java index 337acb6ef88ce..5e0cfd6e4c335 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesCommandQueueCallbacks.java @@ -545,7 +545,7 @@ public class CentralSurfacesCommandQueueCallbacks implements CommandQueue.Callba @Override public void togglePanel() { - if (mCentralSurfaces.isPanelExpanded()) { + if (mShadeViewController.isPanelExpanded()) { mShadeController.animateCollapseShade(); } else { mShadeController.animateExpandShade(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java index a020da584e00c..cb9a88b247491 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/CentralSurfacesImpl.java @@ -627,7 +627,6 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { private final UserSwitcherController mUserSwitcherController; private final LifecycleRegistry mLifecycle = new LifecycleRegistry(this); protected final BatteryController mBatteryController; - protected boolean mPanelExpanded; private UiModeManager mUiModeManager; private LogMaker mStatusBarStateLog; protected final NotificationIconAreaController mNotificationIconAreaController; @@ -1522,22 +1521,15 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { @VisibleForTesting void onShadeExpansionFullyChanged(Boolean isExpanded) { - if (mPanelExpanded != isExpanded) { - mPanelExpanded = isExpanded; - if (getShadeViewController() != null) { - // Needed to update SYSUI_STATE_NOTIFICATION_PANEL_VISIBLE - getShadeViewController().updateSystemUiStateFlags(); - } - if (isExpanded && mStatusBarStateController.getState() != StatusBarState.KEYGUARD) { - if (DEBUG) { - Log.v(TAG, "clearing notification effects from Height"); - } - clearNotificationEffects(); + if (isExpanded && mStatusBarStateController.getState() != StatusBarState.KEYGUARD) { + if (DEBUG) { + Log.v(TAG, "clearing notification effects from Height"); } + clearNotificationEffects(); + } - if (!isExpanded) { - mRemoteInputManager.onPanelCollapsed(); - } + if (!isExpanded) { + mRemoteInputManager.onPanelCollapsed(); } } @@ -1858,11 +1850,6 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { } } - @Override - public boolean isPanelExpanded() { - return mPanelExpanded; - } - /** * Called when another window is about to transfer it's input focus. */ @@ -2981,7 +2968,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { if (mShadeSurface.isTracking()) { mNotificationShadeWindowViewController.cancelCurrentTouch(); } - if (mPanelExpanded && mState == StatusBarState.SHADE) { + if (mShadeSurface.isPanelExpanded() && mState == StatusBarState.SHADE) { mShadeController.animateCollapseShade(); } } @@ -3334,7 +3321,7 @@ public class CentralSurfacesImpl implements CoreStartable, CentralSurfaces { mScrimController.setExpansionAffectsAlpha(!unlocking); if (mAlternateBouncerInteractor.isVisibleState()) { - if ((!isOccluded() || isPanelExpanded()) + if ((!isOccluded() || mShadeSurface.isPanelExpanded()) && (mState == StatusBarState.SHADE || mState == StatusBarState.SHADE_LOCKED || mTransitionToFullShadeProgress > 0f)) { mScrimController.transitionTo(ScrimState.AUTH_SCRIMMED_SHADE); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java index 4fb5b0735fcc3..c254783ed81b4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/CentralSurfacesImplTest.java @@ -1059,7 +1059,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { // GIVEN device occluded and panel is NOT expanded mCentralSurfaces.setBarStateForTest(SHADE); // occluding on LS has StatusBarState = SHADE when(mKeyguardStateController.isOccluded()).thenReturn(true); - mCentralSurfaces.mPanelExpanded = false; + when(mNotificationPanelViewController.isPanelExpanded()).thenReturn(false); mCentralSurfaces.updateScrimController(); @@ -1073,7 +1073,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { // GIVEN device occluded and qs IS expanded mCentralSurfaces.setBarStateForTest(SHADE); // occluding on LS has StatusBarState = SHADE when(mKeyguardStateController.isOccluded()).thenReturn(true); - mCentralSurfaces.mPanelExpanded = true; + when(mNotificationPanelViewController.isPanelExpanded()).thenReturn(true); mCentralSurfaces.updateScrimController(); @@ -1144,7 +1144,7 @@ public class CentralSurfacesImplTest extends SysuiTestCase { @Test public void collapseShade_callsanimateCollapseShade_whenExpanded() { // GIVEN the shade is expanded - mCentralSurfaces.onShadeExpansionFullyChanged(true); + when(mNotificationPanelViewController.isPanelExpanded()).thenReturn(true); mCentralSurfaces.setBarStateForTest(SHADE); // WHEN collapseShade is called @@ -1155,9 +1155,9 @@ public class CentralSurfacesImplTest extends SysuiTestCase { } @Test - public void collapseShade_doesNotCallanimateCollapseShade_whenCollapsed() { + public void collapseShade_doesNotCallAnimateCollapseShade_whenCollapsed() { // GIVEN the shade is collapsed - mCentralSurfaces.onShadeExpansionFullyChanged(false); + when(mNotificationPanelViewController.isPanelExpanded()).thenReturn(false); mCentralSurfaces.setBarStateForTest(SHADE); // WHEN collapseShade is called