Merge "Remove isGroup bind param from row content binder" into rvc-dev am: 3f8720b17f

Change-Id: Idbab5e2543130d44b3de42d453e273508f46b743

packages/SystemUI/src/com/android/systemui/statusbar/NotificationViewHierarchyManager.java

@@ -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;
@@ -124,6 +127,7 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
         mBubbleController = bubbleController;
         mDynamicPrivacyController = privacyController;
         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()

packages/SystemUI/src/com/android/systemui/statusbar/dagger/StatusBarDependenciesModule.java

@@ -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);
     }
 
     /**

packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationEntryManager.java

@@ -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) {

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/LowPriorityInflationHelper.java

@@ -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;
+    }
+}

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/inflation/NotificationRowBinderImpl.java

@@ -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;
     }
 
     /**
@@ -225,11 +228,15 @@ public class NotificationRowBinderImpl implements NotificationRowBinder {
             @Nullable 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.

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java

@@ -240,7 +240,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.
@@ -848,15 +847,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();

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationContentInflater.java

@@ -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,
+            Notification.Builder builder, boolean isLowPriority, boolean usesIncreasedHeight,
-            boolean usesIncreasedHeight, boolean usesIncreasedHeadsUpHeight,
+            boolean usesIncreasedHeadsUpHeight, Context packageContext) {
-            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(),

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationRowContentBinder.java

@@ -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.
          */

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindParams.java

@@ -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,
+                mContentViews, mDirtyContentViews, mUseLowPriority, mUseIncreasedHeight,
-                mUseIncreasedHeight, mUseIncreasedHeadsUpHeight, mViewsNeedReinflation);
+                mUseIncreasedHeadsUpHeight, mViewsNeedReinflation);
     }
 
     /**

packages/SystemUI/src/com/android/systemui/statusbar/notification/row/RowContentBindStage.java

@@ -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();

packages/SystemUI/tests/src/com/android/systemui/statusbar/NotificationViewHierarchyManagerTest.java

@@ -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);
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationEntryManagerInflationTest.java

@@ -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);

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java

@@ -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.