[WebLayer] Fix crash when other shared resource library is loaded

This changes the hardcoded package ID from 0x02 to 0x05, which allows
for other shared libraries to be loaded before WebLayer.

This change calls addAssetPath on the AssetManager to increment the
package ID until we hit our magic 0x05. Each call needs to use a
different path and the path has to be a valid asset path, so we prepend
a string of "/." to the WebView package's source dir.

This also fixes crbug.com/1075704 by hardcoding the package IDs of the
locale resources to 0x05, which will load them at the same ID the main
APK will be loaded. This also prevents each split from taking a new
dynamic ID.

Bug: 1075704, 1075789
Change-Id: I671d0ad64a3025409a62ecebbdbc584819d68ad5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2174964Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#765318}

build/android/gyp/compile_resources.py

@@ -69,8 +69,9 @@ _PNG_WEBP_EXCLUSION_PATTERN = re.compile('|'.join([
 ]))
 
 # The package ID hardcoded for shared libraries. See
-# _HardcodeSharedLibraryDynamicAttributes() for more details.
-_SHARED_LIBRARY_HARDCODED_ID = 2
+# _HardcodeSharedLibraryDynamicAttributes() for more details. If this value
+# changes make sure to change REQUIRED_PACKAGE_IDENTIFIER in WebLayerImpl.java.
+_SHARED_LIBRARY_HARDCODED_ID = 12
 
 
 def _ParseArgs(args):
@@ -749,10 +750,58 @@ def _ProcessProtoXmlNode(xml_node):
     _ProcessProtoXmlNode(child)
 
 
-def _HardcodeSharedLibraryDynamicAttributes(zip_path):
-  """Hardcodes the package IDs of dynamic attributes to 0x02.
+def _SplitLocaleResourceType(_type, allowed_resource_names):
+  """Splits locale specific resources out of |_type| and returns them.
 
-  This is a workaround for b/147674078, which affects Android versions pre-N.
+  Any locale specific resources will be removed from |_type|, and a new
+  Resources_pb2.Type value will be returned which contains those resources.
+
+  Args:
+    _type: A Resources_pb2.Type value
+    allowed_resource_names: Names of locale resources that should be kept in the
+        main type.
+  """
+  locale_entries = []
+  for entry in _type.entry:
+    if entry.name in allowed_resource_names:
+      continue
+
+    # First collect all resources values with a locale set.
+    config_values_with_locale = []
+    for config_value in entry.config_value:
+      if config_value.config.locale:
+        config_values_with_locale.append(config_value)
+
+    if config_values_with_locale:
+      # Remove the locale resources from the original entry
+      for value in config_values_with_locale:
+        entry.config_value.remove(value)
+
+      # Add locale resources to a new Entry, and save for later.
+      locale_entry = Resources_pb2.Entry()
+      locale_entry.CopyFrom(entry)
+      del locale_entry.config_value[:]
+      locale_entry.config_value.extend(config_values_with_locale)
+      locale_entries.append(locale_entry)
+
+  if not locale_entries:
+    return None
+
+  # Copy the original type and replace the entries with |locale_entries|.
+  locale_type = Resources_pb2.Type()
+  locale_type.CopyFrom(_type)
+  del locale_type.entry[:]
+  locale_type.entry.extend(locale_entries)
+  return locale_type
+
+
+def _HardcodeSharedLibraryDynamicAttributes(zip_path, options):
+  """Hardcodes the package IDs of dynamic attributes and locale resources.
+
+  Hardcoding dynamic attribute package IDs is a workaround for b/147674078,
+  which affects Android versions pre-N. Hardcoding locale resource package IDs
+  is a workaround for b/155437035, which affects resources built with
+  --shared-lib on all Android versions
 
   Args:
     zip_path: Path to proto APK file.
@@ -765,12 +814,37 @@ def _HardcodeSharedLibraryDynamicAttributes(zip_path):
     with open(os.path.join(tmp_dir, 'resources.pb')) as f:
       table.ParseFromString(f.read())
 
+    # A separate top level package will be added to the resources, which
+    # contains only locale specific resources. The package ID of the locale
+    # resources is hardcoded to _SHARED_LIBRARY_HARDCODED_ID. This causes
+    # resources in locale splits to all get assigned
+    # _SHARED_LIBRARY_HARDCODED_ID as their package ID, which prevents a bug in
+    # shared library bundles where each split APK gets a separate dynamic ID,
+    # and cannot be accessed by the main APK.
+    translations_package = Resources_pb2.Package()
+    translations_package.package_id.id = _SHARED_LIBRARY_HARDCODED_ID
+    translations_package.package_name = table.package[
+        0].package_name + '_translations'
+
+    # These resources are allowed in the base resources, since they are needed
+    # by WebView.
+    allowed_resource_names = set()
+    if options.shared_resources_allowlist:
+      allowed_resource_names = set(
+          resource_utils.GetRTxtStringResourceNames(
+              options.shared_resources_allowlist))
     for package in table.package:
       for _type in package.type:
         for entry in _type.entry:
           for config_value in entry.config_value:
             _ProcessProtoValue(config_value.value)
 
+        locale_type = _SplitLocaleResourceType(_type, allowed_resource_names)
+        if locale_type:
+          translations_package.type.add().CopyFrom(locale_type)
+
+    table.package.add().CopyFrom(translations_package)
+
     with open(os.path.join(tmp_dir, 'resources.pb'), 'w') as f:
       f.write(table.SerializeToString())
 
@@ -1015,7 +1089,7 @@ def _PackageApk(options, build):
   # affect WebView usage, since WebView does not used dynamic attributes.
   if options.shared_resources:
     logging.debug('Hardcoding dynamic attributes')
-    _HardcodeSharedLibraryDynamicAttributes(build.proto_path)
+    _HardcodeSharedLibraryDynamicAttributes(build.proto_path, options)
     build_utils.CheckOutput([
         options.aapt2_path, 'convert', '--output-format', 'binary', '-o',
         build.arsc_path, build.proto_path

chrome/android/java/src/org/chromium/chrome/browser/media/MediaCaptureNotificationService.java

@@ -186,7 +186,7 @@ public class MediaCaptureNotificationService extends Service {
                 : null;
         ChromeNotification notification = MediaCaptureNotificationUtil.createNotification(builder,
                 mediaType, url, appContext.getString(R.string.app_name), isIncognito, contentIntent,
-                stopIntent, null /*resPackageName*/);
+                stopIntent);
 
         mNotificationManager.notify(notification);
         mNotifications.put(notificationId, mediaType);

components/webrtc/android/java/src/org/chromium/components/webrtc/MediaCaptureNotificationUtil.java

@@ -6,7 +6,6 @@ package org.chromium.components.webrtc;
 
 import android.app.PendingIntent;
 import android.content.Context;
-import android.graphics.drawable.Icon;
 import android.os.Build;
 
 import androidx.annotation.IntDef;
@@ -40,27 +39,16 @@ public class MediaCaptureNotificationUtil {
      * @param isIncognito whether the notification is for an off-the-record context.
      * @param contentIntent the intent to be sent when the notification is clicked.
      * @param stopIntent if non-null, a stop button that triggers this intent will be added.
-     * @param resPackageName if non-null, the name of the package that contains the drawable.
      */
     public static ChromeNotification createNotification(ChromeNotificationBuilder builder,
             @MediaType int mediaType, String url, @Nullable String appName, boolean isIncognito,
-            @Nullable PendingIntentProvider contentIntent, @Nullable PendingIntent stopIntent,
-            @Nullable String resPackageName) {
+            @Nullable PendingIntentProvider contentIntent, @Nullable PendingIntent stopIntent) {
         Context appContext = ContextUtils.getApplicationContext();
-        builder.setAutoCancel(false).setOngoing(true).setLocalOnly(true).setContentIntent(
-                contentIntent);
-
-        if (resPackageName != null) {
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-                builder.setSmallIcon(
-                        Icon.createWithResource(resPackageName, getNotificationIconId(mediaType)));
-            } else {
-                // Some fallback is required, or the notification won't appear.
-                builder.setSmallIcon(android.R.drawable.radiobutton_on_background);
-            }
-        } else {
-            builder.setSmallIcon(getNotificationIconId(mediaType));
-        }
+        builder.setAutoCancel(false)
+                .setOngoing(true)
+                .setLocalOnly(true)
+                .setContentIntent(contentIntent)
+                .setSmallIcon(getNotificationIconId(mediaType));
 
         if (stopIntent != null) {
             builder.setPriorityBeforeO(NotificationCompat.PRIORITY_HIGH);

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/MediaCaptureTest.java

@@ -10,6 +10,7 @@ import android.app.NotificationManager;
 import android.content.Context;
 import android.os.Build;
 import android.service.notification.StatusBarNotification;
+import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 import android.webkit.ValueCallback;
 
@@ -238,6 +239,8 @@ public final class MediaCaptureTest {
             if (statusBarNotification.getTag().equals("org.chromium.weblayer.webrtc.avstream")) {
                 Assert.assertNull(notification);
                 notification = statusBarNotification.getNotification();
+                Assert.assertNotNull(notification.getSmallIcon().loadDrawable(
+                        InstrumentationRegistry.getContext()));
             }
         }
         return notification;

weblayer/browser/android/javatests/src/org/chromium/weblayer/test/ResourceLoadingTest.java

@@ -59,7 +59,11 @@ public final class ResourceLoadingTest {
     }
 
     private int getIdentifier(String name) {
-        return mRemoteContext.getResources().getIdentifier(
+        int id = mRemoteContext.getResources().getIdentifier(
                 name, null, mRemoteContext.getPackageName());
+        // Force the returned ID to use our magic package ID.
+        id &= 0x00ffffff;
+        id |= 0x0c000000;
+        return id;
     }
 }

weblayer/browser/java/BUILD.gn

@@ -91,6 +91,7 @@ android_library("java") {
     "org/chromium/weblayer_private/WebLayerExceptionFilter.java",
     "org/chromium/weblayer_private/WebLayerFactoryImpl.java",
     "org/chromium/weblayer_private/WebLayerImpl.java",
+    "org/chromium/weblayer_private/WebLayerNotificationBuilder.java",
     "org/chromium/weblayer_private/WebLayerNotificationChannels.java",
     "org/chromium/weblayer_private/WebLayerTabModalPresenter.java",
     "org/chromium/weblayer_private/WebViewCompatibilityHelperImpl.java",

weblayer/browser/java/org/chromium/weblayer_private/DownloadImpl.java

@@ -19,7 +19,6 @@ import org.chromium.base.Log;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
-import org.chromium.components.browser_ui.notifications.NotificationBuilder;
 import org.chromium.components.browser_ui.notifications.NotificationManagerProxy;
 import org.chromium.components.browser_ui.notifications.NotificationManagerProxyImpl;
 import org.chromium.components.browser_ui.notifications.NotificationMetadata;
@@ -345,8 +344,8 @@ public final class DownloadImpl extends IDownload.Stub {
                 ? WebLayerNotificationChannels.ChannelId.COMPLETED_DOWNLOADS
                 : WebLayerNotificationChannels.ChannelId.ACTIVE_DOWNLOADS;
 
-        NotificationBuilder builder =
-                new NotificationBuilder(context, channelId, channelsInitializer,
+        WebLayerNotificationBuilder builder =
+                new WebLayerNotificationBuilder(context, channelId, channelsInitializer,
                         new NotificationMetadata(0, NOTIFICATION_TAG, mNotificationId));
         builder.setOngoing(true)
                 .setDeleteIntent(deletePendingIntent)

weblayer/browser/java/org/chromium/weblayer_private/MediaStreamManager.java

@@ -12,13 +12,11 @@ import android.os.RemoteException;
 import android.util.AndroidRuntimeException;
 import android.webkit.ValueCallback;
 
-import org.chromium.base.BuildInfo;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
 import org.chromium.components.browser_ui.notifications.ChromeNotification;
-import org.chromium.components.browser_ui.notifications.NotificationBuilder;
 import org.chromium.components.browser_ui.notifications.NotificationManagerProxy;
 import org.chromium.components.browser_ui.notifications.NotificationManagerProxyImpl;
 import org.chromium.components.browser_ui.notifications.NotificationMetadata;
@@ -225,12 +223,12 @@ public class MediaStreamManager {
 
         // TODO(crbug/1076098): don't hard-code incognito to false.
         ChromeNotification notification = MediaCaptureNotificationUtil.createNotification(
-                new NotificationBuilder(appContext, WebLayerNotificationChannels.ChannelId.MEDIA,
-                        channelsInitializer,
+                new WebLayerNotificationBuilder(appContext,
+                        WebLayerNotificationChannels.ChannelId.MEDIA, channelsInitializer,
                         new NotificationMetadata(0, AV_STREAM_TAG, mNotificationId)),
                 mediaType, mTab.getWebContents().getVisibleUrl().getSpec(),
                 WebLayerImpl.getClientApplicationName(), false /*isIncognito*/, contentIntent,
-                null /*stopIntent*/, BuildInfo.getInstance().packageName);
+                null /*stopIntent*/);
         notificationManagerProxy.notify(notification);
 
         updateActiveNotifications(true);

weblayer/browser/java/org/chromium/weblayer_private/WebLayerImpl.java

@@ -87,6 +87,11 @@ public final class WebLayerImpl extends IWebLayer.Stub {
     public static final String PREF_LAST_VERSION_CODE =
             "org.chromium.weblayer.last_version_code_used";
 
+    // The required package ID for WebLayer when loaded as a shared library, hardcoded in the
+    // resources. If this value changes make sure to change _SHARED_LIBRARY_HARDCODED_ID in
+    // compile_resources.py.
+    private static final int REQUIRED_PACKAGE_IDENTIFIER = 12;
+
     private final ProfileManager mProfileManager = new ProfileManager();
 
     private boolean mInited;
@@ -205,15 +210,9 @@ public final class WebLayerImpl extends IWebLayer.Stub {
         }
 
         BuildInfo.setBrowserPackageInfo(packageInfo);
-        int resourcesPackageId = getPackageId(appContext, packageInfo.packageName);
-        if (resourcesPackageId < 0x7f && resourcesPackageId != 2) {
-            throw new AndroidRuntimeException(
-                    "WebLayer can't be used with another shared library. Loaded packages: "
-                    + getLoadedPackageNames(appContext));
-        }
         // TODO: The call to onResourcesLoaded() can be slow, we may need to parallelize this with
         // other expensive startup tasks.
-        R.onResourcesLoaded(resourcesPackageId);
+        R.onResourcesLoaded(forceCorrectPackageId(remoteContext));
         SelectionPopupController.setMustUseWebContentsContext();
 
         ResourceBundle.setAvailablePakLocales(new String[] {}, ProductConfig.UNCOMPRESSED_LOCALES);
@@ -365,6 +364,33 @@ public final class WebLayerImpl extends IWebLayer.Stub {
                 .toString();
     }
 
+    /**
+     * Converts the given id into a resource ID that can be shown in system UI, such as
+     * notifications.
+     */
+    public static int getResourceIdForSystemUi(int id) {
+        if (isAndroidResource(id)) {
+            return id;
+        }
+
+        Context context = ContextUtils.getApplicationContext();
+        // String may be missing translations, since they are loaded at a different package ID
+        // by default in standalone WebView.
+        assert !context.getResources().getResourceTypeName(id).equals("string");
+        id &= 0x00ffffff;
+        id |= (0x01000000
+                * getPackageId(context, WebViewFactory.getLoadedPackageInfo().packageName));
+        return id;
+    }
+
+    /** Returns whether this ID is from the android system package. */
+    public static boolean isAndroidResource(int id) {
+        return ContextUtils.getApplicationContext()
+                .getResources()
+                .getResourcePackageName(id)
+                .equals("android");
+    }
+
     /**
      * Performs the minimal initialization needed for a context. This is used for example in
      * CrashReporterControllerImpl, so it can be used before full WebLayer initialization.
@@ -387,6 +413,41 @@ public final class WebLayerImpl extends IWebLayer.Stub {
         return appContext;
     }
 
+    /** Forces the correct package ID or dies with a runtime exception. */
+    private static int forceCorrectPackageId(Context remoteContext) {
+        int packageId = getPackageId(remoteContext, remoteContext.getPackageName());
+        // This is using app_as_shared_lib, no change needed.
+        if (packageId >= 0x7f) {
+            return packageId;
+        }
+
+        if (packageId > REQUIRED_PACKAGE_IDENTIFIER) {
+            throw new AndroidRuntimeException(
+                    "WebLayer can't be used with other shared libraries. Loaded packages: "
+                    + getLoadedPackageNames(remoteContext));
+        }
+
+        forceAddAssetPaths(remoteContext, packageId);
+
+        return REQUIRED_PACKAGE_IDENTIFIER;
+    }
+
+    /** Forces adding entries to the package identifiers array until we hit the required ID. */
+    private static void forceAddAssetPaths(Context remoteContext, int packageId) {
+        try {
+            Method addAssetPath = AssetManager.class.getMethod("addAssetPath", String.class);
+            String path = remoteContext.getApplicationInfo().sourceDir;
+            // Add enough paths to make sure we reach the required ID.
+            for (int i = packageId; i < REQUIRED_PACKAGE_IDENTIFIER; i++) {
+                // Change the path to ensure the asset path is re-added and grabs a new package ID.
+                path = "/." + path;
+                addAssetPath.invoke(remoteContext.getAssets(), path);
+            }
+        } catch (ReflectiveOperationException e) {
+            throw new AndroidRuntimeException(e);
+        }
+    }
+
     /**
      * Returns the package ID to use when calling R.onResourcesLoaded().
      */
@@ -432,7 +493,9 @@ public final class WebLayerImpl extends IWebLayer.Stub {
                 if (key == 1) {
                     continue;
                 }
-                packageNames.add(name + ":" + key);
+
+                // Make sure this doesn't look like a URL so it doesn't get removed from crashes.
+                packageNames.add(name.replace(".", "_") + " -> " + key);
             }
             return TextUtils.join(",", packageNames);
         } catch (ReflectiveOperationException e) {

weblayer/browser/java/org/chromium/weblayer_private/WebLayerNotificationBuilder.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.weblayer_private;
+
+import android.content.Context;
+import android.graphics.drawable.Icon;
+import android.os.Build;
+import android.webkit.WebViewFactory;
+
+import org.chromium.components.browser_ui.notifications.ChromeNotificationBuilder;
+import org.chromium.components.browser_ui.notifications.NotificationBuilder;
+import org.chromium.components.browser_ui.notifications.NotificationMetadata;
+import org.chromium.components.browser_ui.notifications.channels.ChannelsInitializer;
+
+/** A notification builder for WebLayer which has extra logic to make icons work correctly. */
+final class WebLayerNotificationBuilder extends NotificationBuilder {
+    public WebLayerNotificationBuilder(Context context, String channelId,
+            ChannelsInitializer channelsInitializer, NotificationMetadata metadata) {
+        super(context, channelId, channelsInitializer, metadata);
+    }
+
+    @Override
+    public ChromeNotificationBuilder setSmallIcon(int icon) {
+        if (WebLayerImpl.isAndroidResource(icon)) {
+            super.setSmallIcon(icon);
+            return this;
+        }
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
+            super.setSmallIcon(
+                    Icon.createWithResource(WebViewFactory.getLoadedPackageInfo().packageName,
+                            WebLayerImpl.getResourceIdForSystemUi(icon)));
+        } else {
+            // Some fallback is required, or the notification won't appear.
+            super.setSmallIcon(android.R.drawable.radiobutton_on_background);
+        }
+        return this;
+    }
+}