From fdf80338022a51963f4033b5bc36375c71d9ebcc Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Thu, 7 Mar 2019 17:29:55 -0800 Subject: [PATCH] Fixed the order of notifications when Bubbles are present Bubbles were still counted as showing in the shade, which lead to the ordering logic not working properly anymore. Test: atest SystemUITests Fixes: 126580322 Change-Id: I84e7c06eb9ad91d21dc4ec2430ab71a5987563ad --- .../com/android/systemui/bubbles/BubbleData.java | 6 ++++-- .../NotificationViewHierarchyManager.java | 15 +++++++++++++-- .../stack/NotificationListContainer.java | 8 ++++++++ .../stack/NotificationStackScrollLayout.java | 5 +++++ .../NotificationViewHierarchyManagerTest.java | 3 ++- 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java index 5002f5cce751f..36098352a7ee1 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleData.java @@ -17,6 +17,7 @@ package com.android.systemui.bubbles; import androidx.annotation.Nullable; +import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import java.util.Collection; @@ -29,12 +30,13 @@ import javax.inject.Singleton; * Keeps track of active bubbles. */ @Singleton -class BubbleData { +public class BubbleData { private HashMap mBubbles = new HashMap<>(); + @VisibleForTesting @Inject - BubbleData() {} + public BubbleData() {} /** * The set of bubbles. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index ee5ac7c602aa8..2e35f0686d559 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -22,8 +22,10 @@ import android.os.Trace; import android.util.Log; import android.view.View; import android.view.ViewGroup; +import android.view.ViewParent; import com.android.systemui.R; +import com.android.systemui.bubbles.BubbleData; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; @@ -74,6 +76,7 @@ public class NotificationViewHierarchyManager { * possible. */ private final boolean mAlwaysExpandNonGroupedNotification; + private final BubbleData mBubbleData; private NotificationPresenter mPresenter; private NotificationListContainer mListContainer; @@ -85,7 +88,8 @@ public class NotificationViewHierarchyManager { VisualStabilityManager visualStabilityManager, StatusBarStateController statusBarStateController, NotificationEntryManager notificationEntryManager, - Lazy shadeController) { + Lazy shadeController, + BubbleData bubbleData) { mLockscreenUserManager = notificationLockscreenUserManager; mGroupManager = groupManager; mVisualStabilityManager = visualStabilityManager; @@ -95,6 +99,7 @@ public class NotificationViewHierarchyManager { Resources res = context.getResources(); mAlwaysExpandNonGroupedNotification = res.getBoolean(R.bool.config_alwaysExpandNonGroupedNotifications); + mBubbleData = bubbleData; } public void setUpWithPresenter(NotificationPresenter presenter, @@ -114,7 +119,8 @@ public class NotificationViewHierarchyManager { final int N = activeNotifications.size(); for (int i = 0; i < N; i++) { NotificationEntry ent = activeNotifications.get(i); - if (ent.isRowDismissed() || ent.isRowRemoved()) { + if (ent.isRowDismissed() || ent.isRowRemoved() + || (mBubbleData.getBubble(ent.key) != null && !ent.showInShadeWhenBubble())) { // we don't want to update removed notifications because they could // temporarily become children if they were isolated before. continue; @@ -185,6 +191,11 @@ public class NotificationViewHierarchyManager { if (v.getParent() == null) { mVisualStabilityManager.notifyViewAddition(v); mListContainer.addContainerView(v); + } else if (!mListContainer.containsView(v)) { + // the view is added somewhere else. Let's make sure + // the ordering works properly below, by excluding these + toShow.remove(v); + i--; } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationListContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationListContainer.java index f771be043c075..212808dae8e3e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationListContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationListContainer.java @@ -176,4 +176,12 @@ public interface NotificationListContainer extends ExpandableView.OnHeightChange * @param row The notification to bind. */ default void bindRow(ExpandableNotificationRow row) {} + + /** + * Does this list contain a given view. True by default is fine, since we only ask this if the + * view has a parent. + */ + default boolean containsView(View v) { + return true; + } } 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 a54de5f0f5a99..04d66a51ad803 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 @@ -3272,6 +3272,11 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd }); } + @Override + public boolean containsView(View v) { + return v.getParent() == this; + } + @Override @ShadeViewRefactor(RefactorComponent.STATE_RESOLVER) public void applyExpandAnimationParams(ExpandAnimationParameters params) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java index 6b2e05b23ca76..2172a125ba850 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -37,6 +37,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.Dependency; import com.android.systemui.InitController; import com.android.systemui.SysuiTestCase; +import com.android.systemui.bubbles.BubbleData; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; @@ -97,7 +98,7 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { mViewHierarchyManager = new NotificationViewHierarchyManager(mContext, mLockscreenUserManager, mGroupManager, mVisualStabilityManager, mock(StatusBarStateControllerImpl.class), mEntryManager, - () -> mShadeController); + () -> mShadeController, new BubbleData()); Dependency.get(InitController.class).executePostInitTasks(); mViewHierarchyManager.setUpWithPresenter(mPresenter, mListContainer); }