Commit dc21e3e8 authored by Rayan Kanso's avatar Rayan Kanso Committed by Commit Bot

Fix broken PWA notification badge.

The WebApkServiceClient was overwriting the small icon in the
notification builder. This change checks whether there was already a
provided small icon bitmap before overwriting.

This is the CL that introduced the bug:
https://chromium-review.googlesource.com/c/chromium/src/+/978126
It will always call setSmallIconForRemoteApp on the notification builder for PWAs. For Android M+
this will overwrite the provided small icon bitmap.

Bug: 867351
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: If65970f3349e6a0738003d89e2721e3b2568afdc
Reviewed-on: https://chromium-review.googlesource.com/1181043Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Reviewed-by: default avatarSami Kyöstilä <skyostil@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584475}
parent b369d717
...@@ -63,11 +63,12 @@ public class TrustedWebActivityClient { ...@@ -63,11 +63,12 @@ public class TrustedWebActivityClient {
String channelDisplayName = res.getString(R.string.notification_category_group_general); String channelDisplayName = res.getString(R.string.notification_category_group_general);
mConnection.execute(scope, new Origin(scope).toString(), service -> { mConnection.execute(scope, new Origin(scope).toString(), service -> {
if (!builder.hasSmallIconBitmap()) {
int smallIconId = service.getSmallIconId(); int smallIconId = service.getSmallIconId();
if (smallIconId != -1) { if (smallIconId != -1) {
builder.setSmallIconForRemoteApp(smallIconId, builder.setSmallIconForRemoteApp(
service.getComponentName().getPackageName()); smallIconId, service.getComponentName().getPackageName());
}
} }
Notification notification = builder.build(); Notification notification = builder.build();
......
...@@ -80,8 +80,10 @@ public class WebApkServiceClient { ...@@ -80,8 +80,10 @@ public class WebApkServiceClient {
final ApiUseCallback connectionCallback = new ApiUseCallback() { final ApiUseCallback connectionCallback = new ApiUseCallback() {
@Override @Override
public void useApi(IWebApkApi api) throws RemoteException { public void useApi(IWebApkApi api) throws RemoteException {
int smallIconId = api.getSmallIconId(); if (!notificationBuilder.hasSmallIconBitmap()) {
notificationBuilder.setSmallIconForRemoteApp(smallIconId, webApkPackage); notificationBuilder.setSmallIconForRemoteApp(
api.getSmallIconId(), webApkPackage);
}
boolean notificationPermissionEnabled = api.notificationPermissionEnabled(); boolean notificationPermissionEnabled = api.notificationPermissionEnabled();
if (notificationPermissionEnabled) { if (notificationPermissionEnabled) {
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.browserservices; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.browserservices;
import android.content.ComponentName; import android.content.ComponentName;
import android.content.Context; import android.content.Context;
import android.content.Intent; import android.content.Intent;
import android.graphics.BitmapFactory;
import android.net.Uri; import android.net.Uri;
import android.os.Handler; import android.os.Handler;
import android.os.Looper; import android.os.Looper;
...@@ -161,6 +162,8 @@ public class TrustedWebActivityClientTest { ...@@ -161,6 +162,8 @@ public class TrustedWebActivityClientTest {
mResponseHandler.mGetSmallIconId.waitForCallback(0); mResponseHandler.mGetSmallIconId.waitForCallback(0);
mResponseHandler.mNotifyNotification.waitForCallback(0); mResponseHandler.mNotifyNotification.waitForCallback(0);
Assert.assertEquals(1, mResponseHandler.mGetSmallIconId.getCallCount());
Assert.assertEquals(mResponseHandler.mNotificationTag, NOTIFICATION_TAG); Assert.assertEquals(mResponseHandler.mNotificationTag, NOTIFICATION_TAG);
Assert.assertEquals(mResponseHandler.mNotificationId, NOTIFICATION_ID); Assert.assertEquals(mResponseHandler.mNotificationId, NOTIFICATION_ID);
Assert.assertEquals(mResponseHandler.mNotificationChannel, Assert.assertEquals(mResponseHandler.mNotificationChannel,
...@@ -168,6 +171,29 @@ public class TrustedWebActivityClientTest { ...@@ -168,6 +171,29 @@ public class TrustedWebActivityClientTest {
R.string.notification_category_group_general)); R.string.notification_category_group_general));
} }
/**
* Tests that #notifyNotification does not overwrite the small icon if a bitmap
* was already provided.
*/
@Test
@SmallTest
public void testSmallIconNotOverwritten() throws TimeoutException, InterruptedException {
Assert.assertEquals(0, mResponseHandler.mGetSmallIconId.getCallCount());
Assert.assertEquals(0, mResponseHandler.mNotifyNotification.getCallCount());
ThreadUtils.runOnUiThread(() -> {
NotificationBuilderBase builder = new StandardNotificationBuilder(mTargetContext);
// Set a custom small icon.
builder.setSmallIcon(BitmapFactory.decodeResource(
mTargetContext.getResources(), R.drawable.ic_chrome));
mClient.notifyNotification(SCOPE, NOTIFICATION_TAG, NOTIFICATION_ID, builder);
});
mResponseHandler.mNotifyNotification.waitForCallback(0);
Assert.assertEquals(0, mResponseHandler.mGetSmallIconId.getCallCount());
}
/** /**
* Tests that #cancelNotification gets the service to cancel the notification, using the given * Tests that #cancelNotification gets the service to cancel the notification, using the given
* id and tag. * id and tag.
......
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