Notifications in sensitive contexts now display origin + small icon

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

Index: chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
index 21f25a8566e2eb443b20a747e47fd65c9c418c88..1c164af831e8b0681edd3d2faf58921a08ae017a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
@@ -11,6 +11,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
+import android.graphics.drawable.Icon;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
@@ -519,6 +520,9 @@ public class NotificationPlatformBridge {
                 makePendingIntent(NotificationConstants.ACTION_CLOSE_NOTIFICATION, notificationId,
                         origin, profileId, incognito, tag, webApkPackage, -1 /* actionIndex */);
 
+        String originForDisplay =
+                UrlFormatter.formatUrlForSecurityDisplay(origin, false /* showScheme */);
+
         NotificationBuilderBase notificationBuilder =
                 createNotificationBuilder()
                         .setTitle(title)
@@ -532,8 +536,12 @@ public class NotificationPlatformBridge {
                         .setTicker(createTickerText(title, body))
                         .setTimestamp(timestamp)
                         .setRenotify(renotify)
-                        .setOrigin(UrlFormatter.formatUrlForSecurityDisplay(
-                                origin, false /* showScheme */));
+                        .setOrigin(originForDisplay);
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
+            // Public versions of notifications are only supported on Android L+.
+            notificationBuilder.setPublicVersion(createPublicNotification(badge, originForDisplay));
+        }
 
         for (int actionIndex = 0; actionIndex < actionTitles.length; actionIndex++) {
             notificationBuilder.addAction(actionIcons[actionIndex], actionTitles[actionIndex],
@@ -575,6 +583,35 @@ public class NotificationPlatformBridge {
         }
     }
 
+    private Notification createPublicNotification(Bitmap badge, String originForDisplay) {

[Peter Beverloo, 2016/09/09 12:53:31]

What do you think about moving this to NotificationBuilderBase? It would isolate
android.app.Notification usage to that set of classes, and we might be able to
move the |mIconGenerator| member there too.

[awdf, 2016/09/09 15:55:26]

I agree it makes sense to move this method (& icon generator) to
NotificationBuilderBase. But where would you call it from - setPublicVersion()
(which would have to take two arguments), or get rid of the setter & call it
from the subclass build() methods?

[Peter Beverloo, 2016/09/09 15:58:23]

Calling it from the subclass' build() methods makes a lot of sense to me.
Instead of mPublicVersion we can have createPublicNotification() that uses the
information that's already available.

+        // Use Android's Notification.Builder because we want the default small icon behaviour.
+        Notification.Builder builder =
+                new Notification.Builder(mAppContext)
+                        .setContentText(mAppContext.getString(R.string.notification_hidden_text))
+                        .setSmallIcon(R.drawable.ic_chrome);
+
+        if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.M) {
+            // Set origin as title on L & M, because they look odd without one.
+            builder.setContentTitle(originForDisplay);
+        } else {
+            // On N(+?) 'subtext' displays at the top of the notification and this looks better.
+            builder.setSubText(originForDisplay);
+        }
+
+        // Use the badge if provided and SDK supports it, else use a generated icon.
+        if (badge != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {

[awdf, 2016/09/09 09:51:01]

I don't really like all these version checks, would probably be neater to use
the fact we already have two types of notification builder (basically depending
on N+ vs L/M), and pull all the public notification generation into their
build() methods.

However, doing that might be less clear than explicitly having a
setPublicVersion method on those builders as I've done here.

[Peter Beverloo, 2016/09/09 12:53:31]

I don't mind the version checks too much, the intention check is well documented
and none of the code is hard to follow.

Moving this to NotificationBuilderBase (we can't rely on
CustomNotificationBuilder being <N as users have the ability to enable/disable
custom layouts themselves) might help, since it already has all the necessary
information anyway. Having some NotificationBuilderBase.commonBuild() (modulo
naming :P) might remove duplication from the
{Custom,Standard}NotificationBuilder.build() methods too?

+            // The Icon class was added in Android M.
+            Bitmap publicIcon = badge.copy(badge.getConfig(), true);
+            NotificationBuilderBase.applyWhiteOverlayToBitmap(publicIcon);
+            builder.setSmallIcon(Icon.createWithBitmap(publicIcon));
+        } else if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.M) {
+            // Only set the large icon for L & M because on N(+?) it would add an extra icon on
+            // the right hand side, which looks odd without a notification title.
+            builder.setLargeIcon(generateLargeIconForUrl(originForDisplay));
+        }
+        return builder.build();
+    }
+
     private NotificationBuilderBase createNotificationBuilder() {
         if (useCustomLayouts()) {
             return new CustomNotificationBuilder(mAppContext);
@@ -618,26 +655,26 @@ public class NotificationPlatformBridge {
     @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 generateLargeIconForUrl(origin);
         }
-
         if (icon.getWidth() > mLargeIconWidthPx || icon.getHeight() > mLargeIconHeightPx) {
             return icon.createScaledBitmap(icon, mLargeIconWidthPx, mLargeIconHeightPx,
                                            false /* not filtered */);
         }
-
         return icon;
     }
 
+    private Bitmap generateLargeIconForUrl(String origin) {
+        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);
+    }
+
     /**
      * Determines whether to use standard notification layouts, using NotificationCompat.Builder,
      * or custom layouts using Chrome's own templates.