From b85d1f4f39ade263a29a01a02aff5714f71d51d3 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Wed, 12 May 2021 13:14:21 -0400 Subject: [PATCH] Allow populating left_icon independently of right_icon. * Mainly this fixes a "bug" with the BigPictureStyle.showBigPictureWhenCollapsed() that would cause the big picture, which is known to be important for the notification, to be hidden on the right and shown on the left. Instead the BigPicture will now remain visible in its large form on the right, and the left icon will be populated with the large icon. * Additionally, this fixes some quirks with other layouts, such as MediaStyle and MessagingStyle that the largeIcon would be missing (sometimes only in one expansion state) when it could be shown in the left column. * Finally, this fixes a few bugs in the expansion animation that were caused by logic for determining the showing icon being inconsistent between Notification.java and the notification view wrappers. Bug: 186421651 Test: post notifications of all these types in and outside of groups. Change-Id: Ifd1d817afcb1b6fa8cfb79baee96364af8c7af3c --- core/java/android/app/Notification.java | 130 ++++++++++-------- core/res/res/values/ids.xml | 6 + core/res/res/values/symbols.xml | 2 + .../statusbar/NotificationGroupingUtil.java | 32 +++-- ...ficationBigPictureTemplateViewWrapper.java | 17 ++- .../NotificationTemplateViewWrapper.java | 33 ++++- 6 files changed, 139 insertions(+), 81 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 2dcdd07c99462..54582a3ecff28 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -5309,8 +5309,7 @@ public class Notification implements Parcelable // the change's state in NotificationManagerService were very complex. These behavior // changes are entirely visual, and should otherwise be undetectable by apps. @SuppressWarnings("AndroidFrameworkCompatChange") - private void calculateLargeIconDimens(boolean largeIconShown, - @NonNull StandardTemplateParams p, + private void calculateRightIconDimens(Icon rightIcon, boolean isPromotedPicture, @NonNull TemplateBindResult result) { final Resources resources = mContext.getResources(); final float density = resources.getDisplayMetrics().density; @@ -5323,9 +5322,9 @@ public class Notification implements Parcelable final float viewHeightDp = resources.getDimension( R.dimen.notification_right_icon_size) / density; float viewWidthDp = viewHeightDp; // icons are 1:1 by default - if (largeIconShown && (p.mPromotePicture + if (rightIcon != null && (isPromotedPicture || mContext.getApplicationInfo().targetSdkVersion >= Build.VERSION_CODES.S)) { - Drawable drawable = mN.mLargeIcon.loadDrawable(mContext); + Drawable drawable = rightIcon.loadDrawable(mContext); if (drawable != null) { int iconWidth = drawable.getIntrinsicWidth(); int iconHeight = drawable.getIntrinsicHeight(); @@ -5337,7 +5336,7 @@ public class Notification implements Parcelable } } final float extraMarginEndDpIfVisible = viewWidthDp + iconMarginDp; - result.setRightIconState(largeIconShown, viewWidthDp, + result.setRightIconState(rightIcon != null /* visible */, viewWidthDp, extraMarginEndDpIfVisible, expanderSizeDp); } @@ -5349,19 +5348,43 @@ public class Notification implements Parcelable if (mN.mLargeIcon == null && mN.largeIcon != null) { mN.mLargeIcon = Icon.createWithBitmap(mN.largeIcon); } - boolean showLargeIcon = mN.mLargeIcon != null && !p.hideLargeIcon; - calculateLargeIconDimens(showLargeIcon, p, result); - if (showLargeIcon) { + + // Determine the left and right icons + Icon leftIcon = p.mHideLeftIcon ? null : mN.mLargeIcon; + Icon rightIcon = p.mHideRightIcon ? null + : (p.mPromotedPicture != null ? p.mPromotedPicture : mN.mLargeIcon); + + // Apply the left icon (without duplicating the bitmap) + if (leftIcon != rightIcon || leftIcon == null) { + // If the leftIcon is explicitly hidden or different from the rightIcon, then set it + // explicitly and make sure it won't take the right_icon drawable. + contentView.setImageViewIcon(R.id.left_icon, leftIcon); + contentView.setIntTag(R.id.left_icon, R.id.tag_uses_right_icon_drawable, 0); + } else { + // If the leftIcon equals the rightIcon, just set the flag to use the right_icon + // drawable. This avoids the view having two copies of the same bitmap. + contentView.setIntTag(R.id.left_icon, R.id.tag_uses_right_icon_drawable, 1); + } + + // Always calculate dimens to populate `result` for the GONE case + boolean isPromotedPicture = p.mPromotedPicture != null; + calculateRightIconDimens(rightIcon, isPromotedPicture, result); + + // Bind the right icon + if (rightIcon != null) { contentView.setViewLayoutWidth(R.id.right_icon, result.mRightIconWidthDp, TypedValue.COMPLEX_UNIT_DIP); contentView.setViewVisibility(R.id.right_icon, View.VISIBLE); - contentView.setImageViewIcon(R.id.right_icon, mN.mLargeIcon); - processLargeLegacyIcon(mN.mLargeIcon, contentView, p); + contentView.setImageViewIcon(R.id.right_icon, rightIcon); + contentView.setIntTag(R.id.right_icon, R.id.tag_keep_when_showing_left_icon, + isPromotedPicture ? 1 : 0); + processLargeLegacyIcon(rightIcon, contentView, p); } else { // The "reset" doesn't clear the drawable, so we do it here. This clear is // important because the presence of a drawable in this view (regardless of the // visibility) is used by NotificationGroupingUtil to set the visibility. contentView.setImageViewIcon(R.id.right_icon, null); + contentView.setIntTag(R.id.right_icon, R.id.tag_keep_when_showing_left_icon, 0); } } @@ -7392,25 +7415,11 @@ public class Notification implements Parcelable return super.makeContentView(increasedHeight); } - Icon oldLargeIcon = mBuilder.mN.mLargeIcon; - mBuilder.mN.mLargeIcon = mPictureIcon; - // The legacy largeIcon might not allow us to clear the image, as it's taken in - // replacement if the other one is null. Because we're restoring these legacy icons - // for old listeners, this is in general non-null. - Bitmap largeIconLegacy = mBuilder.mN.largeIcon; - mBuilder.mN.largeIcon = null; - StandardTemplateParams p = mBuilder.mParams.reset() .viewType(StandardTemplateParams.VIEW_TYPE_NORMAL) .fillTextsFrom(mBuilder) - .promotePicture(true); - RemoteViews contentView = getStandardView(mBuilder.getBaseLayoutResource(), - p, null /* result */); - - mBuilder.mN.mLargeIcon = oldLargeIcon; - mBuilder.mN.largeIcon = largeIconLegacy; - - return contentView; + .promotedPicture(mPictureIcon); + return getStandardView(mBuilder.getBaseLayoutResource(), p, null /* result */); } /** @@ -7422,25 +7431,11 @@ public class Notification implements Parcelable return super.makeHeadsUpContentView(increasedHeight); } - Icon oldLargeIcon = mBuilder.mN.mLargeIcon; - mBuilder.mN.mLargeIcon = mPictureIcon; - // The legacy largeIcon might not allow us to clear the image, as it's taken in - // replacement if the other one is null. Because we're restoring these legacy icons - // for old listeners, this is in general non-null. - Bitmap largeIconLegacy = mBuilder.mN.largeIcon; - mBuilder.mN.largeIcon = null; - StandardTemplateParams p = mBuilder.mParams.reset() .viewType(StandardTemplateParams.VIEW_TYPE_HEADS_UP) .fillTextsFrom(mBuilder) - .promotePicture(true); - RemoteViews contentView = getStandardView(mBuilder.getHeadsUpBaseLayoutResource(), - p, null /* result */); - - mBuilder.mN.mLargeIcon = oldLargeIcon; - mBuilder.mN.largeIcon = largeIconLegacy; - - return contentView; + .promotedPicture(mPictureIcon); + return getStandardView(mBuilder.getHeadsUpBaseLayoutResource(), p, null /* result */); } /** @@ -7535,14 +7530,21 @@ public class Notification implements Parcelable mShowBigPictureWhenCollapsed = extras.getBoolean(EXTRA_SHOW_BIG_PICTURE_WHEN_COLLAPSED); + mPictureIcon = getPictureIcon(extras); + } + + /** @hide */ + @Nullable + public static Icon getPictureIcon(@Nullable Bundle extras) { + if (extras == null) return null; // When this style adds a picture, we only add one of the keys. If both were added, // it would most likely be a legacy app trying to override the picture in some way. // Because of that case it's better to give precedence to the legacy field. Bitmap bitmapPicture = extras.getParcelable(EXTRA_PICTURE); if (bitmapPicture != null) { - mPictureIcon = Icon.createWithBitmap(bitmapPicture); + return Icon.createWithBitmap(bitmapPicture); } else { - mPictureIcon = extras.getParcelable(EXTRA_PICTURE_ICON); + return extras.getParcelable(EXTRA_PICTURE_ICON); } } @@ -8318,7 +8320,8 @@ public class Notification implements Parcelable .hideProgress(true) .title(isHeaderless ? conversationTitle : null) .text(null) - .hideLargeIcon(hideRightIcons || isOneToOne) + .hideLeftIcon(isOneToOne) + .hideRightIcon(hideRightIcons || isOneToOne) .headerTextSecondary(isHeaderless ? null : conversationTitle); RemoteViews contentView = mBuilder.applyStandardTemplateWithActions( isConversationLayout @@ -9116,9 +9119,10 @@ public class Notification implements Parcelable StandardTemplateParams p = mBuilder.mParams.reset() .viewType(StandardTemplateParams.VIEW_TYPE_NORMAL) - .hideTime(numActionsToShow > 1) // hide if actions wider than a large icon - .hideSubText(numActionsToShow > 1) // hide if actions wider than a large icon - .hideLargeIcon(numActionsToShow > 0) // large icon or actions; not both + .hideTime(numActionsToShow > 1) // hide if actions wider than a right icon + .hideSubText(numActionsToShow > 1) // hide if actions wider than a right icon + .hideLeftIcon(false) // allow large icon on left when grouped + .hideRightIcon(numActionsToShow > 0) // right icon or actions; not both .hideProgress(true) .fillTextsFrom(mBuilder); TemplateBindResult result = new TemplateBindResult(); @@ -9522,7 +9526,8 @@ public class Notification implements Parcelable .viewType(viewType) .callStyleActions(true) .allowTextWithProgress(true) - .hideLargeIcon(true) + .hideLeftIcon(true) + .hideRightIcon(true) .hideAppName(isCollapsed) .titleViewId(R.id.conversation_text) .title(title) @@ -12232,7 +12237,9 @@ public class Notification implements Parcelable boolean mHideActions; boolean mHideProgress; boolean mHideSnoozeButton; - boolean mPromotePicture; + boolean mHideLeftIcon; + boolean mHideRightIcon; + Icon mPromotedPicture; boolean mCallStyleActions; boolean mAllowTextWithProgress; int mTitleViewId; @@ -12242,7 +12249,6 @@ public class Notification implements Parcelable CharSequence headerTextSecondary; CharSequence summaryText; int maxRemoteInputHistory = Style.MAX_REMOTE_INPUT_HISTORY_LINES; - boolean hideLargeIcon; boolean allowColorization = true; boolean mHighlightExpander = false; @@ -12256,7 +12262,9 @@ public class Notification implements Parcelable mHideActions = false; mHideProgress = false; mHideSnoozeButton = false; - mPromotePicture = false; + mHideLeftIcon = false; + mHideRightIcon = false; + mPromotedPicture = null; mCallStyleActions = false; mAllowTextWithProgress = false; mTitleViewId = R.id.title; @@ -12266,7 +12274,6 @@ public class Notification implements Parcelable summaryText = null; headerTextSecondary = null; maxRemoteInputHistory = Style.MAX_REMOTE_INPUT_HISTORY_LINES; - hideLargeIcon = false; allowColorization = true; mHighlightExpander = false; return this; @@ -12331,8 +12338,8 @@ public class Notification implements Parcelable return this; } - final StandardTemplateParams promotePicture(boolean promotePicture) { - this.mPromotePicture = promotePicture; + final StandardTemplateParams promotedPicture(Icon promotedPicture) { + this.mPromotedPicture = promotedPicture; return this; } @@ -12366,8 +12373,14 @@ public class Notification implements Parcelable return this; } - final StandardTemplateParams hideLargeIcon(boolean hideLargeIcon) { - this.hideLargeIcon = hideLargeIcon; + + final StandardTemplateParams hideLeftIcon(boolean hideLeftIcon) { + this.mHideLeftIcon = hideLeftIcon; + return this; + } + + final StandardTemplateParams hideRightIcon(boolean hideRightIcon) { + this.mHideRightIcon = hideRightIcon; return this; } @@ -12404,7 +12417,8 @@ public class Notification implements Parcelable // Minimally decorated custom views do not show certain pieces of chrome that have // always been shown when using DecoratedCustomViewStyle. boolean hideOtherFields = decorationType <= DECORATION_MINIMAL; - hideLargeIcon(hideOtherFields); + hideLeftIcon(false); // The left icon decoration is better than showing nothing. + hideRightIcon(hideOtherFields); hideProgress(hideOtherFields); hideActions(hideOtherFields); return this; diff --git a/core/res/res/values/ids.xml b/core/res/res/values/ids.xml index c3b35c81cb66a..c4838b83347c5 100644 --- a/core/res/res/values/ids.xml +++ b/core/res/res/values/ids.xml @@ -206,6 +206,12 @@ + + + + + + diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index d8620a706fe2a..d440173ef1996 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -3167,6 +3167,8 @@ + + diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java index 1c5df4119a24b..5b694973d7f46 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java @@ -430,7 +430,6 @@ public class NotificationGroupingUtil { private static class LeftIconApplicator implements ResultApplicator { public static final int[] MARGIN_ADJUSTED_VIEWS = { - R.id.notification_headerless_view_column, R.id.text, R.id.big_text, R.id.title, @@ -438,22 +437,31 @@ public class NotificationGroupingUtil { R.id.notification_header}; @Override - public void apply(View parent, View child, boolean apply, boolean reset) { - ImageView rightIcon = child.findViewById(com.android.internal.R.id.right_icon); + public void apply(View parent, View child, boolean showLeftIcon, boolean reset) { ImageView leftIcon = child.findViewById(com.android.internal.R.id.left_icon); - if (rightIcon == null || leftIcon == null) { + if (leftIcon == null) { return; } - Drawable iconDrawable = rightIcon.getDrawable(); - if (iconDrawable == null) { - return; + ImageView rightIcon = child.findViewById(com.android.internal.R.id.right_icon); + boolean keepRightIcon = rightIcon != null && Integer.valueOf(1).equals( + rightIcon.getTag(R.id.tag_keep_when_showing_left_icon)); + boolean leftIconUsesRightIconDrawable = Integer.valueOf(1).equals( + leftIcon.getTag(R.id.tag_uses_right_icon_drawable)); + if (leftIconUsesRightIconDrawable) { + // Use the right drawable when showing the left, unless the right is being kept + Drawable rightDrawable = rightIcon == null ? null : rightIcon.getDrawable(); + leftIcon.setImageDrawable(showLeftIcon && !keepRightIcon ? rightDrawable : null); } - rightIcon.setVisibility(apply ? View.GONE : View.VISIBLE); - leftIcon.setVisibility(apply ? View.VISIBLE : View.GONE); - leftIcon.setImageDrawable(apply ? iconDrawable : null); + leftIcon.setVisibility(showLeftIcon ? View.VISIBLE : View.GONE); - for (int viewId : MARGIN_ADJUSTED_VIEWS) { - adjustMargins(!apply, child.findViewById(viewId)); + // update the right icon as well + if (rightIcon != null) { + boolean showRightIcon = (keepRightIcon || !showLeftIcon) + && rightIcon.getDrawable() != null; + rightIcon.setVisibility(showRightIcon ? View.VISIBLE : View.GONE); + for (int viewId : MARGIN_ADJUSTED_VIEWS) { + adjustMargins(showRightIcon, child.findViewById(viewId)); + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationBigPictureTemplateViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationBigPictureTemplateViewWrapper.java index 151840afcef12..79f99b85c4101 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationBigPictureTemplateViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationBigPictureTemplateViewWrapper.java @@ -42,12 +42,17 @@ public class NotificationBigPictureTemplateViewWrapper extends NotificationTempl updateImageTag(row.getEntry().getSbn()); } - private void updateImageTag(StatusBarNotification notification) { - final Bundle extras = notification.getNotification().extras; - Icon overriddenIcon = extras.getParcelable(Notification.EXTRA_LARGE_ICON_BIG); - if (overriddenIcon != null) { - mRightIcon.setTag(ImageTransformState.ICON_TAG, overriddenIcon); - mLeftIcon.setTag(ImageTransformState.ICON_TAG, overriddenIcon); + private void updateImageTag(StatusBarNotification sbn) { + final Bundle extras = sbn.getNotification().extras; + boolean bigLargeIconSet = extras.containsKey(Notification.EXTRA_LARGE_ICON_BIG); + if (bigLargeIconSet) { + Icon bigLargeIcon = extras.getParcelable(Notification.EXTRA_LARGE_ICON_BIG); + mRightIcon.setTag(ImageTransformState.ICON_TAG, bigLargeIcon); + mLeftIcon.setTag(ImageTransformState.ICON_TAG, bigLargeIcon); + } else { + // Overwrite in case the superclass populated this tag with the promoted picture, + // which won't be right since this is the expanded state. + mRightIcon.setTag(ImageTransformState.ICON_TAG, getLargeIcon(sbn.getNotification())); } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationTemplateViewWrapper.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationTemplateViewWrapper.java index 0548611ca2d22..a4f11723b978b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationTemplateViewWrapper.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/wrapper/NotificationTemplateViewWrapper.java @@ -20,10 +20,12 @@ import static android.view.View.VISIBLE; import static com.android.systemui.statusbar.notification.row.ExpandableNotificationRow.DEFAULT_HEADER_VISIBLE_AMOUNT; +import android.app.Notification; import android.app.PendingIntent; import android.content.Context; import android.content.res.ColorStateList; import android.graphics.Color; +import android.graphics.drawable.Icon; import android.service.notification.StatusBarNotification; import android.util.ArraySet; import android.view.View; @@ -32,6 +34,8 @@ import android.widget.ImageView; import android.widget.ProgressBar; import android.widget.TextView; +import androidx.annotation.Nullable; + import com.android.internal.util.ContrastColorUtil; import com.android.internal.widget.NotificationActionListLayout; import com.android.systemui.Dependency; @@ -143,16 +147,14 @@ public class NotificationTemplateViewWrapper extends NotificationHeaderViewWrapp com.android.internal.R.dimen.notification_content_margin_top); } - private void resolveTemplateViews(StatusBarNotification notification) { + private void resolveTemplateViews(StatusBarNotification sbn) { mRightIcon = mView.findViewById(com.android.internal.R.id.right_icon); if (mRightIcon != null) { - mRightIcon.setTag(ImageTransformState.ICON_TAG, - notification.getNotification().getLargeIcon()); + mRightIcon.setTag(ImageTransformState.ICON_TAG, getRightIcon(sbn.getNotification())); } mLeftIcon = mView.findViewById(com.android.internal.R.id.left_icon); if (mLeftIcon != null) { - mLeftIcon.setTag(ImageTransformState.ICON_TAG, - notification.getNotification().getLargeIcon()); + mLeftIcon.setTag(ImageTransformState.ICON_TAG, getLargeIcon(sbn.getNotification())); } mTitle = mView.findViewById(com.android.internal.R.id.title); mText = mView.findViewById(com.android.internal.R.id.text); @@ -171,6 +173,27 @@ public class NotificationTemplateViewWrapper extends NotificationHeaderViewWrapp updatePendingIntentCancellations(); } + @Nullable + protected final Icon getLargeIcon(Notification n) { + Icon modernLargeIcon = n.getLargeIcon(); + if (modernLargeIcon == null && n.largeIcon != null) { + return Icon.createWithBitmap(n.largeIcon); + } + return modernLargeIcon; + } + + @Nullable + protected final Icon getRightIcon(Notification n) { + if (n.extras.getBoolean(Notification.EXTRA_SHOW_BIG_PICTURE_WHEN_COLLAPSED) + && n.getNotificationStyle() == Notification.BigPictureStyle.class) { + Icon pictureIcon = Notification.BigPictureStyle.getPictureIcon(n.extras); + if (pictureIcon != null) { + return pictureIcon; + } + } + return getLargeIcon(n); + } + private void updatePendingIntentCancellations() { if (mActions != null) { int numActions = mActions.getChildCount();