[WebLayer] Add RELRO sharing for renderers not forking from webview_z…

…ygote While investigating WebLayer perf, I noticed a significant memory difference in renderer processes from WebLayer vs Chrome on Android Go. It turns out Android Go doesn't use the WebView zygote and WebView runs in single process mode, which means there was no code handling RELRO sharing for renderers in this case. This situation will also apply to low end devices running Trichrome/Standalone WebView in N+ where we use the WebView provider for renderer processes, so this change adds support for those as well. Pinpoint run on Android Go: https://pinpoint-dot-chromeperf.appspot.com/job/17aab8fad20000 renderer private_dirty_size: -31.1% renderer private_footprint_size: -15.3% Bug: 1146438 Change-Id: Ic2ec1b282e84fe2643940b56d41ea6e0f21cadcb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2654726 Reviewed-by: Richard Coles <[email protected]> Reviewed-by: Yaron Friedman <[email protected]> Commit-Queue: Clark DuVall <[email protected]> Cr-Commit-Position: refs/heads/master@{#848715}
AryanGitHub · Jan 29, 2021 · d685493 · d685493
1 parent b0d5697
commit d685493
Show file tree

Hide file tree

Showing 18 changed files with 69 additions and 53 deletions.
diff --git a/android_webview/glue/BUILD.gn b/android_webview/glue/BUILD.gn
@@ -48,7 +48,6 @@ android_library("glue_java") {
     "java/src/com/android/webview/chromium/GlueApiHelperForQ.java",
     "java/src/com/android/webview/chromium/GlueApiHelperForR.java",
     "java/src/com/android/webview/chromium/GraphicsUtils.java",
-    "java/src/com/android/webview/chromium/MonochromeLibraryPreloader.java",
     "java/src/com/android/webview/chromium/PacProcessorImpl.java",
     "java/src/com/android/webview/chromium/SafeBrowsingResponseAdapter.java",
     "java/src/com/android/webview/chromium/ServiceWorkerClientAdapter.java",
@@ -79,6 +78,7 @@ android_library("glue_java") {
     "java/src/com/android/webview/chromium/WebViewContentsClientAdapter.java",
     "java/src/com/android/webview/chromium/WebViewDatabaseAdapter.java",
     "java/src/com/android/webview/chromium/WebViewDelegateFactory.java",
+    "java/src/com/android/webview/chromium/WebViewLibraryPreloader.java",
     "java/src/com/android/webview/chromium/WebViewRenderProcessAdapter.java",
     "java/src/com/android/webview/chromium/WebViewRenderProcessClientAdapter.java",
     "java/src/com/android/webview/chromium/WebkitToSharedGlueConverter.java",

diff --git a/android_webview/glue/java/src/com/android/webview/chromium/MonochromeLibraryPreloader.java b/android_webview/glue/java/src/com/android/webview/chromium/MonochromeLibraryPreloader.java
diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewLibraryPreloader.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewLibraryPreloader.java
@@ -0,0 +1,21 @@
+// Copyright 2021 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 com.android.webview.chromium;
+
+import android.webkit.WebViewFactory;
+
+import org.chromium.base.library_loader.NativeLibraryPreloader;
+
+/**
+ * The library preloader for Monochrome and Trichrome for sharing native library's relro
+ * between Chrome and WebView/WebLayer.
+ */
+public class WebViewLibraryPreloader extends NativeLibraryPreloader {
+    @Override
+    public int loadLibrary(String packageName) {
+        return WebViewFactory.loadWebViewNativeLibraryFromPackage(
+                packageName, getClass().getClassLoader());
+    }
+}
diff --git a/android_webview/nonembedded/BUILD.gn b/android_webview/nonembedded/BUILD.gn
@@ -25,6 +25,7 @@ android_library("nonembedded_java") {
     "//android_webview:android_webview_product_config_java",
     "//android_webview:common_aidl_java",
     "//android_webview:common_java",
+    "//android_webview/glue:glue_java",
     "//android_webview/proto:metrics_bridge_records_proto_java",
     "//base:base_java",
     "//base:jni_java",

diff --git a/.../nonembedded/java/src/org/chromium/android_webview/nonembedded/WebViewApkApplication.java b/.../nonembedded/java/src/org/chromium/android_webview/nonembedded/WebViewApkApplication.java
@@ -8,6 +8,9 @@
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.pm.PackageManager;
+import android.os.Build;
+
+import com.android.webview.chromium.WebViewLibraryPreloader;
 
 import org.chromium.android_webview.AwLocaleConfig;
 import org.chromium.android_webview.common.CommandLineUtil;
@@ -70,6 +73,12 @@ public static void maybeInitProcessGlobals() {
             UmaRecorderHolder.setAllowNativeUmaRecorder(false);
             UmaRecorderHolder.setNonNativeDelegate(new AwNonembeddedUmaRecorder());
         }
+
+        // Limit to N+ since external services were added in N.
+        if (!LibraryLoader.getInstance().isLoadedByZygote()
+                && Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
+            LibraryLoader.getInstance().setNativeLibraryPreloader(new WebViewLibraryPreloader());
+        }
     }
 
     // Returns true if running in the "webview_apk" or "webview_service" process.

diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
@@ -479,27 +479,28 @@ public void ensureMainDexInitialized() {
      * that it won't be (implicitly) called during library loading.
      */
     public void preloadNow() {
-        preloadNowOverrideApplicationContext(ContextUtils.getApplicationContext());
+        preloadNowOverridePackageName(
+                ContextUtils.getApplicationContext().getApplicationInfo().packageName);
     }
 
     /**
      * Similar to {@link #preloadNow}, but allows specifying app context to use.
      */
-    public void preloadNowOverrideApplicationContext(Context appContext) {
+    public void preloadNowOverridePackageName(String packageName) {
         synchronized (mLock) {
             setLinkerImplementationIfNeededAlreadyLocked();
             if (mUseChromiumLinker) return;
-            preloadAlreadyLocked(appContext.getApplicationInfo(), false /* inZygote */);
+            preloadAlreadyLocked(packageName, false /* inZygote */);
         }
     }
 
     @GuardedBy("mLock")
-    private void preloadAlreadyLocked(ApplicationInfo appInfo, boolean inZygote) {
+    private void preloadAlreadyLocked(String packageName, boolean inZygote) {
         try (TraceEvent te = TraceEvent.scoped("LibraryLoader.preloadAlreadyLocked")) {
             // Preloader uses system linker, we shouldn't preload if Chromium linker is used.
             assert !useChromiumLinker() || inZygote;
             if (mLibraryPreloader != null && !mLibraryPreloaderCalled) {
-                mLibraryPreloader.loadLibrary(appInfo);
+                mLibraryPreloader.loadLibrary(packageName);
                 mLibraryPreloaderCalled = true;
             }
         }
@@ -629,7 +630,7 @@ private void loadWithChromiumLinker(ApplicationInfo appInfo, String library) {
     @SuppressLint("UnsafeDynamicallyLoadedCode")
     private void loadWithSystemLinkerAlreadyLocked(ApplicationInfo appInfo, boolean inZygote) {
         setEnvForNative();
-        preloadAlreadyLocked(appInfo, inZygote);
+        preloadAlreadyLocked(appInfo.packageName, inZygote);
 
         // If the libraries are located in the zip file, assert that the device API level is M or
         // higher. On devices <=M, the libraries should always be loaded by LegacyLinker.

diff --git a/base/android/java/src/org/chromium/base/library_loader/NativeLibraryPreloader.java b/base/android/java/src/org/chromium/base/library_loader/NativeLibraryPreloader.java
@@ -4,17 +4,15 @@
 
 package org.chromium.base.library_loader;
 
-import android.content.pm.ApplicationInfo;
-
 /**
  * This is interface to preload the native library before calling System.loadLibrary.
  *
  * Preloading shouldn't call System.loadLibrary() or otherwise cause any Chromium
  * code to be run, because it can be called before Chromium command line is known.
  * It can however open the library via dlopen() or android_dlopen_ext() so that
  * dlopen() later called by System.loadLibrary() becomes a noop. This is what the
- * only subclass (MonochromeLibraryPreloader) is doing.
+ * only subclass (WebViewLibraryPreloader) is doing.
  */
 public abstract class NativeLibraryPreloader {
-    public abstract int loadLibrary(ApplicationInfo appInfo);
+    public abstract int loadLibrary(String packageName);
 }
diff --git a/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConnection.java b/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConnection.java
@@ -16,6 +16,7 @@
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
+import org.chromium.base.BuildInfo;
 import org.chromium.base.ChildBindingState;
 import org.chromium.base.Log;
 import org.chromium.base.MemoryPressureLevel;
@@ -260,6 +261,8 @@ public ChildProcessConnection(final Context context, ComponentName serviceName,
         mFallbackServiceName = fallbackServiceName;
         mServiceBundle = serviceBundle != null ? serviceBundle : new Bundle();
         mServiceBundle.putBoolean(ChildProcessConstants.EXTRA_BIND_TO_CALLER, bindToCaller);
+        mServiceBundle.putString(ChildProcessConstants.EXTRA_BROWSER_PACKAGE_NAME,
+                BuildInfo.getInstance().packageName);
         mBindToCaller = bindToCaller;
         mInstanceName = instanceName;
         mBindAsExternalService = bindAsExternalService;

diff --git a/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConstants.java b/base/android/java/src/org/chromium/base/process_launcher/ChildProcessConstants.java
@@ -24,4 +24,8 @@ public interface ChildProcessConstants {
 
     // Key for the file descriptors that should be mapped in the child process.
     public static final String EXTRA_FILES = "org.chromium.base.process_launcher.extra.extraFiles";
+
+    // Key for the browser package name.
+    public static final String EXTRA_BROWSER_PACKAGE_NAME =
+            "org.chromium.base.process_launcher.extra.browser_package_name";
 }
diff --git a/base/android/java/src/org/chromium/base/process_launcher/ChildProcessService.java b/base/android/java/src/org/chromium/base/process_launcher/ChildProcessService.java
@@ -342,9 +342,16 @@ public IBinder onBind(Intent intent) {
                 intent.getBooleanExtra(ChildProcessConstants.EXTRA_BIND_TO_CALLER, false);
         mServiceBound = true;
         mDelegate.onServiceBound(intent);
+
+        String packageName =
+                intent.getStringExtra(ChildProcessConstants.EXTRA_BROWSER_PACKAGE_NAME);
+        if (packageName == null) {
+            packageName = getApplicationContext().getApplicationInfo().packageName;
+        }
         // Don't block bind() with any extra work, post it to the application thread instead.
+        final String preloadPackageName = packageName;
         new Handler(Looper.getMainLooper())
-                .post(() -> mDelegate.preloadNativeLibrary(getApplicationContext()));
+                .post(() -> mDelegate.preloadNativeLibrary(preloadPackageName));
         return mBinder;
     }
 

diff --git a/base/android/java/src/org/chromium/base/process_launcher/ChildProcessServiceDelegate.java b/base/android/java/src/org/chromium/base/process_launcher/ChildProcessServiceDelegate.java
@@ -42,9 +42,10 @@ public interface ChildProcessServiceDelegate {
      * Called when the delegate should preload the native library.
      * Preloading is automatically done during library loading, but can also be called explicitly
      * to speed up the loading. See {@link LibraryLoader.preloadNow}.
-     * @param hostContext The host context the library should be preloaded with (i.e. Chrome).
+     * @param packageName The package name the library should be preloaded with (i.e.
+     * org.chromium.chrome).
      */
-    void preloadNativeLibrary(Context hostContext);
+    void preloadNativeLibrary(String packageName);
 
     /**
      * Should return a map that associatesfile descriptors' IDs to keys.

diff --git a/base/test/android/java/src/org/chromium/base/MultiprocessTestClientServiceDelegate.java b/base/test/android/java/src/org/chromium/base/MultiprocessTestClientServiceDelegate.java
@@ -51,7 +51,7 @@ public void onConnectionSetup(Bundle connectionBundle, List<IBinder> callbacks)
     }
 
     @Override
-    public void preloadNativeLibrary(Context hostContext) {
+    public void preloadNativeLibrary(String packageName) {
         LibraryLoader.getInstance().preloadNow();
     }
 

diff --git a/chrome/android/java/DEPS b/chrome/android/java/DEPS
@@ -66,10 +66,10 @@ specific_include_rules = {
   # Special-case where monochrome composes chrome+webview
   "MonochromeApplication\.java": [
     "+android_webview/nonembedded/java/src/org/chromium/android_webview/nonembedded/WebViewApkApplication.java",
-    "+android_webview/glue/java/src/com/android/webview/chromium/MonochromeLibraryPreloader.java",
+    "+android_webview/glue/java/src/com/android/webview/chromium/WebViewLibraryPreloader.java",
   ],
   "SplitMonochromeApplication\.java": [
     "+android_webview/nonembedded/java/src/org/chromium/android_webview/nonembedded/WebViewApkApplication.java",
-    "+android_webview/glue/java/src/com/android/webview/chromium/MonochromeLibraryPreloader.java",
+    "+android_webview/glue/java/src/com/android/webview/chromium/WebViewLibraryPreloader.java",
   ],
 }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/base/SplitMonochromeApplication.java b/chrome/android/java/src/org/chromium/chrome/browser/base/SplitMonochromeApplication.java
@@ -6,10 +6,7 @@
 
 import android.content.Context;
 
-import com.android.webview.chromium.MonochromeLibraryPreloader;
-
 import org.chromium.android_webview.nonembedded.WebViewApkApplication;
-import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.chrome.browser.version.ChromeVersionInfo;
 import org.chromium.content_public.browser.ChildProcessCreationParams;
@@ -49,9 +46,6 @@ protected Impl createNonBrowserApplication() {
 
     public static void initializeMonochromeProcessCommon(String packageName) {
         WebViewApkApplication.maybeInitProcessGlobals();
-        if (!LibraryLoader.getInstance().isLoadedByZygote()) {
-            LibraryLoader.getInstance().setNativeLibraryPreloader(new MonochromeLibraryPreloader());
-        }
 
         // ChildProcessCreationParams is only needed for browser process, though it is
         // created and set in all processes. We must set isExternalService to true for

diff --git a/.../public/android/java/src/org/chromium/content/app/ContentChildProcessServiceDelegate.java b/.../public/android/java/src/org/chromium/content/app/ContentChildProcessServiceDelegate.java
@@ -78,11 +78,11 @@ public void onConnectionSetup(Bundle connectionBundle, List<IBinder> clientInter
     }
 
     @Override
-    public void preloadNativeLibrary(Context hostContext) {
+    public void preloadNativeLibrary(String packageName) {
         // This function can be called before command line is set. That is fine because
         // preloading explicitly doesn't run any Chromium code, see NativeLibraryPreloader
         // for more info.
-        LibraryLoader.getInstance().preloadNowOverrideApplicationContext(hostContext);
+        LibraryLoader.getInstance().preloadNowOverridePackageName(packageName);
     }
 
     @Override

diff --git a/...t/shell/android/shell_apk/src/org/chromium/content_shell_apk/TestChildProcessService.java b/...t/shell/android/shell_apk/src/org/chromium/content_shell_apk/TestChildProcessService.java
@@ -74,7 +74,7 @@ public void onConnectionSetup(Bundle connectionBundle, List<IBinder> clientInter
         }
 
         @Override
-        public void preloadNativeLibrary(Context hostContext) {
+        public void preloadNativeLibrary(String packageName) {
             LibraryLoader.getInstance().preloadNow();
         }
 

diff --git a/docs/android_native_libraries.md b/docs/android_native_libraries.md
@@ -186,7 +186,7 @@ Builds on | Variant | Chrome | Library | Webview
  * For Android N-P:
    * The OS maintains a RELRO file on disk with the contents of the GNU_RELRO segment.
    * All Android apps that contain a WebView load `libmonochrome.so` at the same virtual address and apply RELRO sharing against the memory-mapped RELRO file.
-   * Chrome uses `MonochromeLibraryPreloader` to call into the same WebView library loading code.
+   * Chrome uses `WebViewLibraryPreloader` to call into the same WebView library loading code.
      * When Monochrome is the WebView provider, `libmonochrome.so` is loaded with the system's cached RELRO's applied.
    * `System.loadLibrary()` is called afterwards.
      * When Monochrome is the WebView provider, this only calls JNI_OnLoad, since the library is already loaded. Otherwise, this loads the library and no RELRO sharing occurs.

diff --git a/weblayer/browser/java/org/chromium/weblayer_private/ChildProcessServiceImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/ChildProcessServiceImpl.java
@@ -7,7 +7,6 @@
 import android.app.Service;
 import android.content.Context;
 import android.content.Intent;
-import android.content.pm.ApplicationInfo;
 import android.os.Build;
 import android.os.IBinder;
 import android.webkit.WebViewFactory;
@@ -68,12 +67,12 @@ private ChildProcessServiceImpl(Service service, Context context) {
         mService = ChildProcessServiceFactory.create(service, context);
     }
 
-    private static void setLibraryPreloader(String packageName, ClassLoader classLoader) {
+    public static void setLibraryPreloader(String webLayerPackageName, ClassLoader classLoader) {
         if (!LibraryLoader.getInstance().isLoadedByZygote()) {
             LibraryLoader.getInstance().setNativeLibraryPreloader(new NativeLibraryPreloader() {
                 @Override
-                public int loadLibrary(ApplicationInfo info) {
-                    return loadNativeLibrary(packageName, classLoader);
+                public int loadLibrary(String packageName) {
+                    return loadNativeLibrary(webLayerPackageName, classLoader);
                 }
             });
         }