From 99104836083738f9e5e73ab4170879d91de6ba67 Mon Sep 17 00:00:00 2001 From: Selim Cinek Date: Wed, 25 Jan 2017 14:47:33 -0800 Subject: [PATCH] Further improved the coloring optimization Fixes a few follow up bugs from the colorized notification CL. This also allows media notifications with a media session to be colorized by default. Test: existing tests pass Bug: 34469375 Change-Id: I36348591a84fcd39d103d0ae3f64720f2fca2cf2 --- core/java/android/app/Notification.java | 87 +++++++++++++++---- .../android/view/NotificationHeaderView.java | 3 +- .../ActivatableNotificationView.java | 2 +- .../statusbar/ExpandableNotificationRow.java | 4 +- .../notification/NotificationComparator.java | 3 +- 5 files changed, 75 insertions(+), 24 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 1c9acae239ed8..d1226f225ed07 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -600,9 +600,10 @@ public class Notification implements Parcelable /** * Special value of {@link #color} used as a place holder for an invalid color. + * @hide */ @ColorInt - private static final int COLOR_INVALID = 1; + public static final int COLOR_INVALID = 1; /** * Sphere of visibility of this notification, which affects how and when the SystemUI reveals @@ -2446,7 +2447,6 @@ public class Notification implements Parcelable private NotificationColorUtil mColorUtil; private boolean mIsLegacy; private boolean mIsLegacyInitialized; - private boolean mColorUtilInited = false; /** * Caches a contrast-enhanced version of {@link #mCachedContrastColorIsFor}. @@ -2564,11 +2564,8 @@ public class Notification implements Parcelable } private NotificationColorUtil getColorUtil() { - if (!mColorUtilInited) { - mColorUtilInited = true; - if (isLegacy() || isColorized()) { - mColorUtil = NotificationColorUtil.getInstance(mContext); - } + if (mColorUtil == null) { + mColorUtil = NotificationColorUtil.getInstance(mContext); } return mColorUtil; } @@ -3054,12 +3051,16 @@ public class Notification implements Parcelable * Set whether this notification should be colorized. When set, the color set with * {@link #setColor(int)} will be used as the background color of this notification. *

- * The coloring will only be applied if the notification is ongoing. * This should only be used for high priority ongoing tasks like navigation, an ongoing * call, or other similarly high-priority events for the user. + *

+ * For most styles, the coloring will only be applied if the notification is ongoing. + * However, for {@link MediaStyle} and {@link DecoratedMediaCustomViewStyle} notifications + * that have a media session attached there is no requirement for it to be ongoing. * * @see Builder#setOngoing(boolean) * @see Builder#setColor(int) + * @see MediaStyle#setMediaSession(MediaSession.Token) */ public Builder setColorized(boolean colorize) { mN.extras.putBoolean(EXTRA_COLORIZED, colorize); @@ -3633,13 +3634,21 @@ public class Notification implements Parcelable } private void bindExpandButton(RemoteViews contentView) { - int color = isColorized() ? getPrimaryTextColor() : resolveContrastColor(); + int color = getPrimaryHighlightColor(); contentView.setDrawableParameters(R.id.expand_button, false, -1, color, PorterDuff.Mode.SRC_ATOP, -1); contentView.setInt(R.id.notification_header, "setOriginalNotificationColor", color); } + /** + * @return the color that is used as the first primary highlight color. This is applied + * in several places like the action buttons or the app name in the header. + */ + private int getPrimaryHighlightColor() { + return isColorized() ? getPrimaryTextColor() : resolveContrastColor(); + } + private void bindHeaderChronometerAndTime(RemoteViews contentView) { if (showsTimeOrChronometer()) { contentView.setViewVisibility(R.id.time_divider, View.VISIBLE); @@ -4216,7 +4225,7 @@ public class Notification implements Parcelable } private CharSequence processLegacyText(CharSequence charSequence) { - if (isLegacy() || isColorized()) { + if (isLegacy() || textColorsNeedInversion()) { return getColorUtil().invertCharSequenceColors(charSequence); } else { return charSequence; @@ -4229,7 +4238,7 @@ public class Notification implements Parcelable private void processSmallIconColor(Icon smallIcon, RemoteViews contentView, boolean ambient) { boolean colorable = !isLegacy() || getColorUtil().isGrayscaleIcon(mContext, smallIcon); - int color = ambient ? resolveAmbientColor() : resolveContrastColor(); + int color = ambient ? resolveAmbientColor() : getPrimaryHighlightColor(); if (colorable) { contentView.setDrawableParameters(R.id.icon, false, -1, color, PorterDuff.Mode.SRC_ATOP, -1); @@ -4490,6 +4499,15 @@ public class Notification implements Parcelable private boolean isColorized() { return mN.isColorized(); } + + private boolean textColorsNeedInversion() { + if (mStyle == null || !MediaStyle.class.equals(mStyle.getClass())) { + return false; + } + int targetSdkVersion = mContext.getApplicationInfo().targetSdkVersion; + return targetSdkVersion > Build.VERSION_CODES.M + && targetSdkVersion < Build.VERSION_CODES.O; + } } /** @@ -4501,6 +4519,14 @@ public class Notification implements Parcelable return (flags & ongoingFlags) != 0; } + /** + * @return whether this notification has a media session attached + * @hide + */ + public boolean hasMediaSession() { + return extras.getParcelable(Notification.EXTRA_MEDIA_SESSION) != null; + } + /** * @return the style class of this notification * @hide @@ -4515,12 +4541,23 @@ public class Notification implements Parcelable } /** - * @return true if this notification is colorized. This also factors in wheather the + * @return true if this notification is colorized. This also factors in whether the * notification is ongoing. * * @hide */ public boolean isColorized() { + Class style = getNotificationStyle(); + if (MediaStyle.class.equals(style)) { + Boolean colorized = (Boolean) extras.get(EXTRA_COLORIZED); + if ((colorized == null || colorized) && hasMediaSession()) { + return true; + } + } else if (DecoratedMediaCustomViewStyle.class.equals(style)) { + if (extras.getBoolean(EXTRA_COLORIZED) && hasMediaSession()) { + return true; + } + } return extras.getBoolean(EXTRA_COLORIZED) && isOngoing(); } @@ -5766,21 +5803,27 @@ public class Notification implements Parcelable * shown as icon-only pushbuttons, suitable for transport controls. The Bitmap given to * {@link Notification.Builder#setLargeIcon(android.graphics.Bitmap) setLargeIcon()} will be * treated as album artwork. - * + *

* Unlike the other styles provided here, MediaStyle can also modify the standard-size * {@link Notification#contentView}; by providing action indices to * {@link #setShowActionsInCompactView(int...)} you can promote up to 3 actions to be displayed * in the standard view alongside the usual content. - * + *

* Notifications created with MediaStyle will have their category set to * {@link Notification#CATEGORY_TRANSPORT CATEGORY_TRANSPORT} unless you set a different * category using {@link Notification.Builder#setCategory(String) setCategory()}. - * + *

* Finally, if you attach a {@link android.media.session.MediaSession.Token} using * {@link android.app.Notification.MediaStyle#setMediaSession(MediaSession.Token)}, * the System UI can identify this as a notification representing an active media session * and respond accordingly (by showing album artwork in the lockscreen, for example). * + *

+ * Starting at {@link android.os.Build.VERSION_CODES#O Android O} any notification that has a + * media session attached with {@link #setMediaSession(MediaSession.Token)} will be colorized. + * You can opt-out of this behavior by using {@link Notification.Builder#setColorized(boolean)}. + *

+ * * To use this style with your Notification, feed it to * {@link Notification.Builder#setStyle(android.app.Notification.Style)} like so: * 

@@ -5795,6 +5838,7 @@ public class Notification implements Parcelable
      *
* * @see Notification#bigContentView + * @see Notification.Builder#setColorized(boolean) */ public static class MediaStyle extends Style { static final int MAX_MEDIA_BUTTONS_IN_COMPACT = 3; @@ -5932,7 +5976,7 @@ public class Notification implements Parcelable final Action action = mBuilder.mActions.get(mActionsToShowInCompact[i]); final RemoteViews button = generateMediaActionButton(action, - mBuilder.resolveContrastColor()); + getPrimaryHighlightColor()); view.addView(com.android.internal.R.id.media_actions, button); } } @@ -5946,6 +5990,10 @@ public class Notification implements Parcelable return view; } + private int getPrimaryHighlightColor() { + return mBuilder.getPrimaryHighlightColor(); + } + private RemoteViews makeMediaBigContentView() { final int actionCount = Math.min(mBuilder.mActions.size(), MAX_MEDIA_BUTTONS); // Dont add an expanded view if there is no more content to be revealed @@ -5963,7 +6011,7 @@ public class Notification implements Parcelable big.removeAllViews(com.android.internal.R.id.media_actions); for (int i = 0; i < actionCount; i++) { final RemoteViews button = generateMediaActionButton(mBuilder.mActions.get(i), - mBuilder.resolveContrastColor()); + getPrimaryHighlightColor()); big.addView(com.android.internal.R.id.media_actions, button); } } @@ -6108,7 +6156,10 @@ public class Notification implements Parcelable * {@link android.app.Notification.Builder#setCustomBigContentView(RemoteViews)} and * {@link android.app.Notification.Builder#setCustomHeadsUpContentView(RemoteViews)} to set the * corresponding custom views to display. - * + *

+ * Contrary to {@link MediaStyle} a developer has to opt-in to the colorizing of the + * notification by using {@link Notification.Builder#setColorized(boolean)}. + *

* To use this style with your Notification, feed it to * {@link Notification.Builder#setStyle(android.app.Notification.Style)} like so: * 

diff --git a/core/java/android/view/NotificationHeaderView.java b/core/java/android/view/NotificationHeaderView.java
index 0da710a1fb33a..c3d3f3918ccf4 100644
--- a/core/java/android/view/NotificationHeaderView.java
+++ b/core/java/android/view/NotificationHeaderView.java
@@ -17,6 +17,7 @@
 package android.view;
 
 import android.annotation.Nullable;
+import android.app.Notification;
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Outline;
@@ -39,7 +40,7 @@ import java.util.ArrayList;
  */
 @RemoteViews.RemoteView
 public class NotificationHeaderView extends ViewGroup {
-    public static final int NO_COLOR = -1;
+    public static final int NO_COLOR = Notification.COLOR_INVALID;
     private final int mChildMinWidth;
     private final int mContentEndMargin;
     private View mAppName;
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/ActivatableNotificationView.java b/packages/SystemUI/src/com/android/systemui/statusbar/ActivatableNotificationView.java
index 4b1baa2edb396..0bd6491213218 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/ActivatableNotificationView.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/ActivatableNotificationView.java
@@ -105,7 +105,7 @@ public abstract class ActivatableNotificationView extends ExpandableOutlineView
     private boolean mDimmed;
     private boolean mDark;
 
-    private int mBgTint = NO_COLOR;
+    protected int mBgTint = NO_COLOR;
     private float mBgAlpha = 1f;
 
     /**
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java
index 85e2bd6161686..2e9c7fd85e420 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/ExpandableNotificationRow.java
@@ -1880,14 +1880,14 @@ public class ExpandableNotificationRow extends ActivatableNotificationView {
             } else if (isUserLocked()) {
                 return mChildrenContainer.getGroupExpandFraction();
             }
-        } else if (isColorized()) {
+        } else if (isColorized() && (!mIsLowPriority || isExpanded())) {
             return -1.0f;
         }
         return 0.0f;
     }
 
     private boolean isColorized() {
-        return mIsColorized;
+        return mIsColorized && mBgTint != NO_COLOR;
     }
 
     @Override
diff --git a/services/core/java/com/android/server/notification/NotificationComparator.java b/services/core/java/com/android/server/notification/NotificationComparator.java
index 279bf9005fe38..d6c89a41dc320 100644
--- a/services/core/java/com/android/server/notification/NotificationComparator.java
+++ b/services/core/java/com/android/server/notification/NotificationComparator.java
@@ -159,8 +159,7 @@ public class NotificationComparator
     }
 
     private boolean isMediaNotification(NotificationRecord record) {
-        return record.getNotification().extras.getParcelable(
-                Notification.EXTRA_MEDIA_SESSION) != null;
+        return record.getNotification().hasMediaSession();
     }
 
     private boolean isCall(NotificationRecord record) {