Plumb Notification action icons through to the UI layer.

chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java

Index: chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
index 9fab09e420a46fe11139b7add4ffd13fca20e614..f04eebc1152bfd0b073609c96c37686fddb5bcbd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
@@ -12,8 +12,6 @@
 import android.graphics.drawable.BitmapDrawable;
 import android.graphics.drawable.Drawable;
 import android.os.Build;
-import android.support.v4.app.NotificationCompat;
-import android.support.v4.app.NotificationCompat.Action;
 import android.text.format.DateFormat;
 import android.util.DisplayMetrics;
 import android.util.TypedValue;
@@ -32,6 +30,11 @@
  */
 public class CustomNotificationBuilder extends NotificationBuilderBase {
     /**
+     * The maximum width of action icons in dp units.
+     */
+    private static final int MAX_ACTION_ICON_WIDTH_DP = 32;

[Peter Beverloo, 2016/02/10 12:29:37]

Looking at AOSP from JB onward, the width/height of an action icon are 48dp
having an 8dp padding all around, thus (48-2*8)=32dp. Is that correct?

[Michael van Ouwerkerk, 2016/02/10 17:18:12]

Yes, that's correct. And there's a simpler and more official bit of
documentation for addAction here:
http://developer.android.com/reference/android/app/Notification.Builder.html

+
+    /**
      * The maximum number of lines of body text for the expanded state. Fewer lines are used when
      * the text is scaled up, with a minimum of one line.
      */
@@ -116,7 +119,7 @@ public Notification build() {
             bigView.setViewVisibility(R.id.small_icon_footer, View.VISIBLE);
         }
 
-        NotificationCompat.Builder builder = new NotificationCompat.Builder(mContext);
+        Notification.Builder builder = new Notification.Builder(mContext);
         builder.setTicker(mTickerText);
         builder.setSmallIcon(mSmallIconId);
         builder.setContentIntent(mContentIntent);
@@ -133,10 +136,10 @@ public Notification build() {
         builder.setSubText(mOrigin);
         builder.setLargeIcon(mLargeIcon);
         for (Action action : mActions) {
-            builder.addAction(action);
+            addAction(builder, action);
         }
         if (mSettingsAction != null) {
-            builder.addAction(mSettingsAction);
+            addAction(builder, mSettingsAction);
         }
 
         Notification notification = builder.build();
@@ -163,30 +166,40 @@ private void addActionButtons(RemoteViews bigView) {
             RemoteViews view =
                     new RemoteViews(mContext.getPackageName(), R.layout.web_notification_button);
 
-            if (action.getIcon() != 0) {
+            if ((action.iconBitmap != null && useActionIconBitmaps()) || action.iconId != 0) {

[Peter Beverloo, 2016/02/10 12:29:37]

Part of the idea for custom layouts was the ability for Android <M to have
action icons, you explicitly disable that here, with a comment on line 304
referring an API that's not being used.

RemoteViews.setImageViewBitmap() is API >=3, so I don't think we need this
restriction?

[Michael van Ouwerkerk, 2016/02/10 17:18:12]

Ah, sorry, that was not very neat. Done.

                 // TODO(mvanouwerkerk): If the icon can be provided by web developers, limit its
                 // dimensions and decide whether or not to paint it.
                 if (useMaterial()) {
                     view.setInt(R.id.button_icon, "setColorFilter", BUTTON_ICON_COLOR_MATERIAL);
                 }
-                view.setImageViewResource(R.id.button_icon, action.getIcon());
+
+                int iconWidth = 0;
+                if (action.iconBitmap != null && useActionIconBitmaps()) {
+                    view.setImageViewBitmap(R.id.button_icon, action.iconBitmap);
+                    iconWidth = action.iconBitmap.getWidth();
+                } else if (action.iconId != 0) {

[Peter Beverloo, 2016/02/10 12:29:37]

It's a shame that asserts don't work, I'd have loved this to be "} else { assert
action.iconId != 0; ..."

[Michael van Ouwerkerk, 2016/02/10 17:18:12]

Acknowledged.

+                    view.setImageViewResource(R.id.button_icon, action.iconId);
+                    BitmapFactory.Options options = new BitmapFactory.Options();
+                    options.inJustDecodeBounds = true;
+                    BitmapFactory.decodeResource(resources, action.iconId, options);
+                    iconWidth = options.outWidth;
+                }
+                iconWidth = dpToPx(
+                        Math.min(pxToDp(iconWidth, metrics), MAX_ACTION_ICON_WIDTH_DP), metrics);
 
                 // Set the padding of the button so the text does not overlap with the icon. Flip
                 // between left and right manually as RemoteViews does not expose a method that sets
                 // padding in a writing-direction independent way.
-                BitmapFactory.Options options = new BitmapFactory.Options();
-                options.inJustDecodeBounds = true;
-                BitmapFactory.decodeResource(resources, action.getIcon(), options);
                 int buttonPadding =
                         dpToPx(BUTTON_PADDING_START_DP + BUTTON_ICON_PADDING_DP, metrics)
-                        + options.outWidth;
+                        + iconWidth;
                 int buttonPaddingLeft = LocalizationUtils.isLayoutRtl() ? 0 : buttonPadding;
                 int buttonPaddingRight = LocalizationUtils.isLayoutRtl() ? buttonPadding : 0;
                 view.setViewPadding(R.id.button, buttonPaddingLeft, 0, buttonPaddingRight, 0);
             }
 
-            view.setTextViewText(R.id.button, action.getTitle());
-            view.setOnClickPendingIntent(R.id.button, action.getActionIntent());
+            view.setTextViewText(R.id.button, action.title);
+            view.setOnClickPendingIntent(R.id.button, action.intent);
             bigView.addView(R.id.buttons, view);
         }
     }
@@ -195,7 +208,7 @@ private void configureSettingsButton(RemoteViews bigView) {
         if (mSettingsAction == null) {
             return;
         }
-        bigView.setOnClickPendingIntent(R.id.origin, mSettingsAction.getActionIntent());
+        bigView.setOnClickPendingIntent(R.id.origin, mSettingsAction.intent);
         if (useMaterial()) {
             bigView.setInt(R.id.origin_settings_icon, "setColorFilter", BUTTON_ICON_COLOR_MATERIAL);
         }
@@ -254,21 +267,27 @@ static int calculateMaxBodyLines(float fontScale) {
      * @return The amount of padding to be used, in pixels.
      */
     @VisibleForTesting
-    static int calculateScaledPadding(float fontScale, DisplayMetrics displayMetrics) {
+    static int calculateScaledPadding(float fontScale, DisplayMetrics metrics) {
         float paddingScale = 1.0f;
         if (fontScale > 1.0f) {
             fontScale = Math.min(fontScale, FONT_SCALE_LARGE);
             paddingScale = (FONT_SCALE_LARGE - fontScale) / (FONT_SCALE_LARGE - 1.0f);
         }
-        return dpToPx(paddingScale * MAX_SCALABLE_PADDING_DP, displayMetrics);
+        return dpToPx(paddingScale * MAX_SCALABLE_PADDING_DP, metrics);
     }
 
     /**
      * Converts a dp value to a px value.
      */
-    private static int dpToPx(float value, DisplayMetrics displayMetrics) {
-        return Math.round(
-                TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, value, displayMetrics));
+    private static int dpToPx(float value, DisplayMetrics metrics) {
+        return Math.round(TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, value, metrics));
+    }
+
+    /**
+     * Converts a px value to a dp value.
+     */
+    private static int pxToDp(float value, DisplayMetrics metrics) {
+        return Math.round(value / (float) (metrics.densityDpi / DisplayMetrics.DENSITY_DEFAULT));
     }
 
     /**
@@ -277,4 +296,13 @@ private static int dpToPx(float value, DisplayMetrics displayMetrics) {
     private static boolean useMaterial() {
         return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
     }
+
+    /**
+     * Whether to use bitmaps for action icons, if they are specified.
+     */
+    private static boolean useActionIconBitmaps() {
+        // The Notification.Action.Builder(Icon, CharSequence, PendingIntent) constructor is API
+        // level 23.
+        return Build.VERSION.SDK_INT >= Build.VERSION_CODES.M;
+    }
 }