Scale large notification icons according to device settings.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
index 7cad72d9fe0532833fde4442f1e9c176209ddbf4..f3c165341b5cc730960603d2e58018bcf146aace 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
@@ -70,7 +70,10 @@ public class NotificationUIManager {
     private final Context mAppContext;
     private final NotificationManagerProxy mNotificationManager;
 
-    private RoundedIconGenerator mIconGenerator;
+    @VisibleForTesting public RoundedIconGenerator mIconGenerator;
+    private final int mLargeIconWidthPx;
+    private final int mLargeIconHeightPx;
+    private final float mDensity;
 
     private long mLastNotificationClickMs = 0L;
 
@@ -91,6 +94,17 @@ public class NotificationUIManager {
     }
 
     /**
+     * Returns the current instance of the NotificationUIManager. Should only be used by tests.

[Miguel Garcia, 2015/09/28 17:47:12]

Doesn't the @VisibleForTesting annotation already imply that this is only is
supposed to be private + test visibility?

Also consider making it package private since the test will presumably live in
the same package.

[Peter Beverloo, 2015/09/28 18:03:03]

Done.

+     *
+     * @return The instance of the NotificationUIManager, if any.
+     */
+    @Nullable
+    @VisibleForTesting
+    public static NotificationUIManager getInstanceForTests() {
+        return sInstance;
+    }
+
+    /**
      * Overrides the notification manager which is to be used for displaying Notifications on the
      * Android framework. Should only be used for testing. Tests are expected to clean up after
      * themselves by setting this to NULL again.
@@ -113,6 +127,14 @@ public class NotificationUIManager {
             mNotificationManager = new NotificationManagerProxyImpl(
                     (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE));
         }
+
+        Resources resources = mAppContext.getResources();
+

[Miguel Garcia, 2015/09/28 17:47:13]

nit can you initialize density after line 137 so it matches the order in which
they are declared above?

[Peter Beverloo, 2015/09/28 18:03:03]

Done.

+        mDensity = resources.getDisplayMetrics().density;
+        mLargeIconWidthPx =

[Miguel Garcia, 2015/09/28 17:47:13]

I assume neither of this can ever return 0? Otherwise createScaledBitmap in #506
will explode.

[Peter Beverloo, 2015/09/28 18:03:03]

Stack Overflow would have exploded already if that were the case :).

+            resources.getDimensionPixelSize(android.R.dimen.notification_large_icon_width);
+        mLargeIconHeightPx =
+            resources.getDimensionPixelSize(android.R.dimen.notification_large_icon_height);
     }
 
     /**
@@ -372,8 +394,8 @@ public class NotificationUIManager {
      * @param title Title to be displayed in the notification.
      * @param body Message to be displayed in the notification. Will be trimmed to one line of
      *             text by the Android notification system.
-     * @param icon Icon to be displayed in the notification. When this isn't a valid Bitmap, a
-     *             default icon will be generated instead.
+     * @param icon Icon to be displayed in the notification. Valid Bitmap icons will be scaled to
+     *             the platforms, whereas a default icon will be generated for invalid Bitmaps.
      * @param vibrationPattern Vibration pattern following the Web Vibration syntax.
      * @param silent Whether the default sound, vibration and lights should be suppressed.
      * @param actionTitles Titles of actions to display alongside the notification.
@@ -383,10 +405,6 @@ public class NotificationUIManager {
     private void displayNotification(long persistentNotificationId, String origin, String tag,
             String title, String body, Bitmap icon, int[] vibrationPattern, boolean silent,
             String[] actionTitles) {
-        if (icon == null || icon.getWidth() == 0) {
-            icon = getIconGenerator().generateIconForUrl(origin, true);
-        }
-
         Resources res = mAppContext.getResources();
 
         // Set up a pending intent for going to the settings screen for |origin|.
@@ -404,7 +422,7 @@ public class NotificationUIManager {
                 .setContentTitle(title)
                 .setContentText(body)
                 .setStyle(new NotificationCompat.BigTextStyle().bigText(body))
-                .setLargeIcon(icon)
+                .setLargeIcon(ensureNormalizedIcon(icon, origin))
                 .setSmallIcon(R.drawable.ic_chrome)
                 .setContentIntent(makePendingIntent(
                         NotificationConstants.ACTION_CLICK_NOTIFICATION,
@@ -459,40 +477,36 @@ public class NotificationUIManager {
     }
 
     /**
-     * Ensures the existance of an icon generator, which is created lazily.
+     * Ensures the availability of an icon for the notification.
+     *
+     * If |icon| is a valid, non-empty Bitmap, the bitmap will be scaled to be of an appropriate
+     * size for the current Android device. Otherwise, a default icon will be created based on the
+     * origin the notification is being displayed for.
      *
-     * @return The icon generator which can be used.
+     * @param icon The developer-provided icon they intend to use for the notification.
+     * @param origin The origin the notification is being displayed for.
+     * @return An appropriately sized icon to use for the notification.
      */
-    private RoundedIconGenerator getIconGenerator() {
-        if (mIconGenerator == null) {
-            mIconGenerator = createRoundedIconGenerator(mAppContext);
+    @VisibleForTesting
+    public Bitmap ensureNormalizedIcon(Bitmap icon, String origin) {
+        if (icon == null || icon.getWidth() == 0) {
+            if (mIconGenerator == null) {
+                int cornerRadiusPx = Math.min(mLargeIconWidthPx, mLargeIconHeightPx) / 2;
+                mIconGenerator =
+                        new RoundedIconGenerator(mLargeIconWidthPx, mLargeIconHeightPx,
+                                                 cornerRadiusPx,
+                                                 NOTIFICATION_ICON_BG_COLOR,
+                                                 NOTIFICATION_TEXT_SIZE_DP * mDensity);
+            }
+
+            return mIconGenerator.generateIconForUrl(origin, true);
         }
 
-        return mIconGenerator;
-    }
+        if (icon.getWidth() > mLargeIconWidthPx || icon.getHeight() > mLargeIconHeightPx) {
+            return icon.createScaledBitmap(icon, mLargeIconWidthPx, mLargeIconHeightPx, false);

[Miguel Garcia, 2015/09/28 17:47:12]

can you add /* not filtered */ right after the last parameter?

[Peter Beverloo, 2015/09/28 18:03:03]

Done.

+        }
 
-    /**
-     * Creates the rounded icon generator to use for notifications based on the dimensions
-     * and resolution of the device we're running on.
-     *
-     * @param appContext The application context to retrieve resources from.
-     * @return The newly created rounded icon generator.
-     */
-    @VisibleForTesting
-    public static RoundedIconGenerator createRoundedIconGenerator(Context appContext) {
-        Resources res = appContext.getResources();
-        float density = res.getDisplayMetrics().density;
-
-        int widthPx = res.getDimensionPixelSize(android.R.dimen.notification_large_icon_width);
-        int heightPx =
-                res.getDimensionPixelSize(android.R.dimen.notification_large_icon_height);
-
-        return new RoundedIconGenerator(
-                widthPx,
-                heightPx,
-                Math.min(widthPx, heightPx) / 2,
-                NOTIFICATION_ICON_BG_COLOR,
-                NOTIFICATION_TEXT_SIZE_DP * density);
+        return icon;
     }
 
     /**