Commit f63587ac authored by Clark DuVall's avatar Clark DuVall Committed by Chromium LUCI CQ

[WebLayer] Fix relro sharing in GPU and load native lib in the background

WebLayer relro sharing in the browser process has likely always been
broken in WebView compatibility mode because the ClassLoader created
does not have a class loader namespace, see here: https://cs.android.com/android/platform/superproject/+/master:frameworks/base/native/webview/loader/loader.cpp;l=129-132;drc=master
Unfortunately, this is difficult to fix since sharing relro in WebLayer
when it is loaded in the same process as WebView causes crashes. This
will be investigated more in a follow up.

Relro sharing was never implemented for the WebLayer GPU process, and
this change adds support to do that.

In addition, library loading is kicked off on a separate thread much
earlier in startup, which allows it to finish before it is needed on the
UI thread.

Pinpoint runs:
startup.mobile: https://pinpoint-dot-chromeperf.appspot.com/job/169ab7ef520000
-18.4% messageloop_start_time
-3.4% navigation_commit_time
-1.8% first_contentful_paint_time

weblayer_startup: https://pinpoint-dot-chromeperf.appspot.com/job/17959d48d20000
-6.4% weblayer_startup_wall_time

system_health.memory_mobile: https://pinpoint-dot-chromeperf.appspot.com/job/16d61b37520000
-10.0% (3.9MiB) all_processes private_dirty_size
-29.8% (3.7Mib) gpu_process private_dirty_size

Bug: 1146438
Change-Id: I52292f0472f18397b0a900307649e6f1b54bd5c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581007
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835580}
parent 050fee06
......@@ -425,7 +425,7 @@ public class LibraryLoader {
}
private void loadWithChromiumLinker(ApplicationInfo appInfo, String library) {
Linker linker = Linker.getInstance();
Linker linker = Linker.getInstance(appInfo);
if (isInZipFile()) {
String sourceDir = appInfo.sourceDir;
......
......@@ -5,6 +5,7 @@
package org.chromium.base.library_loader;
import android.annotation.SuppressLint;
import android.content.pm.ApplicationInfo;
import android.os.Bundle;
import android.os.Parcel;
import android.os.ParcelFileDescriptor;
......@@ -202,7 +203,7 @@ public abstract class Linker {
*
* @return the Linker implementation instance.
*/
public static Linker getInstance() {
public static Linker getInstance(ApplicationInfo info) {
// A non-monochrome APK (such as ChromePublic.apk) can be installed on N+ in these
// circumstances:
// * installing APK manually
......@@ -222,8 +223,7 @@ public abstract class Linker {
if (sSingleton == null) {
// With incremental install, it's important to fall back to the "normal"
// library loading path in order for the libraries to be found.
String appClass =
ContextUtils.getApplicationContext().getApplicationInfo().className;
String appClass = info.className;
boolean isIncrementalInstall =
appClass != null && appClass.contains("incrementalinstall");
if (LibraryLoader.getInstance().useModernLinker() && !isIncrementalInstall) {
......@@ -237,6 +237,14 @@ public abstract class Linker {
}
}
/**
* Version of getInstance() which will use the ApplicationInfo from
* ContextUtils.getApplicationContext().
*/
public static Linker getInstance() {
return getInstance(ContextUtils.getApplicationContext().getApplicationInfo());
}
/**
* Obtain a random base load address at which to place loaded libraries.
*
......
......@@ -27,6 +27,8 @@ public final class ChildProcessServiceImpl extends IChildProcessService.Stub {
@UsedByReflection("WebLayer")
public static IBinder create(Service service, Context appContext, Context remoteContext) {
WebLayerImpl.setLibraryPreloader(
remoteContext.getPackageName(), remoteContext.getClassLoader());
ClassLoaderContextWrapperFactory.setLightDarkResourceOverrideContext(
remoteContext, remoteContext);
// Wrap the app context so that it can be used to load WebLayer implementation classes.
......
......@@ -44,6 +44,7 @@ import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.LibraryProcessType;
import org.chromium.base.library_loader.NativeLibraryPreloader;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.components.browser_ui.contacts_picker.ContactsPickerDialog;
import org.chromium.components.browser_ui.photo_picker.DecoderServiceHost;
......@@ -220,6 +221,13 @@ public final class WebLayerImpl extends IWebLayer.Stub {
notifyWebViewRunningInProcess(remoteContext.getClassLoader());
}
// Load library in the background since it may be expensive.
// TODO(crbug.com/1146438): Look into enabling relro sharing in browser process. It seems to
// crash when WebView is loaded in the same process.
new Thread(() -> {
LibraryLoader.getInstance().loadNowOverrideApplicationContext(remoteContext);
}).start();
Context appContext = minimalInitForContext(
ObjectWrapper.unwrap(appContextWrapper, Context.class), remoteContext);
PackageInfo packageInfo = WebViewFactory.getLoadedPackageInfo();
......@@ -251,12 +259,6 @@ public final class WebLayerImpl extends IWebLayer.Stub {
/*readCommandLine=*/true);
TraceEvent.begin("WebLayer init");
// If a remote context is not provided, the client is an older version that loads the native
// library on the client side.
if (remoteContextWrapper != null) {
loadNativeLibrary(packageInfo.packageName);
}
BuildInfo.setBrowserPackageInfo(packageInfo);
BuildInfo.setFirebaseAppId(
FirebaseConfig.getFirebaseAppIdForPackage(packageInfo.packageName));
......@@ -278,10 +280,6 @@ public final class WebLayerImpl extends IWebLayer.Stub {
DeviceUtils.addDeviceSpecificUserAgentSwitch();
ContentUriUtils.setFileProviderUtil(new FileProviderHelper());
// TODO: Validate that doing this disk IO on the main thread is necessary.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
LibraryLoader.getInstance().ensureInitialized();
}
GmsBridge.getInstance().setSafeBrowsingHandler();
GmsBridge.getInstance().initializeBuiltInPaymentApps();
......@@ -322,6 +320,17 @@ public final class WebLayerImpl extends IWebLayer.Stub {
TraceEvent.end("WebLayer init");
}
public static void setLibraryPreloader(String packageName, ClassLoader classLoader) {
if (!LibraryLoader.getInstance().isLoadedByZygote()) {
LibraryLoader.getInstance().setNativeLibraryPreloader(new NativeLibraryPreloader() {
@Override
public int loadLibrary(ApplicationInfo info) {
return loadNativeLibrary(packageName, classLoader);
}
});
}
}
@Override
public IBrowserFragment createBrowserFragmentImpl(
IRemoteFragmentClient fragmentClient, IObjectWrapper fragmentArgs) {
......@@ -751,20 +760,20 @@ public final class WebLayerImpl extends IWebLayer.Stub {
}
}
private void loadNativeLibrary(String packageName) {
private static int loadNativeLibrary(String packageName, ClassLoader cl) {
// Loading the library triggers disk access.
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {
WebViewFactory.loadWebViewNativeLibraryFromPackage(
packageName, getClass().getClassLoader());
return WebViewFactory.loadWebViewNativeLibraryFromPackage(packageName, cl);
} else {
try {
Method loadNativeLibrary =
WebViewFactory.class.getDeclaredMethod("loadNativeLibrary");
loadNativeLibrary.setAccessible(true);
loadNativeLibrary.invoke(null);
return (int) loadNativeLibrary.invoke(null);
} catch (ReflectiveOperationException e) {
Log.e(TAG, "Failed to load native library.", e);
return 6; // LIBLOAD_FAILED_TO_LOAD_LIBRARY
}
}
}
......
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