From 3cb379c551a20c0b1c0036b38feb59329d365499 Mon Sep 17 00:00:00 2001 From: Kevin Han Date: Tue, 3 Dec 2019 13:31:44 -0800 Subject: [PATCH] Bind/unbind notif children content dynamically Unbind notificiation children content when they won't be visible anyway in order to save memory. Bind again lazily if we ever do actually need it. This is going into the old pipeline so we can start testing it out early, but ultimately, we also plan to put this in the new pipeline (though in a form where we try to unbind the row entirely). Bug: 145748993 Test: Post group on Notify APK. Clear notifs and confirm no weird UX Test: atest DynamicChildBindControllerTest Change-Id: I41959627a55fa7041629ce52af5539b3f0c3cecc --- .../NotificationViewHierarchyManager.java | 33 +++-- .../dagger/StatusBarDependenciesModule.java | 7 +- .../DynamicChildBindController.java | 118 ++++++++++++++++ .../stack/NotificationChildrenContainer.java | 3 +- .../NotificationViewHierarchyManagerTest.java | 11 +- .../DynamicChildBindControllerTest.java | 133 ++++++++++++++++++ 6 files changed, 281 insertions(+), 24 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/DynamicChildBindController.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/DynamicChildBindControllerTest.java diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java index 7e70c2059e60f..fe2f1f3eefc5a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java @@ -30,6 +30,7 @@ import com.android.systemui.bubbles.BubbleController; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.dagger.StatusBarModule; +import com.android.systemui.statusbar.notification.DynamicChildBindController; import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; @@ -59,11 +60,12 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle private final Handler mHandler; - //TODO: change this top >? - private final HashMap> - mTmpChildOrderMap = new HashMap<>(); + /** Re-usable map of notifications to their sorted children.*/ + private final HashMap> mTmpChildOrderMap = + new HashMap<>(); // Dependencies: + private final DynamicChildBindController mDynamicChildBindController; protected final NotificationLockscreenUserManager mLockscreenUserManager; protected final NotificationGroupManager mGroupManager; protected final VisualStabilityManager mVisualStabilityManager; @@ -105,7 +107,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle KeyguardBypassController bypassController, BubbleController bubbleController, DynamicPrivacyController privacyController, - ForegroundServiceSectionController fgsSectionController) { + ForegroundServiceSectionController fgsSectionController, + DynamicChildBindController dynamicChildBindController) { mContext = context; mHandler = mainHandler; mLockscreenUserManager = notificationLockscreenUserManager; @@ -121,6 +124,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle mBubbleController = bubbleController; mDynamicPrivacyController = privacyController; privacyController.addListener(this); + mDynamicChildBindController = dynamicChildBindController; } public void setUpWithPresenter(NotificationPresenter presenter, @@ -175,13 +179,12 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle ent.getRow().setNeedsRedaction(needsRedaction); if (mGroupManager.isChildInGroupWithSummary(ent.getSbn())) { NotificationEntry summary = mGroupManager.getGroupSummary(ent.getSbn()); - List orderedChildren = - mTmpChildOrderMap.get(summary.getRow()); + List orderedChildren = mTmpChildOrderMap.get(summary); if (orderedChildren == null) { orderedChildren = new ArrayList<>(); - mTmpChildOrderMap.put(summary.getRow(), orderedChildren); + mTmpChildOrderMap.put(summary, orderedChildren); } - orderedChildren.add(ent.getRow()); + orderedChildren.add(ent); } else { toShow.add(ent.getRow()); } @@ -260,6 +263,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle } + mDynamicChildBindController.updateChildContentViews(mTmpChildOrderMap); mVisualStabilityManager.onReorderingFinished(); // clear the map again for the next usage mTmpChildOrderMap.clear(); @@ -274,6 +278,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle private void addNotificationChildrenAndSort() { // Let's now add all notification children which are missing boolean orderChanged = false; + ArrayList orderedRows = new ArrayList<>(); for (int i = 0; i < mListContainer.getContainerChildCount(); i++) { View view = mListContainer.getContainerChildAt(i); if (!(view instanceof ExpandableNotificationRow)) { @@ -283,11 +288,11 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle ExpandableNotificationRow parent = (ExpandableNotificationRow) view; List children = parent.getNotificationChildren(); - List orderedChildren = mTmpChildOrderMap.get(parent); + List orderedChildren = mTmpChildOrderMap.get(parent.getEntry()); for (int childIndex = 0; orderedChildren != null && childIndex < orderedChildren.size(); childIndex++) { - ExpandableNotificationRow childView = orderedChildren.get(childIndex); + ExpandableNotificationRow childView = orderedChildren.get(childIndex).getRow(); if (children == null || !children.contains(childView)) { if (childView.getParent() != null) { Log.wtf(TAG, "trying to add a notification child that already has " + @@ -300,11 +305,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle parent.addChildNotification(childView, childIndex); mListContainer.notifyGroupChildAdded(childView); } + orderedRows.add(childView); } // Finally after removing and adding has been performed we can apply the order. - orderChanged |= parent.applyChildOrder(orderedChildren, mVisualStabilityManager, + orderChanged |= parent.applyChildOrder(orderedRows, mVisualStabilityManager, mEntryManager); + orderedRows.clear(); } if (orderChanged) { mListContainer.generateChildOrderChangedEvent(); @@ -323,13 +330,13 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle ExpandableNotificationRow parent = (ExpandableNotificationRow) view; List children = parent.getNotificationChildren(); - List orderedChildren = mTmpChildOrderMap.get(parent); + List orderedChildren = mTmpChildOrderMap.get(parent.getEntry()); if (children != null) { toRemove.clear(); for (ExpandableNotificationRow childRow : children) { if ((orderedChildren == null - || !orderedChildren.contains(childRow)) + || !orderedChildren.contains(childRow.getEntry())) && !childRow.keepInParent()) { toRemove.add(childRow); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java index cd5bb775de8c6..f714d981a0e0d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java @@ -32,6 +32,7 @@ import com.android.systemui.statusbar.NotificationMediaManager; import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.NotificationViewHierarchyManager; import com.android.systemui.statusbar.SmartReplyController; +import com.android.systemui.statusbar.notification.DynamicChildBindController; import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; @@ -135,7 +136,8 @@ public interface StatusBarDependenciesModule { KeyguardBypassController bypassController, BubbleController bubbleController, DynamicPrivacyController privacyController, - ForegroundServiceSectionController fgsSectionController) { + ForegroundServiceSectionController fgsSectionController, + DynamicChildBindController dynamicChildBindController) { return new NotificationViewHierarchyManager( context, mainHandler, @@ -147,7 +149,8 @@ public interface StatusBarDependenciesModule { bypassController, bubbleController, privacyController, - fgsSectionController); + fgsSectionController, + dynamicChildBindController); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/DynamicChildBindController.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/DynamicChildBindController.java new file mode 100644 index 0000000000000..059d6ffa91801 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/DynamicChildBindController.java @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification; + +import static com.android.systemui.statusbar.notification.row.NotificationContentInflater.FLAG_CONTENT_VIEW_CONTRACTED; +import static com.android.systemui.statusbar.notification.row.NotificationContentInflater.FLAG_CONTENT_VIEW_EXPANDED; +import static com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer.NUMBER_OF_CHILDREN_WHEN_CHILDREN_EXPANDED; + +import com.android.systemui.statusbar.notification.collection.NotifPipeline; +import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.RowContentBindParams; +import com.android.systemui.statusbar.notification.row.RowContentBindStage; + +import java.util.List; +import java.util.Map; + +import javax.inject.Inject; + +/** + * Controller that binds/unbinds views content views on notification group children. + * + * We currently only show a limited number of notification children even if more exist, so we + * can save memory by freeing content views when they're not visible and binding them again when + * they get close to being visible. + * + * Eventually, when {@link NotifPipeline} takes over as the new notification pipeline, we'll have + * more control over which notifications even make it to inflation in the first place and be able + * to enforce this at an earlier stage at the level of the {@link ExpandableNotificationRow}, but + * for now, we're just doing it at the level of content views. + */ +public class DynamicChildBindController { + private final RowContentBindStage mStage; + private final int mChildBindCutoff; + + @Inject + public DynamicChildBindController(RowContentBindStage stage) { + this(stage, CHILD_BIND_CUTOFF); + } + + /** + * @param childBindCutoff the cutoff where we no longer bother having content views bound + */ + DynamicChildBindController( + RowContentBindStage stage, + int childBindCutoff) { + mStage = stage; + mChildBindCutoff = childBindCutoff; + } + + /** + * Update the child content views, unbinding content views on children that won't be visible + * and binding content views on children that will be visible eventually. + * + * @param groupNotifs map of notification summaries to their children + */ + public void updateChildContentViews( + Map> groupNotifs) { + for (NotificationEntry entry : groupNotifs.keySet()) { + List children = groupNotifs.get(entry); + for (int j = 0; j < children.size(); j++) { + NotificationEntry childEntry = children.get(j); + if (j >= mChildBindCutoff) { + if (hasChildContent(childEntry)) { + freeChildContent(childEntry); + } + } else { + if (!hasChildContent(childEntry)) { + bindChildContent(childEntry); + } + } + } + } + } + + private boolean hasChildContent(NotificationEntry entry) { + ExpandableNotificationRow row = entry.getRow(); + return row.getPrivateLayout().getContractedChild() != null + || row.getPrivateLayout().getExpandedChild() != null; + } + + private void freeChildContent(NotificationEntry entry) { + RowContentBindParams params = mStage.getStageParams(entry); + params.freeContentViews(FLAG_CONTENT_VIEW_CONTRACTED); + params.freeContentViews(FLAG_CONTENT_VIEW_EXPANDED); + mStage.requestRebind(entry, null); + } + + private void bindChildContent(NotificationEntry entry) { + RowContentBindParams params = mStage.getStageParams(entry); + params.requireContentViews(FLAG_CONTENT_VIEW_CONTRACTED); + params.requireContentViews(FLAG_CONTENT_VIEW_EXPANDED); + mStage.requestRebind(entry, null); + } + + /** + * How big the buffer of extra views we keep around to be ready to show when we do need to + * dynamically inflate. + */ + private static final int EXTRA_VIEW_BUFFER_COUNT = 1; + + private static final int CHILD_BIND_CUTOFF = + NUMBER_OF_CHILDREN_WHEN_CHILDREN_EXPANDED + EXTRA_VIEW_BUFFER_COUNT; +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java index 75ceb0fa19899..d7c88e3a3f60f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java @@ -53,8 +53,7 @@ public class NotificationChildrenContainer extends ViewGroup { static final int NUMBER_OF_CHILDREN_WHEN_COLLAPSED = 2; @VisibleForTesting static final int NUMBER_OF_CHILDREN_WHEN_SYSTEM_EXPANDED = 5; - @VisibleForTesting - static final int NUMBER_OF_CHILDREN_WHEN_CHILDREN_EXPANDED = 8; + public static final int NUMBER_OF_CHILDREN_WHEN_CHILDREN_EXPANDED = 8; private static final AnimationProperties ALPHA_FADE_IN = new AnimationProperties() { private AnimationFilter mAnimationFilter = new AnimationFilter().animateAlpha(); 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 8e87e0a802ef4..cc5f149eabd3a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java @@ -41,11 +41,11 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.bubbles.BubbleController; import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper; +import com.android.systemui.statusbar.notification.DynamicChildBindController; import com.android.systemui.statusbar.notification.DynamicPrivacyController; import com.android.systemui.statusbar.notification.NotificationEntryManager; import com.android.systemui.statusbar.notification.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; -import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; import com.android.systemui.statusbar.notification.logging.NotificationLogger; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.notification.row.ExpandableView; @@ -106,17 +106,14 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase { mock(KeyguardBypassController.class), mock(BubbleController.class), mock(DynamicPrivacyController.class), - mock(ForegroundServiceSectionController.class)); + mock(ForegroundServiceSectionController.class), + mock(DynamicChildBindController.class)); mViewHierarchyManager.setUpWithPresenter(mPresenter, mListContainer); } private NotificationEntry createEntry() throws Exception { ExpandableNotificationRow row = mHelper.createRow(); - NotificationEntry entry = new NotificationEntryBuilder() - .setSbn(row.getEntry().getSbn()) - .build(); - entry.setRow(row); - return entry; + return row.getEntry(); } @Test diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/DynamicChildBindControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/DynamicChildBindControllerTest.java new file mode 100644 index 0000000000000..bf2d59880ffb7 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/DynamicChildBindControllerTest.java @@ -0,0 +1,133 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.notification; + +import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_CONTRACTED; +import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_EXPANDED; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.test.suitebuilder.annotation.SmallTest; +import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; +import android.util.ArrayMap; +import android.view.LayoutInflater; +import android.view.View; + +import com.android.systemui.R; +import com.android.systemui.SysuiTestCase; +import com.android.systemui.statusbar.notification.collection.NotificationEntry; +import com.android.systemui.statusbar.notification.collection.NotificationEntryBuilder; +import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.RowContentBindParams; +import com.android.systemui.statusbar.notification.row.RowContentBindStage; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +@SmallTest +@RunWith(AndroidTestingRunner.class) +@TestableLooper.RunWithLooper +public class DynamicChildBindControllerTest extends SysuiTestCase { + + private DynamicChildBindController mDynamicChildBindController; + private Map> mGroupNotifs = new ArrayMap<>(); + private static final int TEST_CHILD_BIND_CUTOFF = 5; + + @Mock private RowContentBindStage mBindStage; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + allowTestableLooperAsMainThread(); + when(mBindStage.getStageParams(any())).thenReturn(new RowContentBindParams()); + mDynamicChildBindController = + new DynamicChildBindController(mBindStage, TEST_CHILD_BIND_CUTOFF); + } + + @Test + public void testContentViewsOfChildrenBeyondCutoffAreFreed() { + // GIVEN a group notification with one view beyond the cutoff with content bound + NotificationEntry summary = addGroup(TEST_CHILD_BIND_CUTOFF + 1); + NotificationEntry lastChild = mGroupNotifs.get(summary).get(TEST_CHILD_BIND_CUTOFF); + + RowContentBindParams bindParams = mock(RowContentBindParams.class); + when(mBindStage.getStageParams(lastChild)).thenReturn(bindParams); + + // WHEN the controller gets the list + mDynamicChildBindController.updateChildContentViews(mGroupNotifs); + + // THEN we free content views + verify(bindParams).freeContentViews(FLAG_CONTENT_VIEW_CONTRACTED); + verify(bindParams).freeContentViews(FLAG_CONTENT_VIEW_EXPANDED); + verify(mBindStage).requestRebind(eq(lastChild), any()); + } + + @Test + public void testContentViewsBeforeCutoffAreBound() { + // GIVEN a group notification with one view before the cutoff with content unbound + NotificationEntry summary = addGroup(TEST_CHILD_BIND_CUTOFF); + NotificationEntry lastChild = mGroupNotifs.get(summary).get(TEST_CHILD_BIND_CUTOFF - 1); + + lastChild.getRow().getPrivateLayout().setContractedChild(null); + lastChild.getRow().getPrivateLayout().setExpandedChild(null); + + RowContentBindParams bindParams = mock(RowContentBindParams.class); + when(mBindStage.getStageParams(lastChild)).thenReturn(bindParams); + + // WHEN the controller gets the list + mDynamicChildBindController.updateChildContentViews(mGroupNotifs); + + // THEN we bind content views + verify(bindParams).requireContentViews(FLAG_CONTENT_VIEW_CONTRACTED); + verify(bindParams).requireContentViews(FLAG_CONTENT_VIEW_EXPANDED); + verify(mBindStage).requestRebind(eq(lastChild), any()); + } + + private NotificationEntry addGroup(int size) { + NotificationEntry summary = new NotificationEntryBuilder().build(); + summary.setRow(createRow()); + ArrayList children = new ArrayList<>(); + for (int i = 0; i < size; i++) { + NotificationEntry child = new NotificationEntryBuilder().build(); + child.setRow(createRow()); + children.add(child); + } + mGroupNotifs.put(summary, children); + return summary; + } + + private ExpandableNotificationRow createRow() { + ExpandableNotificationRow row = (ExpandableNotificationRow) + LayoutInflater.from(mContext).inflate(R.layout.status_bar_notification_row, null); + row.getPrivateLayout().setContractedChild(new View(mContext)); + row.getPrivateLayout().setExpandedChild(new View(mContext)); + return row; + } +} +