From 91d0f10a36667ba9fec5caff620560792534f3ee Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 18 Sep 2018 17:39:55 -0400 Subject: Fix notifications showing on keyguard The root cause here is that some components (NotificationKeyguardUserManager, NotificationStackScrollLayout, NotificationShelf) have subtle dependencies about how they update based on StatusBarState which used to be satisfied. When we migrated to StatusBarStateController the dependencies were broken sometimes. The fix here is multifactorial: 1. Introduce a ranking to StatusBarStateController listeners so that we can unwind the dependencies for now. (This should be fixed long-term) 2. Move NSSL's actual notification updating to a new onStatePostChange() method of the listeners 3. Explicitly call updatePublicMode() when hiding the keyguard because SbStateController does not give callbacks when setting a state to the same value. Will move updatePublicMode into NotificationKeyguardUserManager in a later commit to reduce the surface area here. Bug: 115739177 Change-Id: I97786619a1843922aeedcdb51e067a7c46cacaec Fixes: 114297820 Test: manual --- .../systemui/statusbar/NotificationShelf.java | 3 +- .../statusbar/StatusBarStateController.java | 93 +++++++++++++++++++--- .../stack/NotificationStackScrollLayout.java | 19 ++++- .../systemui/statusbar/phone/StatusBar.java | 11 ++- .../statusbar/phone/StatusBarWindowController.java | 3 +- 5 files changed, 113 insertions(+), 16 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelf.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelf.java index f7615e6448db..d8f7b6142dd8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelf.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelf.java @@ -121,7 +121,8 @@ public class NotificationShelf extends ActivatableNotificationView implements @Override protected void onAttachedToWindow() { super.onAttachedToWindow(); - Dependency.get(StatusBarStateController.class).addListener(mStateListener); + Dependency.get(StatusBarStateController.class) + .addListener(mStateListener, StatusBarStateController.RANK_SHELF); } @Override diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateController.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateController.java index 7a69f2eee06c..d6719f0a03e1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateController.java @@ -16,42 +16,70 @@ package com.android.systemui.statusbar; +import static java.lang.annotation.RetentionPolicy.SOURCE; + +import android.annotation.IntDef; import android.util.ArraySet; +import android.util.Log; +import com.android.internal.annotations.GuardedBy; +import java.lang.annotation.Retention; +import java.util.ArrayList; +import java.util.Comparator; /** * Tracks and reports on {@link StatusBarState}. */ public class StatusBarStateController { + private static final String TAG = "SbStateController"; private static final int MAX_STATE = StatusBarState.FULLSCREEN_USER_SWITCHER; private static final int MIN_STATE = StatusBarState.SHADE; - private final ArraySet mListeners = new ArraySet<>(); + private static final Comparator mComparator + = (o1, o2) -> Integer.compare(o1.rank, o2.rank); + + private final ArrayList mListeners = new ArrayList<>(); private int mState; private int mLastState; private boolean mLeaveOpenOnKeyguardHide; + // TODO: b/115739177 (remove this explicit ordering if we can) + @Retention(SOURCE) + @IntDef({RANK_STATUS_BAR, RANK_STATUS_BAR_WINDOW_CONTROLLER, RANK_STACK_SCROLLER, RANK_SHELF}) + public @interface SbStateListenerRank {} + // This is the set of known dependencies when updating StatusBarState + public static final int RANK_STATUS_BAR = 0; + public static final int RANK_STATUS_BAR_WINDOW_CONTROLLER = 1; + public static final int RANK_STACK_SCROLLER = 2; + public static final int RANK_SHELF = 3; + public int getState() { return mState; } - public void setState(int state) { + public boolean setState(int state) { if (state > MAX_STATE || state < MIN_STATE) { throw new IllegalArgumentException("Invalid state " + state); } if (state == mState) { - return; + return false; } synchronized (mListeners) { - for (StateListener listener : new ArraySet<>(mListeners)) { - listener.onStatePreChange(mState, state); + for (RankedListener rl : new ArrayList<>(mListeners)) { + rl.listener.onStatePreChange(mState, state); } mLastState = mState; mState = state; - for (StateListener listener : new ArraySet<>(mListeners)) { - listener.onStateChanged(mState); + for (RankedListener rl : new ArrayList<>(mListeners)) { + rl.listener.onStateChanged(mState); + } + + for (RankedListener rl : new ArrayList<>(mListeners)) { + rl.listener.onStatePostChange(); } } + + return true; } public boolean goingToFullShade() { @@ -72,20 +100,67 @@ public class StatusBarStateController { public void addListener(StateListener listener) { synchronized (mListeners) { - mListeners.add(listener); + addListenerInternalLocked(listener, Integer.MAX_VALUE); } } + /** + * Add a listener and a rank based on the priority of this message + * @param listener the listener + * @param rank the order in which you'd like to be called. Ranked listeners will be + * notified before unranked, and we will sort ranked listeners from low to high + * + * @deprecated This method exists only to solve latent inter-dependencies from refactoring + * StatusBarState out of StatusBar.java. Any new listeners should be built not to need ranking + * (i.e., they are non-dependent on the order of operations of StatusBarState listeners). + */ + public void addListener(StateListener listener, @SbStateListenerRank int rank) { + synchronized (mListeners) { + addListenerInternalLocked(listener, rank); + } + } + + @GuardedBy("mListeners") + private void addListenerInternalLocked(StateListener listener, int rank) { + // Protect against double-subscribe + for (RankedListener rl : mListeners) { + if (rl.listener.equals(listener)) { + return; + } + } + + RankedListener rl = new RankedListener(listener, rank); + mListeners.add(rl); + mListeners.sort(mComparator); + } + public void removeListener(StateListener listener) { synchronized (mListeners) { - mListeners.remove(listener); + mListeners.removeIf((it) -> it.listener.equals(listener)); } } + public static String describe(int state) { + return StatusBarState.toShortString(state); + } + public interface StateListener { public default void onStatePreChange(int oldState, int newState) { } + public default void onStatePostChange() { + } + public void onStateChanged(int newState); } + + private class RankedListener { + private final StateListener listener; + private final int rank; + + private RankedListener(StateListener l, int r) { + listener = l; + rank = r; + } + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index 52844fe0a54a..2fab58b97e93 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -620,7 +620,8 @@ public class NotificationStackScrollLayout extends ViewGroup @ShadeViewRefactor(RefactorComponent.SHADE_VIEW) protected void onAttachedToWindow() { super.onAttachedToWindow(); - Dependency.get(StatusBarStateController.class).addListener(mStateListener); + Dependency.get(StatusBarStateController.class) + .addListener(mStateListener, StatusBarStateController.RANK_STACK_SCROLLER); Dependency.get(ConfigurationController.class).addCallback(this); } @@ -4248,8 +4249,9 @@ public class NotificationStackScrollLayout extends ViewGroup mDimAnimator.addUpdateListener(mDimUpdateListener); mDimAnimator.start(); } + @ShadeViewRefactor(RefactorComponent.SHADE_VIEW) - public void setHideSensitive(boolean hideSensitive, boolean animate) { + private void setHideSensitive(boolean hideSensitive, boolean animate) { if (hideSensitive != mAmbientState.isHideSensitive()) { int childCount = getChildCount(); for (int i = 0; i < childCount; i++) { @@ -5008,8 +5010,12 @@ public class NotificationStackScrollLayout extends ViewGroup private void setStatusBarState(int statusBarState) { mStatusBarState = statusBarState; mAmbientState.setStatusBarState(statusBarState); + } + + private void onStatePostChange() { boolean onKeyguard = onKeyguard(); boolean publicMode = mLockscreenUserManager.isAnyProfilePublicMode(); + if (mHeadsUpAppearanceController != null) { mHeadsUpAppearanceController.setPublicMode(publicMode); } @@ -5026,6 +5032,8 @@ public class NotificationStackScrollLayout extends ViewGroup updateFooter(); updateChildren(); onUpdateRowStates(); + + mEntryManager.updateNotifications(); } @ShadeViewRefactor(RefactorComponent.SHADE_VIEW) @@ -5949,5 +5957,10 @@ public class NotificationStackScrollLayout extends ViewGroup public void onStateChanged(int newState) { setStatusBarState(newState); } - }; + + @Override + public void onStatePostChange() { + NotificationStackScrollLayout.this.onStatePostChange(); + } + }; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java index 7ec4db21b694..aa000fd84644 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java @@ -632,7 +632,7 @@ public class StatusBar extends SystemUI implements DemoMode, mColorExtractor = Dependency.get(SysuiColorExtractor.class); mColorExtractor.addOnColorsChangedListener(this); - mStatusBarStateController.addListener(this); + mStatusBarStateController.addListener(this, StatusBarStateController.RANK_STATUS_BAR); mWindowManager = (WindowManager) mContext.getSystemService(Context.WINDOW_SERVICE); @@ -3456,7 +3456,14 @@ public class StatusBar extends SystemUI implements DemoMode, mIsKeyguard = false; Trace.beginSection("StatusBar#hideKeyguard"); boolean staying = mStatusBarStateController.leaveOpenOnKeyguardHide(); - mStatusBarStateController.setState(StatusBarState.SHADE); + if (!(mStatusBarStateController.setState(StatusBarState.SHADE))) { + //TODO: StatusBarStateController should probably know about hiding the keyguard and + // notify listeners. + + // If the state didn't change, we may still need to update public mode + updatePublicMode(); + mEntryManager.updateNotifications(); + } View viewToClick = null; if (mStatusBarStateController.leaveOpenOnKeyguardHide()) { if (!mKeyguardRequested) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarWindowController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarWindowController.java index 167bba60b390..200964c760a0 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarWindowController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarWindowController.java @@ -93,7 +93,8 @@ public class StatusBarWindowController implements Callback, Dumpable, Configurat mKeyguardScreenRotation = shouldEnableKeyguardScreenRotation(); mDozeParameters = dozeParameters; mScreenBrightnessDoze = mDozeParameters.getScreenBrightnessDoze(); - Dependency.get(StatusBarStateController.class).addListener(mStateListener); + Dependency.get(StatusBarStateController.class).addListener( + mStateListener, StatusBarStateController.RANK_STATUS_BAR_WINDOW_CONTROLLER); Dependency.get(ConfigurationController.class).addCallback(this); } -- cgit v1.2.3-59-g8ed1b