Commit 9ab2faeb authored by Xi Han's avatar Xi Han Committed by Commit Bot

Chrome sends notifications with Channel to WebAPKs which targets SDK 26 on Android O.

In Chrome:
- WebAPK's channel is set under "Sites" notification group and
  "readable origin" as the channel, the same channel as Webapps.
- If the device is Android o+, Chrome will set a channel to the
  notification.

In WebAPK:
- WebApkServiceImpl#notifyNotification() is deprecated. Chrome calls
  notifyNotificationWithChannel() instead.
- If on Android O+, WebAPK will ensure the notification channel is
  created if its notifications have a channel id.

To test this change, we need to make sure the following cases all work:
- New WebAPK, new Chrome;
- New WebAPK, old Chrome;
- Old WebAPK, new Chrome.

Bug: 700228
Change-Id: I176a8d45f021a33ea441837476b1ca09e973a48b
Reviewed-on: https://chromium-review.googlesource.com/786300Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarPeter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: default avatarAnita Woodruff <awdf@chromium.org>
Commit-Queue: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521841}
parent a9783e12
......@@ -79,8 +79,8 @@ public class CustomNotificationBuilder extends NotificationBuilderBase {
private final Context mContext;
public CustomNotificationBuilder(Context context, String channelId) {
super(context.getResources(), channelId);
public CustomNotificationBuilder(Context context) {
super(context.getResources());
mContext = context;
}
......
......@@ -106,11 +106,11 @@ public abstract class NotificationBuilderBase {
private final int mLargeIconWidthPx;
private final int mLargeIconHeightPx;
private final RoundedIconGenerator mIconGenerator;
protected final String mChannelId;
protected CharSequence mTitle;
protected CharSequence mBody;
protected CharSequence mOrigin;
protected String mChannelId;
protected CharSequence mTickerText;
protected Bitmap mImage;
protected int mSmallIconId;
......@@ -126,13 +126,12 @@ public abstract class NotificationBuilderBase {
protected int mPriority;
private Bitmap mLargeIcon;
public NotificationBuilderBase(Resources resources, String channelId) {
public NotificationBuilderBase(Resources resources) {
mLargeIconWidthPx =
resources.getDimensionPixelSize(android.R.dimen.notification_large_icon_width);
mLargeIconHeightPx =
resources.getDimensionPixelSize(android.R.dimen.notification_large_icon_height);
mIconGenerator = createIconGenerator(resources);
mChannelId = channelId;
}
/**
......@@ -235,6 +234,14 @@ public abstract class NotificationBuilderBase {
return this;
}
/**
* Sets the channel id of the notification.
*/
public NotificationBuilderBase setChannelId(String channelId) {
mChannelId = channelId;
return this;
}
/**
* Adds an action to the notification, displayed as a button adjacent to the notification
* content.
......
......@@ -7,8 +7,10 @@ package org.chromium.chrome.browser.notifications;
import android.annotation.TargetApi;
import android.content.Context;
import android.os.Build;
import android.text.TextUtils;
import org.chromium.chrome.browser.notifications.channels.ChannelsInitializer;
import org.chromium.chrome.browser.webapps.WebApkServiceClient;
/**
* Builder to be used on Android O until we target O and these APIs are in the support library.
......@@ -26,7 +28,12 @@ public class NotificationBuilderForO extends NotificationBuilder {
// does not target O or sets its own channels. E.g. Web apk notifications.
return;
}
channelsInitializer.ensureInitialized(channelId);
// If the channel ID matches {@link WebApkServiceClient#CHANNEL_ID_WEBAPKS}, we don't create
// the channel in Chrome. Instead, the channel will be created in WebAPKs.
if (!TextUtils.equals(channelId, WebApkServiceClient.CHANNEL_ID_WEBAPKS)) {
channelsInitializer.ensureInitialized(channelId);
}
mBuilder.setChannelId(channelId);
}
}
......@@ -583,7 +583,7 @@ public class NotificationPlatformBridge {
boolean hasImage = image != null;
boolean forWebApk = !webApkPackage.isEmpty();
NotificationBuilderBase notificationBuilder =
createNotificationBuilder(context, forWebApk, hasImage, origin)
createNotificationBuilder(context, hasImage)
.setTitle(title)
.setBody(body)
.setImage(image)
......@@ -598,6 +598,16 @@ public class NotificationPlatformBridge {
.setOrigin(UrlFormatter.formatUrlForSecurityDisplay(
origin, false /* showScheme */));
if (shouldSetChannelId(forWebApk)) {
// TODO(crbug.com/700377): Channel ID should be retrieved from cache in native and
// passed through to here with other notification parameters.
String channelId =
ChromeFeatureList.isEnabled(ChromeFeatureList.SITE_NOTIFICATION_CHANNELS)
? SiteChannelsManager.getInstance().getChannelIdForOrigin(origin)
: ChannelDefinitions.CHANNEL_ID_SITES;
notificationBuilder.setChannelId(channelId);
}
for (int actionIndex = 0; actionIndex < actions.length; actionIndex++) {
PendingIntent intent = makePendingIntent(context,
NotificationConstants.ACTION_CLICK_NOTIFICATION, notificationId, origin,
......@@ -661,22 +671,16 @@ public class NotificationPlatformBridge {
}
}
private NotificationBuilderBase createNotificationBuilder(
Context context, boolean forWebApk, boolean hasImage, String origin) {
// Don't set a channelId for web apk notifications because the channel won't be
// initialized for the web apk and it will crash on notify - see crbug.com/727178.
// (It's okay to not set a channel on them because web apks don't target O yet.)
// TODO(crbug.com/700377): Channel ID should be retrieved from cache in native and passed
// through to here with other notification parameters.
String channelId = (forWebApk || Build.VERSION.SDK_INT < Build.VERSION_CODES.O)
? null
: ChromeFeatureList.isEnabled(ChromeFeatureList.SITE_NOTIFICATION_CHANNELS)
? SiteChannelsManager.getInstance().getChannelIdForOrigin(origin)
: ChannelDefinitions.CHANNEL_ID_SITES;
private NotificationBuilderBase createNotificationBuilder(Context context, boolean hasImage) {
if (useCustomLayouts(hasImage)) {
return new CustomNotificationBuilder(context, channelId);
return new CustomNotificationBuilder(context);
}
return new StandardNotificationBuilder(context, channelId);
return new StandardNotificationBuilder(context);
}
/** Returns whether to set a channel id when building a notification. */
private boolean shouldSetChannelId(boolean forWebApk) {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !forWebApk;
}
/**
......
......@@ -14,8 +14,8 @@ import android.os.Build;
public class StandardNotificationBuilder extends NotificationBuilderBase {
private final Context mContext;
public StandardNotificationBuilder(Context context, String channelId) {
super(context.getResources(), channelId);
public StandardNotificationBuilder(Context context) {
super(context.getResources());
mContext = context;
}
......
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.webapps;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.content.res.Resources;
import android.graphics.Bitmap;
......@@ -45,6 +46,12 @@ public class WebApkServiceClient {
}
}
/**
* Keeps the value consistent with {@link
* org.chromium.webapk.shell_apk.WebApkServiceImplWrapper#DEFAULT_NOTIFICATION_CHANNEL_ID}.
*/
public static final String CHANNEL_ID_WEBAPKS = "default_channel_id";
private static final String CATEGORY_WEBAPK_API = "android.intent.category.WEBAPK_API";
private static final String TAG = "cr_WebApk";
......@@ -93,7 +100,14 @@ public class WebApkServiceClient {
}
boolean notificationPermissionEnabled = api.notificationPermissionEnabled();
if (notificationPermissionEnabled) {
api.notifyNotification(platformTag, platformID, notificationBuilder.build());
String channelName = null;
if (webApkTargetsAtLeastO(webApkPackage)) {
notificationBuilder.setChannelId(CHANNEL_ID_WEBAPKS);
channelName = ContextUtils.getApplicationContext().getString(
org.chromium.chrome.R.string.webapk_notification_channel_name);
}
api.notifyNotificationWithChannel(
platformTag, platformID, notificationBuilder.build(), channelName);
}
WebApkUma.recordNotificationPermissionStatus(notificationPermissionEnabled);
}
......@@ -134,4 +148,17 @@ public class WebApkServiceClient {
return null;
}
}
/** Returns whether the WebAPK targets SDK 26+. */
private boolean webApkTargetsAtLeastO(String webApkPackage) {
try {
ApplicationInfo info =
ContextUtils.getApplicationContext().getPackageManager().getApplicationInfo(
webApkPackage, 0);
return info.targetSdkVersion >= Build.VERSION_CODES.O;
} catch (PackageManager.NameNotFoundException e) {
}
return false;
}
}
......@@ -3025,6 +3025,9 @@ You must have Bluetooth and Location turned on in order to use the Physical Web.
<message name="IDS_WEBAPK_OFFLINE_DIALOG" desc="The message on the dialog shown when launching a WebAPK needs network connection.">
To use <ph name="APP_NAME">%1$s<ex>PWA List</ex></ph> for the first time, please connect to the internet.
</message>
<message name="IDS_WEBAPK_NOTIFICATION_CHANNEL_NAME" desc="The visible name of the notification channel of WebAPKs on Android O+.">
General
</message>
<!-- Keyboard shortcuts in Android N-->
<message name="IDS_KEYBOARD_SHORTCUT_OPEN_NEW_TAB" desc="A text label that appears next to the keyboard shortcut to open a new tab in Chrome. The shortcut description is shown in a system dialog along with all other supported shortcuts. [CHAR-LIMIT=55]">
......
......@@ -77,25 +77,25 @@ public class CustomNotificationBuilderTest {
new int[] {Color.WHITE}, 1 /* width */, 1 /* height */, Bitmap.Config.ARGB_8888);
actionIcon = actionIcon.copy(Bitmap.Config.ARGB_8888, true /* isMutable */);
Notification notification =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
.setSmallIcon(R.drawable.ic_chrome)
.setLargeIcon(largeIcon)
.setTitle("title")
.setBody("body")
.setOrigin("origin")
.setTicker("ticker")
.setDefaults(Notification.DEFAULT_ALL)
.setVibrate(new long[] {100L})
.setContentIntent(contentIntent)
.setDeleteIntent(deleteIntent)
.addButtonAction(
actionIcon, "button", createIntent(context, "ActionButtonOne"))
.addButtonAction(
actionIcon, "button", createIntent(context, "ActionButtonTwo"))
.addSettingsAction(
0 /* iconId */, "settings", createIntent(context, "SettingsButton"))
.build();
Notification notification = new CustomNotificationBuilder(context)
.setSmallIcon(R.drawable.ic_chrome)
.setLargeIcon(largeIcon)
.setTitle("title")
.setBody("body")
.setOrigin("origin")
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.setTicker("ticker")
.setDefaults(Notification.DEFAULT_ALL)
.setVibrate(new long[] {100L})
.setContentIntent(contentIntent)
.setDeleteIntent(deleteIntent)
.addButtonAction(actionIcon, "button",
createIntent(context, "ActionButtonOne"))
.addButtonAction(actionIcon, "button",
createIntent(context, "ActionButtonTwo"))
.addSettingsAction(0 /* iconId */, "settings",
createIntent(context, "SettingsButton"))
.build();
assertSmallNotificationIconAsExpected(context, notification, smallIcon);
assertLargeNotificationIconAsExpected(context, notification, largeIcon);
......@@ -149,8 +149,9 @@ public class CustomNotificationBuilderTest {
@DisableIf.Build(sdk_is_greater_than = 23, message = "crbug.com/779228")
public void testZeroActionButtons() {
Context context = InstrumentationRegistry.getTargetContext();
Notification notification =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES).build();
Notification notification = new CustomNotificationBuilder(context)
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.build();
View bigView = notification.bigContentView.apply(context, new LinearLayout(context));
ArrayList<View> buttons = new ArrayList<>();
bigView.findViewsWithText(buttons, "button", View.FIND_VIEWS_WITH_TEXT);
......@@ -167,12 +168,12 @@ public class CustomNotificationBuilderTest {
@DisableIf.Build(sdk_is_greater_than = 23, message = "crbug.com/779228")
public void testMaxActionButtons() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase builder =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
.addButtonAction(null /* iconBitmap */, "button",
createIntent(context, "ActionButtonOne"))
.addButtonAction(null /* iconBitmap */, "button",
createIntent(context, "ActionButtonTwo"));
NotificationBuilderBase builder = new CustomNotificationBuilder(context)
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.addButtonAction(null /* iconBitmap */, "button",
createIntent(context, "ActionButtonOne"))
.addButtonAction(null /* iconBitmap */, "button",
createIntent(context, "ActionButtonTwo"));
try {
builder.addButtonAction(
null /* iconBitmap */, "button", createIntent(context, "ActionButtonThree"));
......@@ -208,13 +209,13 @@ public class CustomNotificationBuilderTest {
new int[] {Color.RED}, 1 /* width */, 1 /* height */, Bitmap.Config.ARGB_8888);
actionIcon = actionIcon.copy(Bitmap.Config.ARGB_8888, true /* isMutable */);
Notification notification =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
.setLargeIcon(largeIcon)
.setSmallIcon(smallIcon)
.addButtonAction(
actionIcon, "button", createIntent(context, "ActionButton"))
.build();
Notification notification = new CustomNotificationBuilder(context)
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.setLargeIcon(largeIcon)
.setSmallIcon(smallIcon)
.addButtonAction(actionIcon, "button",
createIntent(context, "ActionButton"))
.build();
// The large icon should be unchanged.
assertLargeNotificationIconAsExpected(context, notification, largeIcon);
......@@ -240,10 +241,11 @@ public class CustomNotificationBuilderTest {
Context context = InstrumentationRegistry.getTargetContext();
int maxLength = CustomNotificationBuilder.MAX_CHARSEQUENCE_LENGTH;
Notification notification =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
new CustomNotificationBuilder(context)
.setTitle(createString('a', maxLength + 1))
.setBody(createString('b', maxLength + 1))
.setOrigin(createString('c', maxLength + 1))
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.setTicker(createString('d', maxLength + 1))
.addButtonAction(null /* iconBitmap */, createString('e', maxLength + 1),
createIntent(context, "ActionButtonOne"))
......@@ -299,8 +301,9 @@ public class CustomNotificationBuilderTest {
public void testGeneratesLargeIconFromOriginWhenNoLargeIconProvided() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase notificationBuilder =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
.setOrigin("https://www.google.com");
new CustomNotificationBuilder(context)
.setOrigin("https://www.google.com")
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES);
Notification notification = notificationBuilder.build();
......@@ -317,8 +320,9 @@ public class CustomNotificationBuilderTest {
public void testGeneratesLargeIconFromOriginWhenLargeIconProvidedIsNull() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase notificationBuilder =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
new CustomNotificationBuilder(context)
.setOrigin("https://www.chromium.org")
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.setLargeIcon(null);
Notification notification = notificationBuilder.build();
......@@ -344,7 +348,8 @@ public class CustomNotificationBuilderTest {
public void testAddTextActionSetsRemoteInput() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase notificationBuilder =
new CustomNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
new CustomNotificationBuilder(context)
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.addTextAction(null, "Action Title", null, "Placeholder");
Notification notification = notificationBuilder.build();
......
......@@ -66,13 +66,13 @@ public class NotificationBuilderBaseTest {
String origin = "https://example.com";
NotificationBuilderBase notificationBuilder = new NotificationBuilderBase(resources,
ChannelDefinitions.CHANNEL_ID_BROWSER) {
NotificationBuilderBase notificationBuilder = new NotificationBuilderBase(resources) {
@Override
public Notification build() {
return null;
}
};
notificationBuilder.setChannelId(ChannelDefinitions.CHANNEL_ID_BROWSER);
Bitmap fromNullIcon = notificationBuilder.ensureNormalizedIcon(null, origin);
Assert.assertNotNull(fromNullIcon);
Assert.assertEquals(largeIconWidthPx, fromNullIcon.getWidth());
......
......@@ -79,10 +79,11 @@ public class StandardNotificationBuilderTest {
new int[] {Color.GRAY}, 1 /* width */, 1 /* height */, Bitmap.Config.ARGB_8888);
actionIcon = actionIcon.copy(Bitmap.Config.ARGB_8888, true /* isMutable */);
return new StandardNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
return new StandardNotificationBuilder(context)
.setTitle("title")
.setBody("body")
.setOrigin("origin")
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.setTicker(new SpannableStringBuilder("ticker"))
.setImage(image)
.setLargeIcon(largeIcon)
......@@ -176,14 +177,14 @@ public class StandardNotificationBuilderTest {
@Feature({"Browser", "Notifications"})
public void testSetSmallIcon() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase notificationBuilder =
new StandardNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES);
NotificationBuilderBase notificationBuilder = new StandardNotificationBuilder(context);
Bitmap bitmap =
BitmapFactory.decodeResource(context.getResources(), R.drawable.chrome_sync_logo);
notificationBuilder.setSmallIcon(R.drawable.ic_chrome);
notificationBuilder.setSmallIcon(bitmap); // Should override on M+
notificationBuilder.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES);
Notification notification = notificationBuilder.build();
......@@ -198,9 +199,8 @@ public class StandardNotificationBuilderTest {
Assert.assertTrue(expected.sameAs(result));
// Check using the same bitmap on another builder gives the same result.
NotificationBuilderBase otherBuilder =
new StandardNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES);
otherBuilder.setSmallIcon(bitmap);
NotificationBuilderBase otherBuilder = new StandardNotificationBuilder(context);
otherBuilder.setSmallIcon(bitmap).setChannelId(ChannelDefinitions.CHANNEL_ID_SITES);
Notification otherNotification = otherBuilder.build();
Assert.assertTrue(expected.sameAs(
NotificationTestUtil.getSmallIconFromNotification(context, otherNotification)));
......@@ -219,7 +219,8 @@ public class StandardNotificationBuilderTest {
public void testAddTextActionSetsRemoteInput() {
Context context = InstrumentationRegistry.getTargetContext();
NotificationBuilderBase notificationBuilder =
new StandardNotificationBuilder(context, ChannelDefinitions.CHANNEL_ID_SITES)
new StandardNotificationBuilder(context)
.setChannelId(ChannelDefinitions.CHANNEL_ID_SITES)
.addTextAction(null, "Action Title", null, "Placeholder");
Notification notification = notificationBuilder.build();
......
......@@ -14,6 +14,7 @@ interface IWebApkApi {
int getSmallIconId();
// Display a notification.
// DEPRECATED: Use notifyNotificationWithChannel.
void notifyNotification(String platformTag, int platformID, in Notification notification);
// Cancel a notification.
......@@ -21,4 +22,8 @@ interface IWebApkApi {
// Get if notification permission is enabled.
boolean notificationPermissionEnabled();
// Display a notification with a specified channel name.
void notifyNotificationWithChannel(String platformTag, int platformID,
in Notification notification, String channelName);
}
......@@ -4,14 +4,18 @@
package org.chromium.webapk.lib.runtime_library;
import android.annotation.TargetApi;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.content.Context;
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.Parcel;
import android.os.RemoteException;
import android.support.v4.app.NotificationManagerCompat;
import android.util.Log;
/**
* Implements services offered by the WebAPK to Chrome.
......@@ -21,7 +25,7 @@ public class WebApkServiceImpl extends IWebApkApi.Stub {
public static final String KEY_SMALL_ICON_ID = "small_icon_id";
public static final String KEY_HOST_BROWSER_UID = "host_browser_uid";
private static final String TAG = "WebApkServiceImpl";
private static final String TAG = "cr_WebApkServiceImpl";
private final Context mContext;
......@@ -65,7 +69,9 @@ public class WebApkServiceImpl extends IWebApkApi.Stub {
@Override
public void notifyNotification(String platformTag, int platformID, Notification notification) {
getNotificationManager().notify(platformTag, platformID, notification);
Log.w(TAG,
"Should NOT reach WebApkServiceImpl#notifyNotification(String, int,"
+ " Notification).");
}
@Override
......@@ -78,7 +84,20 @@ public class WebApkServiceImpl extends IWebApkApi.Stub {
return NotificationManagerCompat.from(mContext).areNotificationsEnabled();
}
@TargetApi(Build.VERSION_CODES.O)
@Override
public void notifyNotificationWithChannel(
String platformTag, int platformID, Notification notification, String channelName) {
NotificationManager notificationManager = getNotificationManager();
if (notification.getChannelId() != null) {
NotificationChannel channel = new NotificationChannel(notification.getChannelId(),
channelName, NotificationManager.IMPORTANCE_DEFAULT);
notificationManager.createNotificationChannel(channel);
}
notificationManager.notify(platformTag, platformID, notification);
}
private NotificationManager getNotificationManager() {
return (NotificationManager) mContext.getSystemService(Context.NOTIFICATION_SERVICE);
}
}
}
\ No newline at end of file
......@@ -5,4 +5,4 @@
# Must be incremented whenever the runtime library is updated. The WebAPK
# re-extracts the runtime library from the Chrome APK when
# |runtime_library_version| is incremented.
runtime_library_version = 3
runtime_library_version = 4
......@@ -93,6 +93,14 @@ public class WebApkServiceImplWrapper extends IWebApkApi.Stub {
Log.w(TAG, "Should NOT reach WebApkServiceImplWrapper#cancelNotification(String, int).");
}
@Override
public void notifyNotificationWithChannel(
String platformTag, int platformID, Notification notification, String channelName) {
Log.w(TAG,
"Should NOT reach WebApkServiceImplWrapper#notifyNotificationWithChannel("
+ "String, int, Notification, String)");
}
/** Creates a WebAPK notification channel on Android O+ if one does not exist. */
protected void ensureNotificationChannelExists() {
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.O) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment