From 29675b294126bd00bb51bc8616f476aa90abacf1 Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Wed, 4 Oct 2017 11:29:58 -0400 Subject: Fix NPE in WebView relro creator process. There's no context available in the relro creator process, resulting in a NPE and crash when it tries to see if WebView is supported. Skip the check in this case, because we know it's supported if we ran the relro creation process at all. Change-Id: I95e9aad7407b8f73dddf8a8b685d41d2f500736a Fixes: 67398770 Test: boot, make sure no crash dialog from "android" --- core/java/android/webkit/WebViewLibraryLoader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index 6f9e8ece4b13..21316e7db9e4 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -80,7 +80,7 @@ class WebViewLibraryLoader { } finally { // We must do our best to always notify the update service, even if something fails. try { - WebViewFactory.getUpdateService().notifyRelroCreationCompleted(); + WebViewFactory.getUpdateServiceUnchecked().notifyRelroCreationCompleted(); } catch (RemoteException e) { Log.e(LOGTAG, "error notifying update service", e); } -- cgit v1.2.3 From ae498f270230e20b5a777ed2d6387a21767625a3 Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Thu, 5 Oct 2017 15:34:13 +0100 Subject: [WebView] Only pass one path to relro creation/loading at a time. There's no need to send both 32-bit and 64-bit paths to the native side of the relro-creation/loading logic, we can check which one to send on the java side instead. Bug: 28736099 Test: Load WebView app, ensure relro file is loaded into the app process. Change-Id: Ia3fb4b3ed686c3e70c26a384aae966bda179d225 --- core/java/android/webkit/WebViewLibraryLoader.java | 30 ++++++++++------------ 1 file changed, 13 insertions(+), 17 deletions(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index 6f9e8ece4b13..db722d51ea5f 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -62,20 +62,18 @@ class WebViewLibraryLoader { boolean result = false; boolean is64Bit = VMRuntime.getRuntime().is64Bit(); try { - if (args.length != 2 || args[0] == null || args[1] == null) { + if (args.length != 1 || args[0] == null) { Log.e(LOGTAG, "Invalid RelroFileCreator args: " + Arrays.toString(args)); return; } - Log.v(LOGTAG, "RelroFileCreator (64bit = " + is64Bit + "), " - + " 32-bit lib: " + args[0] + ", 64-bit lib: " + args[1]); + Log.v(LOGTAG, "RelroFileCreator (64bit = " + is64Bit + "), lib: " + args[0]); if (!sAddressSpaceReserved) { Log.e(LOGTAG, "can't create relro file; address space not reserved"); return; } - result = nativeCreateRelroFile(args[0] /* path32 */, - args[1] /* path64 */, - CHROMIUM_WEBVIEW_NATIVE_RELRO_32, - CHROMIUM_WEBVIEW_NATIVE_RELRO_64); + result = nativeCreateRelroFile(args[0] /* path */, + is64Bit ? CHROMIUM_WEBVIEW_NATIVE_RELRO_64 : + CHROMIUM_WEBVIEW_NATIVE_RELRO_32); if (result && DEBUG) Log.v(LOGTAG, "created relro file"); } finally { // We must do our best to always notify the update service, even if something fails. @@ -96,7 +94,7 @@ class WebViewLibraryLoader { /** * Create a single relro file by invoking an isolated process that to do the actual work. */ - static void createRelroFile(final boolean is64Bit, String[] nativeLibraryPaths) { + static void createRelroFile(final boolean is64Bit, String nativeLibraryPath) { final String abi = is64Bit ? Build.SUPPORTED_64_BIT_ABIS[0] : Build.SUPPORTED_32_BIT_ABIS[0]; @@ -114,13 +112,12 @@ class WebViewLibraryLoader { }; try { - if (nativeLibraryPaths == null - || nativeLibraryPaths[0] == null || nativeLibraryPaths[1] == null) { + if (nativeLibraryPath == null) { throw new IllegalArgumentException( "Native library paths to the WebView RelRo process must not be null!"); } int pid = LocalServices.getService(ActivityManagerInternal.class).startIsolatedProcess( - RelroFileCreator.class.getName(), nativeLibraryPaths, + RelroFileCreator.class.getName(), new String[] { nativeLibraryPath }, "WebViewLoader-" + abi, abi, Process.SHARED_RELRO_UID, crashHandler); if (pid <= 0) throw new Exception("Failed to start the relro file creator process"); } catch (Throwable t) { @@ -217,8 +214,9 @@ class WebViewLibraryLoader { final String libraryFileName = WebViewFactory.getWebViewLibrary(packageInfo.applicationInfo); - int result = nativeLoadWithRelroFile(libraryFileName, CHROMIUM_WEBVIEW_NATIVE_RELRO_32, - CHROMIUM_WEBVIEW_NATIVE_RELRO_64, clazzLoader); + String relroPath = VMRuntime.getRuntime().is64Bit() ? CHROMIUM_WEBVIEW_NATIVE_RELRO_64 : + CHROMIUM_WEBVIEW_NATIVE_RELRO_32; + int result = nativeLoadWithRelroFile(libraryFileName, relroPath, clazzLoader); if (result != WebViewFactory.LIBLOAD_SUCCESS) { Log.w(LOGTAG, "failed to load with relro file, proceeding without"); } else if (DEBUG) { @@ -313,8 +311,6 @@ class WebViewLibraryLoader { } static native boolean nativeReserveAddressSpace(long addressSpaceToReserve); - static native boolean nativeCreateRelroFile(String lib32, String lib64, - String relro32, String relro64); - static native int nativeLoadWithRelroFile(String lib, String relro32, String relro64, - ClassLoader clazzLoader); + static native boolean nativeCreateRelroFile(String lib, String relro); + static native int nativeLoadWithRelroFile(String lib, String relro, ClassLoader clazzLoader); } -- cgit v1.2.3 From 0e541ef99221d7681e2fe0d721e92c4d6c74073c Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Tue, 24 Oct 2017 14:28:26 +0100 Subject: Refactor some WebView loading logic into WebViewLibraryLoader. This is in preparation for a patch which with bigger changes for WebViewLibraryLoader (including tests). Bug: 28736099 Test: ensure relro loaded into app process on 32-bit device. Change-Id: Ic1aef697c1010380d8bce466ee14627d9cdff9e7 --- core/java/android/webkit/WebViewLibraryLoader.java | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index fa1a3907b1f9..175f35f41ebf 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -127,11 +127,34 @@ class WebViewLibraryLoader { } } + static int prepareNativeLibraries(PackageInfo webviewPackageInfo) + throws WebViewFactory.MissingWebViewPackageException { + String[] nativeLibs = updateWebViewZygoteVmSize(webviewPackageInfo); + if (DEBUG) Log.v(LOGTAG, "creating relro files"); + int numRelros = 0; + + // We must always trigger createRelRo regardless of the value of nativeLibraryPaths. Any + // unexpected values will be handled there to ensure that we trigger notifying any process + // waiting on relro creation. + if (Build.SUPPORTED_32_BIT_ABIS.length > 0) { + if (DEBUG) Log.v(LOGTAG, "Create 32 bit relro"); + createRelroFile(false /* is64Bit */, nativeLibs[0]); + numRelros++; + } + + if (Build.SUPPORTED_64_BIT_ABIS.length > 0) { + if (DEBUG) Log.v(LOGTAG, "Create 64 bit relro"); + createRelroFile(true /* is64Bit */, nativeLibs[1]); + numRelros++; + } + return numRelros; + } + /** * * @return the native WebView libraries in the new WebView APK. */ - static String[] updateWebViewZygoteVmSize(PackageInfo packageInfo) + private static String[] updateWebViewZygoteVmSize(PackageInfo packageInfo) throws WebViewFactory.MissingWebViewPackageException { // Find the native libraries of the new WebView package, to change the size of the // memory region in the Zygote reserved for the library. -- cgit v1.2.3 From 7051dd1eebbbca9e3d7603821ad2b5e922f7d83b Mon Sep 17 00:00:00 2001 From: Nate Fischer Date: Thu, 26 Oct 2017 14:51:25 -0700 Subject: WebView: make notes in docs stand out This CL rewrites notes to use the 'note' CSS class, to help them stand out. This isn't every instance of 'note', but it catches the handful where I think there's a readability benefit to the new format. This changes a few other non-public notes for the sake of consistency. Change-Id: Icf62e34a719c79f74e74abdb55f1be42939c2f9e Fixes: 68324591 Test: make -j40 docs (and manually verify it looks good) --- core/java/android/webkit/WebViewLibraryLoader.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index 6f9e8ece4b13..b3cd9cc6bcd6 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -204,7 +204,9 @@ class WebViewLibraryLoader { /** * Load WebView's native library into the current process. - * Note: assumes that we have waited for relro creation. + * + *

Note: Assumes that we have waited for relro creation. + * * @param clazzLoader class loader used to find the linker namespace to load the library into. * @param packageInfo the package from which WebView is loaded. */ -- cgit v1.2.3 From 36bed13f8340359fac1c709460cfa95142c5e6a1 Mon Sep 17 00:00:00 2001 From: Gustav Sennton Date: Mon, 18 Sep 2017 20:52:43 +0100 Subject: [WebView] Clean up code for finding paths and sizes of native libs Now that we no longer pass the paths of the native WebView libraries to the native loading code we can simplify the code for fetching the path and the size of the native WebView library. Also add automated tests (WebViewLoadingTests) for loading native libraries from test APKs where the libraries are stored either on disk or as uncompressed libraries in the APKs. Bug: 28736099 Test: start WebView-app, and ensure that the relro file is loaded into the app process. Do this for both 32-, and 64-bit apps. And both for a WebView APK where the libraries are loaded onto disk, and an APK where the libraries are loaded uncompressed from the APK. Test: make WebViewLoadingTests && make tradefed-all && \ tradefed.sh run template/local_min --template:map test=WebViewLoadingTests Change-Id: I219dd03aa93de8cf6e64e01f1d8abe38474f5790 --- core/java/android/webkit/WebViewLibraryLoader.java | 229 +++++++++++---------- 1 file changed, 124 insertions(+), 105 deletions(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index 175f35f41ebf..341c69fd2eba 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -16,6 +16,8 @@ package android.webkit; +import android.annotation.NonNull; +import android.annotation.Nullable; import android.app.ActivityManagerInternal; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; @@ -26,6 +28,7 @@ import android.os.SystemProperties; import android.text.TextUtils; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; import com.android.server.LocalServices; import dalvik.system.VMRuntime; @@ -36,7 +39,11 @@ import java.util.Arrays; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; -class WebViewLibraryLoader { +/** + * @hide + */ +@VisibleForTesting +public class WebViewLibraryLoader { private static final String LOGTAG = WebViewLibraryLoader.class.getSimpleName(); private static final String CHROMIUM_WEBVIEW_NATIVE_RELRO_32 = @@ -94,7 +101,7 @@ class WebViewLibraryLoader { /** * Create a single relro file by invoking an isolated process that to do the actual work. */ - static void createRelroFile(final boolean is64Bit, String nativeLibraryPath) { + static void createRelroFile(final boolean is64Bit, @NonNull WebViewNativeLibrary nativeLib) { final String abi = is64Bit ? Build.SUPPORTED_64_BIT_ABIS[0] : Build.SUPPORTED_32_BIT_ABIS[0]; @@ -112,12 +119,12 @@ class WebViewLibraryLoader { }; try { - if (nativeLibraryPath == null) { + if (nativeLib == null || nativeLib.path == null) { throw new IllegalArgumentException( "Native library paths to the WebView RelRo process must not be null!"); } int pid = LocalServices.getService(ActivityManagerInternal.class).startIsolatedProcess( - RelroFileCreator.class.getName(), new String[] { nativeLibraryPath }, + RelroFileCreator.class.getName(), new String[] { nativeLib.path }, "WebViewLoader-" + abi, abi, Process.SHARED_RELRO_UID, crashHandler); if (pid <= 0) throw new Exception("Failed to start the relro file creator process"); } catch (Throwable t) { @@ -127,25 +134,48 @@ class WebViewLibraryLoader { } } + /** + * Perform preparations needed to allow loading WebView from an application. This method should + * be called whenever we change WebView provider. + * @return the number of relro processes started. + */ static int prepareNativeLibraries(PackageInfo webviewPackageInfo) throws WebViewFactory.MissingWebViewPackageException { - String[] nativeLibs = updateWebViewZygoteVmSize(webviewPackageInfo); + WebViewNativeLibrary nativeLib32bit = + getWebViewNativeLibrary(webviewPackageInfo, false /* is64bit */); + WebViewNativeLibrary nativeLib64bit = + getWebViewNativeLibrary(webviewPackageInfo, true /* is64bit */); + updateWebViewZygoteVmSize(nativeLib32bit, nativeLib64bit); + + return createRelros(nativeLib32bit, nativeLib64bit); + } + + /** + * @return the number of relro processes started. + */ + private static int createRelros(@Nullable WebViewNativeLibrary nativeLib32bit, + @Nullable WebViewNativeLibrary nativeLib64bit) { if (DEBUG) Log.v(LOGTAG, "creating relro files"); int numRelros = 0; - // We must always trigger createRelRo regardless of the value of nativeLibraryPaths. Any - // unexpected values will be handled there to ensure that we trigger notifying any process - // waiting on relro creation. if (Build.SUPPORTED_32_BIT_ABIS.length > 0) { - if (DEBUG) Log.v(LOGTAG, "Create 32 bit relro"); - createRelroFile(false /* is64Bit */, nativeLibs[0]); - numRelros++; + if (nativeLib32bit == null) { + Log.e(LOGTAG, "No 32-bit WebView library path, skipping relro creation."); + } else { + if (DEBUG) Log.v(LOGTAG, "Create 32 bit relro"); + createRelroFile(false /* is64Bit */, nativeLib32bit); + numRelros++; + } } if (Build.SUPPORTED_64_BIT_ABIS.length > 0) { - if (DEBUG) Log.v(LOGTAG, "Create 64 bit relro"); - createRelroFile(true /* is64Bit */, nativeLibs[1]); - numRelros++; + if (nativeLib64bit == null) { + Log.e(LOGTAG, "No 64-bit WebView library path, skipping relro creation."); + } else { + if (DEBUG) Log.v(LOGTAG, "Create 64 bit relro"); + createRelroFile(true /* is64Bit */, nativeLib64bit); + numRelros++; + } } return numRelros; } @@ -154,53 +184,28 @@ class WebViewLibraryLoader { * * @return the native WebView libraries in the new WebView APK. */ - private static String[] updateWebViewZygoteVmSize(PackageInfo packageInfo) + private static void updateWebViewZygoteVmSize( + @Nullable WebViewNativeLibrary nativeLib32bit, + @Nullable WebViewNativeLibrary nativeLib64bit) throws WebViewFactory.MissingWebViewPackageException { // Find the native libraries of the new WebView package, to change the size of the // memory region in the Zygote reserved for the library. - String[] nativeLibs = getWebViewNativeLibraryPaths(packageInfo); - if (nativeLibs != null) { - long newVmSize = 0L; - - for (String path : nativeLibs) { - if (path == null || TextUtils.isEmpty(path)) continue; - if (DEBUG) Log.d(LOGTAG, "Checking file size of " + path); - File f = new File(path); - if (f.exists()) { - newVmSize = Math.max(newVmSize, f.length()); - continue; - } - if (path.contains("!/")) { - String[] split = TextUtils.split(path, "!/"); - if (split.length == 2) { - try (ZipFile z = new ZipFile(split[0])) { - ZipEntry e = z.getEntry(split[1]); - if (e != null && e.getMethod() == ZipEntry.STORED) { - newVmSize = Math.max(newVmSize, e.getSize()); - continue; - } - } - catch (IOException e) { - Log.e(LOGTAG, "error reading APK file " + split[0] + ", ", e); - } - } - } - Log.e(LOGTAG, "error sizing load for " + path); - } + long newVmSize = 0L; - if (DEBUG) { - Log.v(LOGTAG, "Based on library size, need " + newVmSize - + " bytes of address space."); - } - // The required memory can be larger than the file on disk (due to .bss), and an - // upgraded version of the library will likely be larger, so always attempt to - // reserve twice as much as we think to allow for the library to grow during this - // boot cycle. - newVmSize = Math.max(2 * newVmSize, CHROMIUM_WEBVIEW_DEFAULT_VMSIZE_BYTES); - Log.d(LOGTAG, "Setting new address space to " + newVmSize); - setWebViewZygoteVmSize(newVmSize); + if (nativeLib32bit != null) newVmSize = Math.max(newVmSize, nativeLib32bit.size); + if (nativeLib64bit != null) newVmSize = Math.max(newVmSize, nativeLib64bit.size); + + if (DEBUG) { + Log.v(LOGTAG, "Based on library size, need " + newVmSize + + " bytes of address space."); } - return nativeLibs; + // The required memory can be larger than the file on disk (due to .bss), and an + // upgraded version of the library will likely be larger, so always attempt to + // reserve twice as much as we think to allow for the library to grow during this + // boot cycle. + newVmSize = Math.max(2 * newVmSize, CHROMIUM_WEBVIEW_DEFAULT_VMSIZE_BYTES); + Log.d(LOGTAG, "Setting new address space to " + newVmSize); + setWebViewZygoteVmSize(newVmSize); } /** @@ -250,64 +255,78 @@ class WebViewLibraryLoader { /** * Fetch WebView's native library paths from {@param packageInfo}. + * @hide */ - static String[] getWebViewNativeLibraryPaths(PackageInfo packageInfo) - throws WebViewFactory.MissingWebViewPackageException { + @Nullable + @VisibleForTesting + public static WebViewNativeLibrary getWebViewNativeLibrary(PackageInfo packageInfo, + boolean is64bit) throws WebViewFactory.MissingWebViewPackageException { ApplicationInfo ai = packageInfo.applicationInfo; final String nativeLibFileName = WebViewFactory.getWebViewLibrary(ai); - String path32; - String path64; - boolean primaryArchIs64bit = VMRuntime.is64BitAbi(ai.primaryCpuAbi); - if (!TextUtils.isEmpty(ai.secondaryCpuAbi)) { - // Multi-arch case. - if (primaryArchIs64bit) { - // Primary arch: 64-bit, secondary: 32-bit. - path64 = ai.nativeLibraryDir; - path32 = ai.secondaryNativeLibraryDir; - } else { - // Primary arch: 32-bit, secondary: 64-bit. - path64 = ai.secondaryNativeLibraryDir; - path32 = ai.nativeLibraryDir; - } - } else if (primaryArchIs64bit) { - // Single-arch 64-bit. - path64 = ai.nativeLibraryDir; - path32 = ""; - } else { - // Single-arch 32-bit. - path32 = ai.nativeLibraryDir; - path64 = ""; + String dir = getWebViewNativeLibraryDirectory(ai, is64bit /* 64bit */); + + WebViewNativeLibrary lib = findNativeLibrary(ai, nativeLibFileName, + is64bit ? Build.SUPPORTED_64_BIT_ABIS : Build.SUPPORTED_32_BIT_ABIS, dir); + + if (DEBUG) { + Log.v(LOGTAG, String.format("Native %d-bit lib: %s", is64bit ? 64 : 32, lib.path)); } + return lib; + } - // Form the full paths to the extracted native libraries. - // If libraries were not extracted, try load from APK paths instead. - if (!TextUtils.isEmpty(path32)) { - path32 += "/" + nativeLibFileName; - File f = new File(path32); - if (!f.exists()) { - path32 = getLoadFromApkPath(ai.sourceDir, - Build.SUPPORTED_32_BIT_ABIS, - nativeLibFileName); - } + /** + * @return the directory of the native WebView library with bitness {@param is64bit}. + * @hide + */ + @VisibleForTesting + public static String getWebViewNativeLibraryDirectory(ApplicationInfo ai, boolean is64bit) { + // Primary arch has the same bitness as the library we are looking for. + if (is64bit == VMRuntime.is64BitAbi(ai.primaryCpuAbi)) return ai.nativeLibraryDir; + + // Secondary arch has the same bitness as the library we are looking for. + if (!TextUtils.isEmpty(ai.secondaryCpuAbi)) { + return ai.secondaryNativeLibraryDir; } - if (!TextUtils.isEmpty(path64)) { - path64 += "/" + nativeLibFileName; - File f = new File(path64); - if (!f.exists()) { - path64 = getLoadFromApkPath(ai.sourceDir, - Build.SUPPORTED_64_BIT_ABIS, - nativeLibFileName); - } + + return ""; + } + + /** + * @return an object describing a native WebView library given the directory path of that + * library, or null if the library couldn't be found. + */ + @Nullable + private static WebViewNativeLibrary findNativeLibrary(ApplicationInfo ai, + String nativeLibFileName, String[] abiList, String libDirectory) + throws WebViewFactory.MissingWebViewPackageException { + if (TextUtils.isEmpty(libDirectory)) return null; + String libPath = libDirectory + "/" + nativeLibFileName; + File f = new File(libPath); + if (f.exists()) { + return new WebViewNativeLibrary(libPath, f.length()); + } else { + return getLoadFromApkPath(ai.sourceDir, abiList, nativeLibFileName); } + } - if (DEBUG) Log.v(LOGTAG, "Native 32-bit lib: " + path32 + ", 64-bit lib: " + path64); - return new String[] { path32, path64 }; + /** + * @hide + */ + @VisibleForTesting + public static class WebViewNativeLibrary { + public final String path; + public final long size; + + WebViewNativeLibrary(String path, long size) { + this.path = path; + this.size = size; + } } - private static String getLoadFromApkPath(String apkPath, - String[] abiList, - String nativeLibFileName) + private static WebViewNativeLibrary getLoadFromApkPath(String apkPath, + String[] abiList, + String nativeLibFileName) throws WebViewFactory.MissingWebViewPackageException { // Search the APK for a native library conforming to a listed ABI. try (ZipFile z = new ZipFile(apkPath)) { @@ -316,13 +335,13 @@ class WebViewLibraryLoader { ZipEntry e = z.getEntry(entry); if (e != null && e.getMethod() == ZipEntry.STORED) { // Return a path formatted for dlopen() load from APK. - return apkPath + "!/" + entry; + return new WebViewNativeLibrary(apkPath + "!/" + entry, e.getSize()); } } } catch (IOException e) { throw new WebViewFactory.MissingWebViewPackageException(e); } - return ""; + return null; } /** -- cgit v1.2.3 From f6690100be5c5cd75c64d9a6a0345acff7b754d1 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Mon, 16 Oct 2017 10:20:32 -0700 Subject: Start processes asynchronously in AMS. Currently, process start is initiated in ActivityManagerService holding the main lock and this takes ~40ms to complete. As seen in some of the issues in previous releases, this is one of the contributors of the lock contention in system_server. This change tries to address this issue by moving the process start outside the locked section. When a process start is required, instead of doing it synchronously, this request will be posted on a handler thread. On the handler thread, this process start request will be completed without holding a lock. If for some reason, we decide the process is not needed anymore before it is actually started or being started, then AMS does everything as usual removing the references to the process from internal state except actually killing the process which will be handled on the handler thread. Bug: 68775202 Test: Ran app startup perf tests using forrest, will update the bug with results. Test: https://docs.google.com/spreadsheets/d/1cW81guRALZXKsN-WZsKyQiCSY-RgkJ2m_M9IfqIquz8 Test: cts-tradefed run singleCommand cts-dev -m CtsActivityManagerDeviceTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsWindowManagerDeviceTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsAppTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsMultiUserHostTestCases Test: adb shell am instrument -e package com.android.server.am -w \ com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner Test: adb shell am instrument -e class com.android.server.pm.UserManagerTest -w \ com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner Test: adb shell am instrument -e package android.content -w \ com.android.frameworks.coretests/android.support.test.runner.AndroidJUnitRunner Test: cts-tradefed run singleCommand cts-dev -m CtsProviderTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsContentTestCases -t \ android.content.cts.ContentResolverTest Test: cts-tradefed run singleCommand cts-dev -m CtsContentTestCases -t \ android.content.cts.ContentProviderTest Test: cts-tradefed run singleCommand cts-dev -m CtsWebkitTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsHostsideWebViewTests Test: cts-tradefed run singleCommand cts-dev -m CtsAssistTestCases Test: make WebViewLoadingTests && make tradefed-all && tradefed.sh \ run template/local_min --template:map test=WebViewLoadingTests Test: adb shell setprop wrap.com.google.android.apps.maps \ '"LIBC_DEBUG_MALLOC_OPTIONS=backtrace logwrapper"' && adb shell dumpsys meminfo --unreachable && check ppid of is logwrapper's pid. Test: cts-tradefed run singleCommand cts-dev -m CtsWrapWrapNoDebugTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsWrapWrapDebugTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsWrapNoWrapTestCases Test: cts-tradefed run singleCommand cts-dev -m CtsWrapWrapDebugMallocDebugTestCases Test: manual Change-Id: I1fe7ce48cd5a4aadccaf6b3d6fdb5cad3304f1d3 --- core/java/android/webkit/WebViewLibraryLoader.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index de0b97d15e23..eb2b6bccf4cc 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -123,10 +123,11 @@ public class WebViewLibraryLoader { throw new IllegalArgumentException( "Native library paths to the WebView RelRo process must not be null!"); } - int pid = LocalServices.getService(ActivityManagerInternal.class).startIsolatedProcess( - RelroFileCreator.class.getName(), new String[] { nativeLib.path }, - "WebViewLoader-" + abi, abi, Process.SHARED_RELRO_UID, crashHandler); - if (pid <= 0) throw new Exception("Failed to start the relro file creator process"); + boolean success = LocalServices.getService(ActivityManagerInternal.class) + .startIsolatedProcess( + RelroFileCreator.class.getName(), new String[] { nativeLib.path }, + "WebViewLoader-" + abi, abi, Process.SHARED_RELRO_UID, crashHandler); + if (!success) throw new Exception("Failed to start the relro file creator process"); } catch (Throwable t) { // Log and discard errors as we must not crash the system server. Log.e(LOGTAG, "error starting relro file creator for abi " + abi, t); -- cgit v1.2.3 From ec572f6a812da3a5537e3d509954cfe484b0c799 Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Wed, 21 Feb 2018 15:55:24 -0500 Subject: Make WebViewLibraryLoader interface more flexible. Pass the library file name to WebViewLibraryLoader.loadNativeLibrary instead of passing the ApplicationInfo, which was only used to look the native library file name up. This will allow loadNativeLibrary to be called from the webview zygote, which doesn't have the ApplicationInfo available. Bug: 63749735 Test: CtsWebkitTests Change-Id: I0bd2de38cd1a0112bbeee6225563d4d0add048e7 --- core/java/android/webkit/WebViewLibraryLoader.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'core/java/android/webkit/WebViewLibraryLoader.java') diff --git a/core/java/android/webkit/WebViewLibraryLoader.java b/core/java/android/webkit/WebViewLibraryLoader.java index eb2b6bccf4cc..cabba06bdff5 100644 --- a/core/java/android/webkit/WebViewLibraryLoader.java +++ b/core/java/android/webkit/WebViewLibraryLoader.java @@ -234,17 +234,14 @@ public class WebViewLibraryLoader { *

Note: Assumes that we have waited for relro creation. * * @param clazzLoader class loader used to find the linker namespace to load the library into. - * @param packageInfo the package from which WebView is loaded. + * @param libraryFileName the filename of the library to load. */ - static int loadNativeLibrary(ClassLoader clazzLoader, PackageInfo packageInfo) - throws WebViewFactory.MissingWebViewPackageException { + public static int loadNativeLibrary(ClassLoader clazzLoader, String libraryFileName) { if (!sAddressSpaceReserved) { Log.e(LOGTAG, "can't load with relro file; address space not reserved"); return WebViewFactory.LIBLOAD_ADDRESS_SPACE_NOT_RESERVED; } - final String libraryFileName = - WebViewFactory.getWebViewLibrary(packageInfo.applicationInfo); String relroPath = VMRuntime.getRuntime().is64Bit() ? CHROMIUM_WEBVIEW_NATIVE_RELRO_64 : CHROMIUM_WEBVIEW_NATIVE_RELRO_32; int result = nativeLoadWithRelroFile(libraryFileName, relroPath, clazzLoader); -- cgit v1.2.3