Remove isGroup bind param from row content binder

What the content binder implementation actually cares about is whether
it should inflate a low priority content view or not, so it doesn't
actually really need to know anything about the group state. The
actual decision making logic for that should happen outside that
class, namely in NotificationRowBinderImpl.

Due to grouping being determined after inflation, however, we also have
to recheck at a later point in the pipeline to see if we inflated the
wrong view and correct it. We put both these methods into a helper class
for now until the new pipeline is implemented.

Bug: 153602376
Fix: 153602376
Test: atest SystemUITests
Test: Add min priority group in Notify APK and see that views are more
than just their header view when unexpanded

Change-Id: Iab7a1cd31a7d4114dec829e24c65cb04dde78667
This commit is contained in:
Kevin Han
2020-04-13 15:46:14 -07:00
parent 42aa54c1a0
commit f69b71e9fd
13 changed files with 121 additions and 77 deletions

View File

@@ -35,6 +35,7 @@ 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.inflation.LowPriorityInflationHelper;
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
import com.android.systemui.statusbar.notification.stack.ForegroundServiceSectionController;
import com.android.systemui.statusbar.notification.stack.NotificationListContainer;
@@ -71,6 +72,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
protected final VisualStabilityManager mVisualStabilityManager;
private final SysuiStatusBarStateController mStatusBarStateController;
private final NotificationEntryManager mEntryManager;
private final LowPriorityInflationHelper mLowPriorityInflationHelper;
/**
* {@code true} if notifications not part of a group should by default be rendered in their
@@ -108,7 +110,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
BubbleController bubbleController,
DynamicPrivacyController privacyController,
ForegroundServiceSectionController fgsSectionController,
DynamicChildBindController dynamicChildBindController) {
DynamicChildBindController dynamicChildBindController,
LowPriorityInflationHelper lowPriorityInflationHelper) {
mContext = context;
mHandler = mainHandler;
mLockscreenUserManager = notificationLockscreenUserManager;
@@ -125,6 +128,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
mDynamicPrivacyController = privacyController;
privacyController.addListener(this);
mDynamicChildBindController = dynamicChildBindController;
mLowPriorityInflationHelper = lowPriorityInflationHelper;
}
public void setUpWithPresenter(NotificationPresenter presenter,
@@ -177,6 +181,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
currentUserId);
ent.setSensitive(sensitive, deviceSensitive);
ent.getRow().setNeedsRedaction(needsRedaction);
mLowPriorityInflationHelper.recheckLowPriorityViewAndInflate(ent, ent.getRow());
boolean isChildInGroup = mGroupManager.isChildInGroupWithSummary(ent.getSbn());
boolean groupChangesAllowed = mVisualStabilityManager.areGroupChangesAllowed()

View File

@@ -37,6 +37,7 @@ 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.inflation.LowPriorityInflationHelper;
import com.android.systemui.statusbar.notification.stack.ForegroundServiceSectionController;
import com.android.systemui.statusbar.phone.KeyguardBypassController;
import com.android.systemui.statusbar.phone.NotificationGroupManager;
@@ -143,7 +144,8 @@ public interface StatusBarDependenciesModule {
BubbleController bubbleController,
DynamicPrivacyController privacyController,
ForegroundServiceSectionController fgsSectionController,
DynamicChildBindController dynamicChildBindController) {
DynamicChildBindController dynamicChildBindController,
LowPriorityInflationHelper lowPriorityInflationHelper) {
return new NotificationViewHierarchyManager(
context,
mainHandler,
@@ -156,7 +158,8 @@ public interface StatusBarDependenciesModule {
bubbleController,
privacyController,
fgsSectionController,
dynamicChildBindController);
dynamicChildBindController,
lowPriorityInflationHelper);
}
/**

View File

@@ -619,7 +619,8 @@ public class NotificationEntryManager implements
entry.setSbn(notification);
for (NotifCollectionListener listener : mNotifCollectionListeners) {
listener.onEntryBind(entry, notification);
} mGroupManager.onEntryUpdated(entry, oldSbn);
}
mGroupManager.onEntryUpdated(entry, oldSbn);
mLogger.logNotifUpdated(entry.getKey());
for (NotificationEntryListener listener : mNotificationEntryListeners) {

View File

@@ -0,0 +1,85 @@
/*
* 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.collection.inflation;
import com.android.systemui.statusbar.FeatureFlags;
import com.android.systemui.statusbar.notification.collection.GroupEntry;
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 com.android.systemui.statusbar.phone.NotificationGroupManager;
import javax.inject.Inject;
import javax.inject.Singleton;
/**
* Helper class that provide methods to help check when we need to inflate a low priority version
* ot notification content.
*/
@Singleton
public class LowPriorityInflationHelper {
private final FeatureFlags mFeatureFlags;
private final NotificationGroupManager mGroupManager;
private final RowContentBindStage mRowContentBindStage;
@Inject
LowPriorityInflationHelper(
FeatureFlags featureFlags,
NotificationGroupManager groupManager,
RowContentBindStage rowContentBindStage) {
mFeatureFlags = featureFlags;
mGroupManager = groupManager;
mRowContentBindStage = rowContentBindStage;
}
/**
* Check if we inflated the wrong version of the view and if we need to reinflate the
* content views to be their low priority version or not.
*
* Whether we inflate the low priority view or not depends on the notification being visually
* part of a group. Since group membership is determined AFTER inflation, we're forced to check
* again at a later point in the pipeline to see if we inflated the wrong view and reinflate
* the correct one here.
*
* TODO: The group manager should run before inflation so that we don't deal with this
*/
public void recheckLowPriorityViewAndInflate(
NotificationEntry entry,
ExpandableNotificationRow row) {
RowContentBindParams params = mRowContentBindStage.getStageParams(entry);
final boolean shouldBeLowPriority = shouldUseLowPriorityView(entry);
if (!row.isRemoved() && row.isLowPriority() != shouldBeLowPriority) {
params.setUseLowPriority(shouldBeLowPriority);
mRowContentBindStage.requestRebind(entry,
en -> row.setIsLowPriority(shouldBeLowPriority));
}
}
/**
* Whether the notification should inflate a low priority version of its content views.
*/
public boolean shouldUseLowPriorityView(NotificationEntry entry) {
boolean isGroupChild;
if (mFeatureFlags.isNewNotifPipelineRenderingEnabled()) {
isGroupChild = (entry.getParent() != GroupEntry.ROOT_ENTRY);
} else {
isGroupChild = mGroupManager.isChildInGroupWithSummary(entry.getSbn());
}
return entry.isAmbient() && !isGroupChild;
}
}

View File

@@ -64,6 +64,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
private final ExpandableNotificationRowComponent.Builder
mExpandableNotificationRowComponentBuilder;
private final IconManager mIconManager;
private final LowPriorityInflationHelper mLowPriorityInflationHelper;
private NotificationPresenter mPresenter;
private NotificationListContainer mListContainer;
@@ -81,7 +82,8 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
NotificationInterruptStateProvider notificationInterruptionStateProvider,
Provider<RowInflaterTask> rowInflaterTaskProvider,
ExpandableNotificationRowComponent.Builder expandableNotificationRowComponentBuilder,
IconManager iconManager) {
IconManager iconManager,
LowPriorityInflationHelper lowPriorityInflationHelper) {
mContext = context;
mNotifBindPipeline = notifBindPipeline;
mRowContentBindStage = rowContentBindStage;
@@ -92,6 +94,7 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
mRowInflaterTaskProvider = rowInflaterTaskProvider;
mExpandableNotificationRowComponentBuilder = expandableNotificationRowComponentBuilder;
mIconManager = iconManager;
mLowPriorityInflationHelper = lowPriorityInflationHelper;
}
/**
@@ -224,11 +227,15 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
NotificationRowContentBinder.InflationCallback inflationCallback) {
final boolean useIncreasedCollapsedHeight =
mMessagingUtil.isImportantMessaging(entry.getSbn(), entry.getImportance());
final boolean isLowPriority = entry.isAmbient();
// If this is our first time inflating, we don't actually know the groupings for real
// yet, so we might actually inflate a low priority content view incorrectly here and have
// to correct it later in the pipeline. On subsequent inflations (i.e. updates), this
// should inflate the correct view.
final boolean isLowPriority = mLowPriorityInflationHelper.shouldUseLowPriorityView(entry);
RowContentBindParams params = mRowContentBindStage.getStageParams(entry);
params.setUseIncreasedCollapsedHeight(useIncreasedCollapsedHeight);
params.setUseLowPriority(entry.isAmbient());
params.setUseLowPriority(isLowPriority);
// TODO: Replace this API with RowContentBindParams directly. Also move to a separate
// redaction controller.

View File

@@ -239,7 +239,6 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
private ExpandableNotificationRow mNotificationParent;
private OnExpandClickListener mOnExpandClickListener;
private View.OnClickListener mOnAppOpsClickListener;
private boolean mIsChildInGroup;
// Listener will be called when receiving a long click event.
// Use #setLongPressPosition to optionally assign positional data with the long press.
@@ -830,15 +829,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
}
mNotificationParent = isChildInGroup ? parent : null;
mPrivateLayout.setIsChildInGroup(isChildInGroup);
// TODO: Move inflation logic out of this call
if (mIsChildInGroup != isChildInGroup) {
mIsChildInGroup = isChildInGroup;
if (!isRemoved() && mIsLowPriority) {
RowContentBindParams params = mRowContentBindStage.getStageParams(mEntry);
params.setUseLowPriority(mIsLowPriority);
mRowContentBindStage.requestRebind(mEntry, null /* callback */);
}
}
resetBackgroundAlpha();
updateBackgroundForGroupState();
updateClickAndFocus();

View File

@@ -132,7 +132,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder
mConversationProcessor,
row,
bindParams.isLowPriority,
bindParams.isChildInGroup,
bindParams.usesIncreasedHeight,
bindParams.usesIncreasedHeadsUpHeight,
callback,
@@ -156,7 +155,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder
InflationProgress result = createRemoteViews(reInflateFlags,
builder,
bindParams.isLowPriority,
bindParams.isChildInGroup,
bindParams.usesIncreasedHeight,
bindParams.usesIncreasedHeadsUpHeight,
packageContext);
@@ -285,11 +283,9 @@ public class NotificationContentInflater implements NotificationRowContentBinder
}
private static InflationProgress createRemoteViews(@InflationFlag int reInflateFlags,
Notification.Builder builder, boolean isLowPriority, boolean isChildInGroup,
boolean usesIncreasedHeight, boolean usesIncreasedHeadsUpHeight,
Context packageContext) {
Notification.Builder builder, boolean isLowPriority, boolean usesIncreasedHeight,
boolean usesIncreasedHeadsUpHeight, Context packageContext) {
InflationProgress result = new InflationProgress();
isLowPriority = isLowPriority && !isChildInGroup;
if ((reInflateFlags & FLAG_CONTENT_VIEW_CONTRACTED) != 0) {
result.newContentView = createContentView(builder, isLowPriority, usesIncreasedHeight);
@@ -702,7 +698,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder
private final Context mContext;
private final boolean mInflateSynchronously;
private final boolean mIsLowPriority;
private final boolean mIsChildInGroup;
private final boolean mUsesIncreasedHeight;
private final InflationCallback mCallback;
private final boolean mUsesIncreasedHeadsUpHeight;
@@ -728,7 +723,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder
ConversationNotificationProcessor conversationProcessor,
ExpandableNotificationRow row,
boolean isLowPriority,
boolean isChildInGroup,
boolean usesIncreasedHeight,
boolean usesIncreasedHeadsUpHeight,
InflationCallback callback,
@@ -743,7 +737,6 @@ public class NotificationContentInflater implements NotificationRowContentBinder
mRemoteViewCache = cache;
mContext = mRow.getContext();
mIsLowPriority = isLowPriority;
mIsChildInGroup = isChildInGroup;
mUsesIncreasedHeight = usesIncreasedHeight;
mUsesIncreasedHeadsUpHeight = usesIncreasedHeadsUpHeight;
mRemoteViewClickHandler = remoteViewClickHandler;
@@ -781,7 +774,7 @@ public class NotificationContentInflater implements NotificationRowContentBinder
mConversationProcessor.processNotification(mEntry, recoveredBuilder);
}
InflationProgress inflationProgress = createRemoteViews(mReInflateFlags,
recoveredBuilder, mIsLowPriority, mIsChildInGroup, mUsesIncreasedHeight,
recoveredBuilder, mIsLowPriority, mUsesIncreasedHeight,
mUsesIncreasedHeadsUpHeight, packageContext);
return inflateSmartReplyViews(inflationProgress, mReInflateFlags, mEntry,
mRow.getContext(), packageContext, mRow.getHeadsUpManager(),

View File

@@ -113,11 +113,6 @@ public interface NotificationRowContentBinder {
*/
public boolean isLowPriority;
/**
* Bind child version of content views.
*/
public boolean isChildInGroup;
/**
* Use increased height when binding contracted view.
*/

View File

@@ -27,7 +27,6 @@ import com.android.systemui.statusbar.notification.row.NotificationRowContentBin
*/
public final class RowContentBindParams {
private boolean mUseLowPriority;
private boolean mUseChildInGroup;
private boolean mUseIncreasedHeight;
private boolean mUseIncreasedHeadsUpHeight;
private boolean mViewsNeedReinflation;
@@ -55,20 +54,6 @@ public final class RowContentBindParams {
return mUseLowPriority;
}
/**
* Set whether content should use group child version of its content views.
*/
public void setUseChildInGroup(boolean useChildInGroup) {
if (mUseChildInGroup != useChildInGroup) {
mDirtyContentViews |= (FLAG_CONTENT_VIEW_CONTRACTED | FLAG_CONTENT_VIEW_EXPANDED);
}
mUseChildInGroup = useChildInGroup;
}
public boolean useChildInGroup() {
return mUseChildInGroup;
}
/**
* Set whether content should use an increased height version of its contracted view.
*/
@@ -163,10 +148,10 @@ public final class RowContentBindParams {
@Override
public String toString() {
return String.format("RowContentBindParams[mContentViews=%x mDirtyContentViews=%x "
+ "mUseLowPriority=%b mUseChildInGroup=%b mUseIncreasedHeight=%b "
+ "mUseLowPriority=%b mUseIncreasedHeight=%b "
+ "mUseIncreasedHeadsUpHeight=%b mViewsNeedReinflation=%b]",
mContentViews, mDirtyContentViews, mUseLowPriority, mUseChildInGroup,
mUseIncreasedHeight, mUseIncreasedHeadsUpHeight, mViewsNeedReinflation);
mContentViews, mDirtyContentViews, mUseLowPriority, mUseIncreasedHeight,
mUseIncreasedHeadsUpHeight, mViewsNeedReinflation);
}
/**

View File

@@ -71,7 +71,6 @@ public class RowContentBindStage extends BindStage<RowContentBindParams> {
BindParams bindParams = new BindParams();
bindParams.isLowPriority = params.useLowPriority();
bindParams.isChildInGroup = params.useChildInGroup();
bindParams.usesIncreasedHeight = params.useIncreasedHeight();
bindParams.usesIncreasedHeadsUpHeight = params.useIncreasedHeadsUpHeight();
boolean forceInflate = params.needsReinflation();

View File

@@ -46,6 +46,7 @@ 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.inflation.LowPriorityInflationHelper;
import com.android.systemui.statusbar.notification.logging.NotificationLogger;
import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
import com.android.systemui.statusbar.notification.row.ExpandableView;
@@ -110,7 +111,8 @@ public class NotificationViewHierarchyManagerTest extends SysuiTestCase {
mock(BubbleController.class),
mock(DynamicPrivacyController.class),
mock(ForegroundServiceSectionController.class),
mock(DynamicChildBindController.class));
mock(DynamicChildBindController.class),
mock(LowPriorityInflationHelper.class));
mViewHierarchyManager.setUpWithPresenter(mPresenter, mListContainer);
}

View File

@@ -63,6 +63,7 @@ import com.android.systemui.statusbar.notification.NotificationFilter;
import com.android.systemui.statusbar.notification.NotificationSectionsFeatureManager;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.NotificationRankingManager;
import com.android.systemui.statusbar.notification.collection.inflation.LowPriorityInflationHelper;
import com.android.systemui.statusbar.notification.collection.inflation.NotificationRowBinderImpl;
import com.android.systemui.statusbar.notification.collection.provider.HighPriorityProvider;
import com.android.systemui.statusbar.notification.icon.IconBuilder;
@@ -264,7 +265,8 @@ public class NotificationEntryManagerInflationTest extends SysuiTestCase {
new IconManager(
mEntryManager,
mock(LauncherApps.class),
new IconBuilder(mContext)));
new IconBuilder(mContext)),
mock(LowPriorityInflationHelper.class));
mEntryManager.setUpWithPresenter(mPresenter);
mEntryManager.addNotificationEntryListener(mEntryListener);

View File

@@ -146,30 +146,6 @@ public class RowContentBindStageTest extends SysuiTestCase {
assertTrue(usedParams.isLowPriority);
}
@Test
public void testSetUseGroupInChild() {
// GIVEN a view with all content bound.
RowContentBindParams params = mRowContentBindStage.getStageParams(mEntry);
params.requireContentViews(FLAG_CONTENT_VIEW_ALL);
params.clearDirtyContentViews();
// WHEN use group is set and stage executed.
params.setUseChildInGroup(true);
mRowContentBindStage.executeStage(mEntry, mRow, (en) -> { });
// THEN binder is called with use group view and contracted/expanded are called to bind.
ArgumentCaptor<BindParams> bindParamsCaptor = ArgumentCaptor.forClass(BindParams.class);
verify(mBinder).bindContent(
eq(mEntry),
any(),
eq(FLAG_CONTENT_VIEW_CONTRACTED | FLAG_CONTENT_VIEW_EXPANDED),
bindParamsCaptor.capture(),
anyBoolean(),
any());
BindParams usedParams = bindParamsCaptor.getValue();
assertTrue(usedParams.isChildInGroup);
}
@Test
public void testSetUseIncreasedHeight() {
// GIVEN a view with all content bound.