Commit f6e84797 authored by Findit's avatar Findit

Revert "Android: Make Linker.java optimized out when use_chromium_linker=false"

This reverts commit 2f2b401b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 692146 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzJmMmI0MDFiZjQwMTdjNTA3ZDgyYTNlNjUyZmFjMTMzYWY3MTk2NGQM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/android-archive-rel/4128

Sample Failed Step: compile

Original change's description:
> Android: Make Linker.java optimized out when use_chromium_linker=false
> 
> Two fixes:
> 1) Move static getters to a separate LibraryLoaderConfig.java
>    * Required because of the non-trivial class initializer in
>      LibraryLoader preventing inlining.
> 2) Change JNI proguard rules to not -keep natives when the linker
>    is not used.
> 
> TBR=agrieve  # Renames
> 
> Change-Id: I0f4a40af6168cbe7243f462c24d230d277e642ab
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1703155
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Sam Maier <smaier@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#692146}


Change-Id: Icfe9c719c7f1fdfa7b95acc41c98319972d50abb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1779168
Cr-Commit-Position: refs/heads/master@{#692197}
parent 1c9d674b
...@@ -39,6 +39,10 @@ ...@@ -39,6 +39,10 @@
-keep class com.android.webview.chromium.DrawGLFunctor -keep class com.android.webview.chromium.DrawGLFunctor
-keep class com.android.webview.chromium.GraphicsUtils -keep class com.android.webview.chromium.GraphicsUtils
# Linker dynamically casts to $TestRunner when running tests. We don't run these
# tests in WebView.
-dontnote org.chromium.base.library_loader.Linker$TestRunner
# Don't note about the API 21 compatibility code which references various # Don't note about the API 21 compatibility code which references various
# hidden APIs via reflection. # hidden APIs via reflection.
-dontnote com.android.webview.chromium.WebViewDelegateFactory$Api21CompatibilityDelegate -dontnote com.android.webview.chromium.WebViewDelegateFactory$Api21CompatibilityDelegate
......
...@@ -3258,7 +3258,6 @@ if (is_android) { ...@@ -3258,7 +3258,6 @@ if (is_android) {
"android/java/src/org/chromium/base/compat/ApiHelperForP.java", "android/java/src/org/chromium/base/compat/ApiHelperForP.java",
"android/java/src/org/chromium/base/library_loader/LegacyLinker.java", "android/java/src/org/chromium/base/library_loader/LegacyLinker.java",
"android/java/src/org/chromium/base/library_loader/LibraryLoader.java", "android/java/src/org/chromium/base/library_loader/LibraryLoader.java",
"android/java/src/org/chromium/base/library_loader/LibraryLoaderConfig.java",
"android/java/src/org/chromium/base/library_loader/LibraryPrefetcher.java", "android/java/src/org/chromium/base/library_loader/LibraryPrefetcher.java",
"android/java/src/org/chromium/base/library_loader/Linker.java", "android/java/src/org/chromium/base/library_loader/Linker.java",
"android/java/src/org/chromium/base/library_loader/LoaderErrors.java", "android/java/src/org/chromium/base/library_loader/LoaderErrors.java",
......
...@@ -191,7 +191,7 @@ public class LibraryLoader { ...@@ -191,7 +191,7 @@ public class LibraryLoader {
*/ */
public void preloadNowOverrideApplicationContext(Context appContext) { public void preloadNowOverrideApplicationContext(Context appContext) {
synchronized (mLock) { synchronized (mLock) {
if (LibraryLoaderConfig.useChromiumLinker()) return; if (useChromiumLinker()) return;
preloadAlreadyLocked(appContext.getApplicationInfo()); preloadAlreadyLocked(appContext.getApplicationInfo());
} }
} }
...@@ -199,7 +199,7 @@ public class LibraryLoader { ...@@ -199,7 +199,7 @@ public class LibraryLoader {
private void preloadAlreadyLocked(ApplicationInfo appInfo) { private void preloadAlreadyLocked(ApplicationInfo appInfo) {
try (TraceEvent te = TraceEvent.scoped("LibraryLoader.preloadAlreadyLocked")) { try (TraceEvent te = TraceEvent.scoped("LibraryLoader.preloadAlreadyLocked")) {
// Preloader uses system linker, we shouldn't preload if Chromium linker is used. // Preloader uses system linker, we shouldn't preload if Chromium linker is used.
assert !LibraryLoaderConfig.useChromiumLinker(); assert !useChromiumLinker();
if (mLibraryPreloader != null && !mLibraryPreloaderCalled) { if (mLibraryPreloader != null && !mLibraryPreloaderCalled) {
mLibraryPreloader.loadLibrary(appInfo); mLibraryPreloader.loadLibrary(appInfo);
mLibraryPreloaderCalled = true; mLibraryPreloaderCalled = true;
...@@ -340,7 +340,7 @@ public class LibraryLoader { ...@@ -340,7 +340,7 @@ public class LibraryLoader {
long startTime = SystemClock.uptimeMillis(); long startTime = SystemClock.uptimeMillis();
if (LibraryLoaderConfig.useChromiumLinker() && !inZygote) { if (useChromiumLinker() && !inZygote) {
Linker linker = Linker.getInstance(); Linker linker = Linker.getInstance();
// See base/android/linker/config.gni, the chromium linker is only enabled when we // See base/android/linker/config.gni, the chromium linker is only enabled when we
...@@ -559,7 +559,7 @@ public class LibraryLoader { ...@@ -559,7 +559,7 @@ public class LibraryLoader {
// Called after all native initializations are complete. // Called after all native initializations are complete.
public void onBrowserNativeInitializationComplete() { public void onBrowserNativeInitializationComplete() {
synchronized (mLock) { synchronized (mLock) {
if (LibraryLoaderConfig.useChromiumLinker()) { if (useChromiumLinker()) {
RecordHistogram.recordTimesHistogram( RecordHistogram.recordTimesHistogram(
"ChromiumAndroidLinker.BrowserLoadTime", mLibraryLoadTimeMs); "ChromiumAndroidLinker.BrowserLoadTime", mLibraryLoadTimeMs);
} }
...@@ -572,12 +572,21 @@ public class LibraryLoader { ...@@ -572,12 +572,21 @@ public class LibraryLoader {
// RecordChromiumAndroidLinkerRendererHistogram() will record it correctly. // RecordChromiumAndroidLinkerRendererHistogram() will record it correctly.
public void registerRendererProcessHistogram() { public void registerRendererProcessHistogram() {
synchronized (mLock) { synchronized (mLock) {
if (LibraryLoaderConfig.useChromiumLinker()) { if (useChromiumLinker()) {
LibraryLoaderJni.get().recordRendererLibraryLoadTime(mLibraryLoadTimeMs); LibraryLoaderJni.get().recordRendererLibraryLoadTime(mLibraryLoadTimeMs);
} }
} }
} }
/**
* Call this method to determine if this chromium project must
* use this linker. If not, System.loadLibrary() should be used to load
* libraries instead.
*/
public static boolean useChromiumLinker() {
return NativeLibraries.sUseLinker;
}
/** /**
* Override the library loader (normally with a mock) for testing. * Override the library loader (normally with a mock) for testing.
* @param loader the mock library loader. * @param loader the mock library loader.
......
// Copyright 2014 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.base.library_loader;
/**
* Build-time configuration of LibraryLoader.
* These are in a separate class from LibraryLoader to ensure that they are inlined.
*/
public class LibraryLoaderConfig {
private LibraryLoaderConfig() {}
/**
* Check that native library linker tests are enabled.
* If not enabled, calls to testing functions will fail with an assertion
* error.
*
* @return true if native library linker tests are enabled.
*/
public static boolean areTestsEnabled() {
return NativeLibraries.sEnableLinkerTests;
}
/**
* Call this method to determine if this chromium project must
* use this linker. If not, System.loadLibrary() should be used to load
* libraries instead.
*/
public static boolean useChromiumLinker() {
return NativeLibraries.sUseLinker;
}
}
...@@ -600,6 +600,16 @@ public abstract class Linker { ...@@ -600,6 +600,16 @@ public abstract class Linker {
} }
/* ---------------------- Testing support methods. ---------------------- */ /* ---------------------- Testing support methods. ---------------------- */
/**
* Check that native library linker tests are enabled.
* If not enabled, calls to testing functions will fail with an assertion
* error.
*
* @return true if native library linker tests are enabled.
*/
public static boolean areTestsEnabled() {
return NativeLibraries.sEnableLinkerTests;
}
/** /**
* Get Linker implementation type. * Get Linker implementation type.
......
...@@ -38,7 +38,7 @@ ...@@ -38,7 +38,7 @@
-keepclasseswithmembers class ** { -keepclasseswithmembers class ** {
@org.chromium.base.annotations.UsedByReflection <fields>; @org.chromium.base.annotations.UsedByReflection <fields>;
} }
-keepclasseswithmembers,includedescriptorclasses class !org.chromium.base.library_loader.**,** { -keepclasseswithmembers,includedescriptorclasses class ** {
native <methods>; native <methods>;
} }
......
# Copyright 2019 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.
-keepclasseswithmembers,includedescriptorclasses class org.chromium.base.library_loader.** {
native <methods>;
}
# Copyright 2019 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.
# All references to Linker.getInstance() should be guarded by:
# LibraryLoaderConfig.useCrazyLinker().
-checkdiscard class org.chromium.base.library_loader.Linker {
*;
}
...@@ -2813,13 +2813,6 @@ if (enable_java_templates) { ...@@ -2813,13 +2813,6 @@ if (enable_java_templates) {
if (defined(invoker.proguard_configs)) { if (defined(invoker.proguard_configs)) {
proguard_configs += invoker.proguard_configs proguard_configs += invoker.proguard_configs
} }
if (_use_chromium_linker) {
proguard_configs +=
[ "//base/android/proguard/chromium_linker.flags" ]
} else {
proguard_configs +=
[ "//base/android/proguard/no_chromium_linker.flags" ]
}
if (_enable_main_dex_list) { if (_enable_main_dex_list) {
proguard_configs += [ "//build/android/multidex.flags" ] proguard_configs += [ "//build/android/multidex.flags" ]
} }
......
...@@ -241,20 +241,6 @@ template("chrome_public_common_apk_or_module_tmpl") { ...@@ -241,20 +241,6 @@ template("chrome_public_common_apk_or_module_tmpl") {
srcjar_deps = [ "//components/module_installer/android:module_installer_apk_build_config" ] srcjar_deps = [ "//components/module_installer/android:module_installer_apk_build_config" ]
} }
if (!defined(use_chromium_linker)) {
use_chromium_linker = chromium_linker_supported
}
if (use_chromium_linker) {
if (!defined(load_library_from_apk)) {
# Whether native libraries should be loaded from within the apk.
# Only attempt loading the library from the APK for 64 bit devices
# until the number of 32 bit devices which don't support this
# approach falls to a minimal level - http://crbug.com/390618.
load_library_from_apk = chromium_linker_supported &&
(current_cpu == "arm64" || current_cpu == "x64")
}
}
if (!is_java_debug) { if (!is_java_debug) {
proguard_enabled = true proguard_enabled = true
if (!defined(proguard_configs)) { if (!defined(proguard_configs)) {
...@@ -274,6 +260,20 @@ template("chrome_public_common_apk_or_module_tmpl") { ...@@ -274,6 +260,20 @@ template("chrome_public_common_apk_or_module_tmpl") {
} }
} }
if (!defined(use_chromium_linker)) {
use_chromium_linker = chromium_linker_supported
}
if (use_chromium_linker) {
if (!defined(load_library_from_apk)) {
# Whether native libraries should be loaded from within the apk.
# Only attempt loading the library from the APK for 64 bit devices
# until the number of 32 bit devices which don't support this
# approach falls to a minimal level - http://crbug.com/390618.
load_library_from_apk = chromium_linker_supported &&
(current_cpu == "arm64" || current_cpu == "x64")
}
}
if (_target_type == "android_apk") { if (_target_type == "android_apk") {
command_line_flags_file = "chrome-command-line" command_line_flags_file = "chrome-command-line"
} else { } else {
......
...@@ -42,6 +42,10 @@ ...@@ -42,6 +42,10 @@
-keep class com.android.webview.chromium.DrawGLFunctor -keep class com.android.webview.chromium.DrawGLFunctor
-keep class com.android.webview.chromium.GraphicsUtils -keep class com.android.webview.chromium.GraphicsUtils
# Linker dynamically casts to $TestRunner when running tests. We don't run these
# tests in WebView.
-dontnote org.chromium.base.library_loader.Linker$TestRunner
# Don't note about the API 21 compatibility code which references various # Don't note about the API 21 compatibility code which references various
# hidden APIs via reflection. # hidden APIs via reflection.
-dontnote com.android.webview.chromium.WebViewDelegateFactory$Api21CompatibilityDelegate -dontnote com.android.webview.chromium.WebViewDelegateFactory$Api21CompatibilityDelegate
...@@ -207,7 +211,7 @@ ...@@ -207,7 +211,7 @@
-keepclasseswithmembers class ** { -keepclasseswithmembers class ** {
@org.chromium.base.annotations.UsedByReflection <fields>; @org.chromium.base.annotations.UsedByReflection <fields>;
} }
-keepclasseswithmembers,includedescriptorclasses class !org.chromium.base.library_loader.**,** { -keepclasseswithmembers,includedescriptorclasses class ** {
native <methods>; native <methods>;
} }
...@@ -269,19 +273,6 @@ ...@@ -269,19 +273,6 @@
-renamesourcefileattribute PG -renamesourcefileattribute PG
-repackageclasses '' -repackageclasses ''
################################################################################
# ../../base/android/proguard/no_chromium_linker.flags
################################################################################
# Copyright 2019 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.
# All references to Linker.getInstance() should be guarded by:
# LibraryLoaderConfig.useCrazyLinker().
-checkdiscard class org.chromium.base.library_loader.Linker {
*;
}
################################################################################ ################################################################################
# ../../build/android/buildhooks/proguard/build_hooks_android_impl.flags # ../../build/android/buildhooks/proguard/build_hooks_android_impl.flags
################################################################################ ################################################################################
......
...@@ -19,7 +19,7 @@ import org.junit.runner.RunWith; ...@@ -19,7 +19,7 @@ import org.junit.runner.RunWith;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.compat.ApiHelperForM; import org.chromium.base.compat.ApiHelperForM;
import org.chromium.base.library_loader.LibraryLoaderConfig; import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.LoadStatusRecorder.LoadLibraryStatus; import org.chromium.base.library_loader.LoadStatusRecorder.LoadLibraryStatus;
import org.chromium.base.metrics.RecordHistogram; import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
...@@ -159,7 +159,7 @@ public class StartupLoadingMetricsTest { ...@@ -159,7 +159,7 @@ public class StartupLoadingMetricsTest {
assertHistogramsRecorded(1, TABBED_SUFFIX); assertHistogramsRecorded(1, TABBED_SUFFIX);
// LibraryLoader checks. // LibraryLoader checks.
if (!LibraryLoaderConfig.useChromiumLinker()) { if (!LibraryLoader.useChromiumLinker()) {
Log.w(TAG, "Skipping test because not using ChromiumLinker."); Log.w(TAG, "Skipping test because not using ChromiumLinker.");
return; return;
} }
......
...@@ -20,7 +20,6 @@ import org.chromium.base.annotations.JNINamespace; ...@@ -20,7 +20,6 @@ import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.MainDex; import org.chromium.base.annotations.MainDex;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.library_loader.LibraryLoader; import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.LibraryLoaderConfig;
import org.chromium.base.library_loader.Linker; import org.chromium.base.library_loader.Linker;
import org.chromium.base.library_loader.ProcessInitException; import org.chromium.base.library_loader.ProcessInitException;
import org.chromium.base.memory.MemoryPressureUma; import org.chromium.base.memory.MemoryPressureUma;
...@@ -83,8 +82,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe ...@@ -83,8 +82,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
mCpuFeatures = connectionBundle.getLong(ContentChildProcessConstants.EXTRA_CPU_FEATURES); mCpuFeatures = connectionBundle.getLong(ContentChildProcessConstants.EXTRA_CPU_FEATURES);
assert mCpuCount > 0; assert mCpuCount > 0;
if (LibraryLoaderConfig.useChromiumLinker() if (LibraryLoader.useChromiumLinker() && !LibraryLoader.getInstance().isLoadedByZygote()) {
&& !LibraryLoader.getInstance().isLoadedByZygote()) {
Bundle sharedRelros = connectionBundle.getBundle(Linker.EXTRA_LINKER_SHARED_RELROS); Bundle sharedRelros = connectionBundle.getBundle(Linker.EXTRA_LINKER_SHARED_RELROS);
if (sharedRelros != null) getLinker().provideSharedRelros(sharedRelros); if (sharedRelros != null) getLinker().provideSharedRelros(sharedRelros);
} }
...@@ -108,7 +106,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe ...@@ -108,7 +106,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
Linker linker = null; Linker linker = null;
boolean requestedSharedRelro = false; boolean requestedSharedRelro = false;
if (LibraryLoaderConfig.useChromiumLinker()) { if (LibraryLoader.useChromiumLinker()) {
assert mLinkerParams != null; assert mLinkerParams != null;
linker = getLinker(); linker = getLinker();
if (mLinkerParams.mWaitForSharedRelro) { if (mLinkerParams.mWaitForSharedRelro) {
...@@ -186,7 +184,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe ...@@ -186,7 +184,7 @@ public class ContentChildProcessServiceDelegate implements ChildProcessServiceDe
// Return a Linker instance. If testing, the Linker needs special setup. // Return a Linker instance. If testing, the Linker needs special setup.
private Linker getLinker() { private Linker getLinker() {
if (LibraryLoaderConfig.areTestsEnabled()) { if (Linker.areTestsEnabled()) {
// For testing, set the Linker implementation and the test runner // For testing, set the Linker implementation and the test runner
// class name to match those used by the parent. // class name to match those used by the parent.
assert mLinkerParams != null; assert mLinkerParams != null;
......
...@@ -24,7 +24,7 @@ import org.chromium.base.VisibleForTesting; ...@@ -24,7 +24,7 @@ import org.chromium.base.VisibleForTesting;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace; import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods; import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.library_loader.LibraryLoaderConfig; import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.library_loader.Linker; import org.chromium.base.library_loader.Linker;
import org.chromium.base.process_launcher.ChildConnectionAllocator; import org.chromium.base.process_launcher.ChildConnectionAllocator;
import org.chromium.base.process_launcher.ChildProcessConnection; import org.chromium.base.process_launcher.ChildProcessConnection;
...@@ -133,7 +133,7 @@ public final class ChildProcessLauncherHelperImpl { ...@@ -133,7 +133,7 @@ public final class ChildProcessLauncherHelperImpl {
ContentChildProcessConstants.EXTRA_CPU_COUNT, CpuFeatures.getCount()); ContentChildProcessConstants.EXTRA_CPU_COUNT, CpuFeatures.getCount());
connectionBundle.putLong( connectionBundle.putLong(
ContentChildProcessConstants.EXTRA_CPU_FEATURES, CpuFeatures.getMask()); ContentChildProcessConstants.EXTRA_CPU_FEATURES, CpuFeatures.getMask());
if (LibraryLoaderConfig.useChromiumLinker()) { if (LibraryLoader.useChromiumLinker()) {
connectionBundle.putBundle(Linker.EXTRA_LINKER_SHARED_RELROS, connectionBundle.putBundle(Linker.EXTRA_LINKER_SHARED_RELROS,
Linker.getInstance().getSharedRelros()); Linker.getInstance().getSharedRelros());
} }
...@@ -594,7 +594,7 @@ public final class ChildProcessLauncherHelperImpl { ...@@ -594,7 +594,7 @@ public final class ChildProcessLauncherHelperImpl {
private static void initLinker() { private static void initLinker() {
assert LauncherThread.runningOnLauncherThread(); assert LauncherThread.runningOnLauncherThread();
if (sLinkerInitialized) return; if (sLinkerInitialized) return;
if (LibraryLoaderConfig.useChromiumLinker()) { if (LibraryLoader.useChromiumLinker()) {
sLinkerLoadAddress = Linker.getInstance().getBaseLoadAddress(); sLinkerLoadAddress = Linker.getInstance().getBaseLoadAddress();
if (sLinkerLoadAddress == 0) { if (sLinkerLoadAddress == 0) {
Log.i(TAG, "Shared RELRO support disabled!"); Log.i(TAG, "Shared RELRO support disabled!");
...@@ -612,7 +612,7 @@ public final class ChildProcessLauncherHelperImpl { ...@@ -612,7 +612,7 @@ public final class ChildProcessLauncherHelperImpl {
// Always wait for the shared RELROs in service processes. // Always wait for the shared RELROs in service processes.
final boolean waitForSharedRelros = true; final boolean waitForSharedRelros = true;
if (LibraryLoaderConfig.areTestsEnabled()) { if (Linker.areTestsEnabled()) {
Linker linker = Linker.getInstance(); Linker linker = Linker.getInstance();
return new ChromiumLinkerParams(sLinkerLoadAddress, waitForSharedRelros, return new ChromiumLinkerParams(sLinkerLoadAddress, waitForSharedRelros,
linker.getTestRunnerClassNameForTesting(), linker.getTestRunnerClassNameForTesting(),
......
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