Commit da809851 authored by Peter Kotwicz's avatar Peter Kotwicz Committed by Commit Bot

[Android Notifications Refactor]: Remove trusted remote app functions from NotificationBuilderBase

This CL moves the untrusted remote app bitmap decoding from NotificationBuilderBase to
WebApkServiceClient. This enables removing:
NotificationBuilderBase#setStatusBarIconForTrustedRemoteApp() and
NotificationBuilderBase#setContentSmallIconForTrustedRemoteApp()

BUG=None
TBR=peter@,peconn@,dominickn@,pshmakov@ (Looks like presubmit OWNERS check is broken)

Change-Id: I15fbc3c286cd6bd276bc098f7eee961c71b2bb31
Reviewed-on: https://chromium-review.googlesource.com/c/1383520
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Reviewed-by: default avatarPavel Shmakov <pshmakov@chromium.org>
Reviewed-by: default avatarPeter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618844}
parent 88f5d8c5
...@@ -106,11 +106,11 @@ public class TrustedWebActivityClient { ...@@ -106,11 +106,11 @@ public class TrustedWebActivityClient {
Bitmap bitmap = service.getSmallIconBitmap(); Bitmap bitmap = service.getSmallIconBitmap();
if (!builder.hasStatusBarIconBitmap()) { if (!builder.hasStatusBarIconBitmap()) {
builder.setStatusBarIconForUntrustedRemoteApp(id, bitmap, builder.setStatusBarIconForRemoteApp(
service.getComponentName().getPackageName()); id, bitmap, service.getComponentName().getPackageName());
} }
if (!builder.hasSmallIconForContent()) { if (!builder.hasSmallIconForContent()) {
builder.setContentSmallIconForUntrustedRemoteApp(bitmap); builder.setContentSmallIconForRemoteApp(bitmap);
} }
} }
......
...@@ -10,10 +10,8 @@ import android.app.Notification; ...@@ -10,10 +10,8 @@ import android.app.Notification;
import android.app.PendingIntent; import android.app.PendingIntent;
import android.app.RemoteInput; import android.app.RemoteInput;
import android.content.Context; import android.content.Context;
import android.content.pm.PackageManager;
import android.content.res.Resources; import android.content.res.Resources;
import android.graphics.Bitmap; import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.graphics.Canvas; import android.graphics.Canvas;
import android.graphics.Color; import android.graphics.Color;
import android.graphics.Paint; import android.graphics.Paint;
...@@ -24,7 +22,6 @@ import android.os.Build; ...@@ -24,7 +22,6 @@ import android.os.Build;
import android.support.annotation.IntDef; import android.support.annotation.IntDef;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import org.chromium.base.ContextUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.widget.RoundedIconGenerator; import org.chromium.chrome.browser.widget.RoundedIconGenerator;
...@@ -248,32 +245,13 @@ public abstract class NotificationBuilderBase { ...@@ -248,32 +245,13 @@ public abstract class NotificationBuilderBase {
/** /**
* Sets the status bar icon for a notification that will be displayed by a different app. * Sets the status bar icon for a notification that will be displayed by a different app.
* The icon must come from a trusted app because this involves decoding a Bitmap from its * This is safe to use for any app.
* resources.
* @param iconId An iconId for a resource in the package that will display the notification.
* @param packageName The package name of the package that will display the notification.
*/
public NotificationBuilderBase setStatusBarIconForTrustedRemoteApp(
int iconId, String packageName) {
setStatusBarIconForRemoteApp(iconId, decodeImageResource(packageName, iconId), packageName);
return this;
}
/**
* Sets the status bar icon for a notification that will be displayed by a different app.
* Unlike {@link #setStatusBarIconForTrustedRemoteApp} this is safe to use for any app.
* @param iconId An iconId for a resource in the package that will display the notification. * @param iconId An iconId for a resource in the package that will display the notification.
* @param iconBitmap The decoded bitmap. Depending on the device we need either id or bitmap. * @param iconBitmap The decoded bitmap. Depending on the device we need either id or bitmap.
* @param packageName The package name of the package that will display the notification. * @param packageName The package name of the package that will display the notification.
*/ */
public NotificationBuilderBase setStatusBarIconForUntrustedRemoteApp( public NotificationBuilderBase setStatusBarIconForRemoteApp(
int iconId, @Nullable Bitmap iconBitmap, String packageName) { int iconId, @Nullable Bitmap iconBitmap, String packageName) {
setStatusBarIconForRemoteApp(iconId, iconBitmap, packageName);
return this;
}
private void setStatusBarIconForRemoteApp(int iconId, @Nullable Bitmap iconBitmap,
String packageName) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
// On Android M+, the small icon has to be from the resources of the app whose context // On Android M+, the small icon has to be from the resources of the app whose context
// is passed to the Notification.Builder constructor. Thus we can't use iconId directly, // is passed to the Notification.Builder constructor. Thus we can't use iconId directly,
...@@ -292,6 +270,7 @@ public abstract class NotificationBuilderBase { ...@@ -292,6 +270,7 @@ public abstract class NotificationBuilderBase {
// NotificationManager is used in NotificationManager#notify. // NotificationManager is used in NotificationManager#notify.
setSmallIconId(iconId); setSmallIconId(iconId);
} }
return this;
} }
private static boolean usingRemoteAppContextAllowed() { private static boolean usingRemoteAppContextAllowed() {
...@@ -301,20 +280,9 @@ public abstract class NotificationBuilderBase { ...@@ -301,20 +280,9 @@ public abstract class NotificationBuilderBase {
/** /**
* Sets the small icon to be shown inside a notification that will be displayed by a different * Sets the small icon to be shown inside a notification that will be displayed by a different
* app. The icon must come from a trusted app. * app. This is safe to use for any app.
*/ */
public NotificationBuilderBase setContentSmallIconForTrustedRemoteApp( public NotificationBuilderBase setContentSmallIconForRemoteApp(@Nullable Bitmap bitmap) {
int iconId, String packageName) {
setSmallIconForContent(decodeImageResource(packageName, iconId));
return this;
}
/**
* Sets the small icon to be shown inside a notification that will be displayed by a different
* app. Unlike {@link #setContentSmallIconForTrustedRemoteApp} this is safe to use for any app.
*/
public NotificationBuilderBase setContentSmallIconForUntrustedRemoteApp(
@Nullable Bitmap bitmap) {
setSmallIconForContent(bitmap); setSmallIconForContent(bitmap);
return this; return this;
} }
...@@ -646,16 +614,4 @@ public abstract class NotificationBuilderBase { ...@@ -646,16 +614,4 @@ public abstract class NotificationBuilderBase {
return new RoundedIconGenerator(largeIconWidthPx, largeIconHeightPx, cornerRadiusPx, return new RoundedIconGenerator(largeIconWidthPx, largeIconHeightPx, cornerRadiusPx,
NOTIFICATION_ICON_BG_COLOR, NOTIFICATION_ICON_TEXT_SIZE_DP * density); NOTIFICATION_ICON_BG_COLOR, NOTIFICATION_ICON_TEXT_SIZE_DP * density);
} }
/** Decodes into a Bitmap an Image resource stored in another package. */
@Nullable
private static Bitmap decodeImageResource(String otherPackage, int resourceId) {
PackageManager packageManager = ContextUtils.getApplicationContext().getPackageManager();
try {
Resources resources = packageManager.getResourcesForApplication(otherPackage);
return BitmapFactory.decodeResource(resources, resourceId);
} catch (PackageManager.NameNotFoundException e) {
return null;
}
}
} }
...@@ -6,9 +6,13 @@ package org.chromium.chrome.browser.webapps; ...@@ -6,9 +6,13 @@ package org.chromium.chrome.browser.webapps;
import android.content.pm.ApplicationInfo; import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager; import android.content.pm.PackageManager;
import android.content.res.Resources;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
import android.os.Build; import android.os.Build;
import android.os.IBinder; import android.os.IBinder;
import android.os.RemoteException; import android.os.RemoteException;
import android.support.annotation.Nullable;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
...@@ -104,11 +108,12 @@ public class WebApkServiceClient { ...@@ -104,11 +108,12 @@ public class WebApkServiceClient {
private void fallbackToWebApkIconIfNecessary(NotificationBuilderBase builder, private void fallbackToWebApkIconIfNecessary(NotificationBuilderBase builder,
String webApkPackage, int iconId) { String webApkPackage, int iconId) {
Bitmap icon = decodeImageResourceFromPackage(webApkPackage, iconId);
if (!builder.hasSmallIconForContent()) { if (!builder.hasSmallIconForContent()) {
builder.setContentSmallIconForTrustedRemoteApp(iconId, webApkPackage); builder.setContentSmallIconForRemoteApp(icon);
} }
if (!builder.hasStatusBarIconBitmap()) { if (!builder.hasStatusBarIconBitmap()) {
builder.setStatusBarIconForTrustedRemoteApp(iconId, webApkPackage); builder.setStatusBarIconForRemoteApp(iconId, icon, webApkPackage);
} }
} }
...@@ -145,4 +150,16 @@ public class WebApkServiceClient { ...@@ -145,4 +150,16 @@ public class WebApkServiceClient {
return false; return false;
} }
/** Decodes into a Bitmap an Image resource stored in an APK with the given package name. */
@Nullable
private static Bitmap decodeImageResourceFromPackage(String packageName, int resourceId) {
PackageManager packageManager = ContextUtils.getApplicationContext().getPackageManager();
try {
Resources resources = packageManager.getResourcesForApplication(packageName);
return BitmapFactory.decodeResource(resources, resourceId);
} catch (PackageManager.NameNotFoundException e) {
return null;
}
}
} }
...@@ -79,8 +79,9 @@ public class TrustedWebActivityClientTest { ...@@ -79,8 +79,9 @@ public class TrustedWebActivityClientTest {
public void usesIconFromService_IfStatusBarIconNotSet() { public void usesIconFromService_IfStatusBarIconNotSet() {
setHasStatusBarBitmap(false); setHasStatusBarBitmap(false);
postNotification(); postNotification();
verify(mNotificationBuilder).setStatusBarIconForUntrustedRemoteApp( verify(mNotificationBuilder)
SERVICE_SMALL_ICON_ID, mServiceSmallIconBitmap, CLIENT_PACKAGE_NAME); .setStatusBarIconForRemoteApp(
SERVICE_SMALL_ICON_ID, mServiceSmallIconBitmap, CLIENT_PACKAGE_NAME);
} }
...@@ -89,22 +90,21 @@ public class TrustedWebActivityClientTest { ...@@ -89,22 +90,21 @@ public class TrustedWebActivityClientTest {
setHasStatusBarBitmap(true); setHasStatusBarBitmap(true);
postNotification(); postNotification();
verify(mNotificationBuilder, never()) verify(mNotificationBuilder, never())
.setStatusBarIconForUntrustedRemoteApp(anyInt(), any(), anyString()); .setStatusBarIconForRemoteApp(anyInt(), any(), anyString());
} }
@Test @Test
public void usesIconFromService_IfContentSmallIconNotSet() { public void usesIconFromService_IfContentSmallIconNotSet() {
setHasContentBitmap(false); setHasContentBitmap(false);
postNotification(); postNotification();
verify(mNotificationBuilder) verify(mNotificationBuilder).setContentSmallIconForRemoteApp(mServiceSmallIconBitmap);
.setContentSmallIconForUntrustedRemoteApp(mServiceSmallIconBitmap);
} }
@Test @Test
public void doesntUseIconFromService_IfContentSmallIconSet() { public void doesntUseIconFromService_IfContentSmallIconSet() {
setHasContentBitmap(true); setHasContentBitmap(true);
postNotification(); postNotification();
verify(mNotificationBuilder, never()).setContentSmallIconForUntrustedRemoteApp(any()); verify(mNotificationBuilder, never()).setContentSmallIconForRemoteApp(any());
} }
......
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