Use Context#createPackageContext to work around Samsung's issue with small icons

The issue is related to the crashes http://crbug/829367 on Samsung (also, Lenovo
and Yulong) Marshmallow devices. Because of those crashes we prohibit setting
small notification icons as Bitmaps.

The icon coming from a web page shown in a TWA comes in a form of a Bitmap, so
we can't use that. A reasonable fallback would be to use the icon specified by
its resource id in meta-data for TWAService or by extending TWAService.

Unfortunately, there is a problem with that fallback as well. For M+ the small
icon has to be from the resources of the app whose context is passed to the
Notification.Builder constructor. Normally we would use the bitmap decoded (on
the client's side) from resource id. But then again, on Samsung M we can't set
that bitmap due to the crashes.

In this CL we create the context of client's app using Context#createForPackage,
pass that into NotificationBuilder, and that way we can use the icon id.

Since this approach is quite unusual and seems risky, it is used only in the
cases we're fixing, and protected by a feature flag.

Bug: 864786
Change-Id: Ia401f56b446e7dcc684697faea77ae2a3a402e67
Reviewed-on: https://chromium-review.googlesource.com/c/1228075
Commit-Queue: Pavel Shmakov <pshmakov@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608516}

chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java

@@ -147,6 +147,8 @@ public abstract class ChromeFeatureList {
     }
 
     // Alphabetical:
+    public static final String ALLOW_REMOTE_CONTEXT_FOR_NOTIFICATIONS =
+            "AllowRemoteContextForNotifications";
     public static final String AUTOFILL_ALLOW_NON_HTTP_ACTIVATION =
             "AutofillAllowNonHttpActivation";
     public static final String ADJUST_WEBAPK_INSTALLATION_SPACE = "AdjustWebApkInstallationSpace";

chrome/android/java/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityClient.java

@@ -93,7 +93,8 @@ public class TrustedWebActivityClient {
 
         Bitmap bitmap = service.getSmallIconBitmap();
         if (!builder.hasStatusBarIconBitmap()) {
-            builder.setStatusBarIconForUntrustedRemoteApp(id, bitmap);
+            builder.setStatusBarIconForUntrustedRemoteApp(id, bitmap,
+                    service.getComponentName().getPackageName());
         }
         if (!builder.hasSmallIconForContent()) {
             builder.setContentSmallIconForUntrustedRemoteApp(bitmap);

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

@@ -142,7 +142,7 @@ public class CustomNotificationBuilder extends NotificationBuilderBase {
         // TODO(crbug.com/697104) We should probably use a Compat builder.
         ChromeNotificationBuilder builder =
                 NotificationBuilderFactory.createChromeNotificationBuilder(
-                        false /* preferCompat */, mChannelId);
+                        false /* preferCompat */, mChannelId, mRemotePackageForBuilderContext);
         builder.setTicker(mTickerText);
         builder.setContentIntent(mContentIntent);
         builder.setDeleteIntent(mDeleteIntent);

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

@@ -25,6 +25,7 @@ import android.support.annotation.IntDef;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.widget.RoundedIconGenerator;
 
 import java.lang.annotation.Retention;
@@ -126,6 +127,13 @@ public abstract class NotificationBuilderBase {
     @Nullable protected Bitmap mSmallIconBitmapForStatusBar;
     @Nullable protected Bitmap mSmallIconBitmapForContent;
 
+    /**
+     * Package name to use for creating remote package context to be passed to NotificationBuilder.
+     * If null, Chrome's context is used. Currently only used as a workaround for a certain issue,
+     * see {@link #setStatusBarIconForRemoteApp}, {@link #deviceSupportsBitmapStatusBarIcons}.
+     */
+    @Nullable protected String mRemotePackageForBuilderContext;
+
     protected PendingIntent mContentIntent;
     protected PendingIntent mDeleteIntent;
     protected List<Action> mActions = new ArrayList<>(MAX_AUTHOR_PROVIDED_ACTION_BUTTONS);
@@ -248,7 +256,7 @@ public abstract class NotificationBuilderBase {
      */
     public NotificationBuilderBase setStatusBarIconForTrustedRemoteApp(
             int iconId, String packageName) {
-        setStatusBarIconForRemoteApp(iconId, decodeImageResource(packageName, iconId));
+        setStatusBarIconForRemoteApp(iconId, decodeImageResource(packageName, iconId), packageName);
         return this;
     }
 
@@ -257,19 +265,29 @@ public abstract class NotificationBuilderBase {
      * 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 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.
      */
     public NotificationBuilderBase setStatusBarIconForUntrustedRemoteApp(
-            int iconId, @Nullable Bitmap iconBitmap) {
-        setStatusBarIconForRemoteApp(iconId, iconBitmap);
+            int iconId, @Nullable Bitmap iconBitmap, String packageName) {
+        setStatusBarIconForRemoteApp(iconId, iconBitmap, packageName);
         return this;
     }
 
-    private void setStatusBarIconForRemoteApp(int iconId, @Nullable Bitmap iconBitmap) {
+    private void setStatusBarIconForRemoteApp(int iconId, @Nullable Bitmap iconBitmap,
+            String packageName) {
         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
             // is passed to the Notification.Builder constructor. Thus we can't use iconId directly,
-            // and instead decode the image and set the icon as a Bitmap.
-            setStatusBarIcon(iconBitmap);
+            // and instead use the decoded Bitmap.
+            if (deviceSupportsBitmapStatusBarIcons()) {
+                setStatusBarIcon(iconBitmap);
+            } else if (usingRemoteAppContextAllowed()) {
+                // For blacklisted M devices we can use neither iconId (see comment below), nor
+                // iconBitmap, because that leads to crashes. Here we attempt to work around that by
+                // using remote app context: with that context iconId can be used.
+                mRemotePackageForBuilderContext = packageName;
+                setSmallIconId(iconId);
+            }  // else we're out of luck.
         } else {
             // Pre Android M, the small icon has to be from the resources of the app whose
             // NotificationManager is used in NotificationManager#notify.
@@ -277,6 +295,11 @@ public abstract class NotificationBuilderBase {
         }
     }
 
+    private static boolean usingRemoteAppContextAllowed() {
+        return ChromeFeatureList.isEnabled(
+                ChromeFeatureList.ALLOW_REMOTE_CONTEXT_FOR_NOTIFICATIONS);
+    }
+
     /**
      * 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.

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

@@ -4,11 +4,17 @@
 
 package org.chromium.chrome.browser.notifications;
 
+import static org.chromium.chrome.browser.ChromeFeatureList.ALLOW_REMOTE_CONTEXT_FOR_NOTIFICATIONS;
+
 import android.content.Context;
+import android.content.pm.PackageManager;
 
 import org.chromium.base.ContextUtils;
+import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.notifications.channels.ChannelsInitializer;
 
+import javax.annotation.Nullable;
+
 /**
  * Factory which supplies the appropriate type of notification builder based on Android version.
  * Should be used for all notifications we create, to ensure a notification channel is set on O.
@@ -27,7 +33,26 @@ public class NotificationBuilderFactory {
      */
     public static ChromeNotificationBuilder createChromeNotificationBuilder(
             boolean preferCompat, String channelId) {
+        return createChromeNotificationBuilder(preferCompat, channelId, null);
+    }
+
+    /**
+     * Same as above, with additional parameter:
+     * @param remoteAppPackageName if not null, tries to create a Context from the package name
+     * and passes it to the builder.
+     */
+    public static ChromeNotificationBuilder createChromeNotificationBuilder(
+            boolean preferCompat, String channelId, @Nullable String remoteAppPackageName) {
         Context context = ContextUtils.getApplicationContext();
+        if (remoteAppPackageName != null) {
+            assert ChromeFeatureList.isEnabled(ALLOW_REMOTE_CONTEXT_FOR_NOTIFICATIONS);
+            try {
+                context = context.createPackageContext(remoteAppPackageName, 0);
+            } catch (PackageManager.NameNotFoundException e) {
+                throw new RuntimeException("Failed to create context for package "
+                        + remoteAppPackageName, e);
+            }
+        }
 
         NotificationManagerProxyImpl notificationManagerProxy =
                 new NotificationManagerProxyImpl(context);

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

@@ -26,7 +26,7 @@ public class StandardNotificationBuilder extends NotificationBuilderBase {
         // TODO(crbug.com/697104) We should probably use a Compat builder.
         ChromeNotificationBuilder builder =
                 NotificationBuilderFactory.createChromeNotificationBuilder(
-                        false /* preferCompat */, mChannelId);
+                        false /* preferCompat */, mChannelId, mRemotePackageForBuilderContext);
 
         builder.setContentTitle(mTitle);
         builder.setContentText(mBody);

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityClientTest.java

@@ -11,6 +11,7 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import android.content.ComponentName;
 import android.graphics.Bitmap;
 import android.net.Uri;
 import android.os.RemoteException;
@@ -37,6 +38,7 @@ import org.chromium.chrome.browser.notifications.NotificationBuilderBase;
 public class TrustedWebActivityClientTest {
 
     private static final int SERVICE_SMALL_ICON_ID = 1;
+    private static final String CLIENT_PACKAGE_NAME = "com.example.app";
 
     @Mock
     private TrustedWebActivityServiceConnectionManager mConnection;
@@ -63,6 +65,7 @@ public class TrustedWebActivityClientTest {
 
         when(mService.getSmallIconId()).thenReturn(SERVICE_SMALL_ICON_ID);
         when(mService.getSmallIconBitmap()).thenReturn(mServiceSmallIconBitmap);
+        when(mService.getComponentName()).thenReturn(new ComponentName(CLIENT_PACKAGE_NAME, ""));
 
         mClient = new TrustedWebActivityClient(mConnection);
     }
@@ -72,7 +75,7 @@ public class TrustedWebActivityClientTest {
         setHasStatusBarBitmap(false);
         postNotification();
         verify(mNotificationBuilder).setStatusBarIconForUntrustedRemoteApp(
-                SERVICE_SMALL_ICON_ID, mServiceSmallIconBitmap);
+                SERVICE_SMALL_ICON_ID, mServiceSmallIconBitmap, CLIENT_PACKAGE_NAME);
     }
 
 
@@ -81,7 +84,7 @@ public class TrustedWebActivityClientTest {
         setHasStatusBarBitmap(true);
         postNotification();
         verify(mNotificationBuilder, never())
-                .setStatusBarIconForUntrustedRemoteApp(anyInt(), any());
+                .setStatusBarIconForUntrustedRemoteApp(anyInt(), any(), anyString());
     }
 
     @Test

chrome/browser/about_flags.cc

@@ -4520,6 +4520,14 @@ const FeatureEntry kFeatureEntries[] = {
      flag_descriptions::kEnableDiscoverAppDescription, kOsCrOS,
      FEATURE_VALUE_TYPE(chromeos::features::kDiscoverApp)},
 #endif  // defined(OS_CHROMEOS)
+
+#if defined(OS_ANDROID)
+    {"allow-remote-context-for-notifications",
+     flag_descriptions::kAllowRemoteContextForNotificationsName,
+     flag_descriptions::kAllowRemoteContextForNotificationsDescription,
+     kOsAndroid,
+     FEATURE_VALUE_TYPE(chrome::android::kAllowRemoteContextForNotifications)},
+#endif  // defined(OS_ANDROID)
 };
 
 class FlagsStateSingleton {

chrome/browser/android/chrome_feature_list.cc

@@ -74,6 +74,7 @@ const base::Feature* kFeaturesExposedToJava[] = {
     &feed::kInterestFeedContentSuggestions,
     &invalidation::switches::kFCMInvalidations,
     &kAdjustWebApkInstallationSpace,
+    &kAllowRemoteContextForNotifications,
     &kAndroidPayIntegrationV1,
     &kAndroidPayIntegrationV2,
     &kAndroidPaymentApps,
@@ -198,6 +199,9 @@ const base::Feature kAdjustWebApkInstallationSpace = {
 const base::Feature kAndroidPayIntegrationV1{"AndroidPayIntegrationV1",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
+const base::Feature kAllowRemoteContextForNotifications{
+    "AllowRemoteContextForNotifications", base::FEATURE_ENABLED_BY_DEFAULT};
+
 const base::Feature kAndroidPayIntegrationV2{"AndroidPayIntegrationV2",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 

chrome/browser/android/chrome_feature_list.h

@@ -13,6 +13,7 @@ namespace android {
 
 // Alphabetical:
 extern const base::Feature kAdjustWebApkInstallationSpace;
+extern const base::Feature kAllowRemoteContextForNotifications;
 extern const base::Feature kAndroidPayIntegrationV1;
 extern const base::Feature kAndroidPayIntegrationV2;
 extern const base::Feature kAndroidPaymentApps;

chrome/browser/flag_descriptions.cc

@@ -2236,6 +2236,12 @@ const char kAccessibilityTabSwitcherName[] = "Accessibility Tab Switcher";
 const char kAccessibilityTabSwitcherDescription[] =
     "Enable the accessibility tab switcher for Android.";
 
+const char kAllowRemoteContextForNotificationsName[] =
+    "Allow using remote app context for notifications";
+const char kAllowRemoteContextForNotificationsDescription[] =
+    "Allow using Context#createPackageContext to work around issues with status"
+    "bar icons on certain Android M devices.";
+
 const char kAndroidAutofillAccessibilityName[] = "Autofill Accessibility";
 const char kAndroidAutofillAccessibilityDescription[] =
     "Enable accessibility for autofill popup.";

chrome/browser/flag_descriptions.h

@@ -1344,6 +1344,9 @@ extern const char kAiaFetchingDescription[];
 extern const char kAccessibilityTabSwitcherName[];
 extern const char kAccessibilityTabSwitcherDescription[];
 
+extern const char kAllowRemoteContextForNotificationsName[];
+extern const char kAllowRemoteContextForNotificationsDescription[];
+
 extern const char kAndroidAutofillAccessibilityName[];
 extern const char kAndroidAutofillAccessibilityDescription[];
 

tools/metrics/histograms/enums.xml

@@ -29449,6 +29449,7 @@ from previous Chrome versions.
   <int value="-1438279809" label="GamepadExtensions:disabled"/>
   <int value="-1436251034" label="VoiceSearchOnLocalNtp:disabled"/>
   <int value="-1433719718" label="enable-webrtc-stun-origin"/>
+  <int value="-1433452630" label="AllowRemoteContextForNotifications:enabled"/>
   <int value="-1433087548" label="enable-app-install-alerts"/>
   <int value="-1431563697" label="WebPaymentsMethodSectionOrderV2:enabled"/>
   <int value="-1426817842" label="BlockTabUnders:enabled"/>
@@ -31388,6 +31389,7 @@ from previous Chrome versions.
   <int value="2139048614" label="UseSurfaceLayerForVideo:enabled"/>
   <int value="2141067485" label="SystemWebApps:enabled"/>
   <int value="2141463681" label="enable-offer-upload-credit-cards"/>
+  <int value="2142661816" label="AllowRemoteContextForNotifications:disabled"/>
   <int value="2142979536" label="EnableManualFallbacksFilling:disabled"/>
 </enum>