Commit 77da206d authored by Andrew Grieve's avatar Andrew Grieve Committed by Commit Bot

Android: Make NativeLibraries.java fields final

I was hoping this would remove
NativeLibraries.getTestRunnerClassNameForTesting()
but it doesn't look like it does :(.

Still - this way of creating the file brings it in line with how
we create R.java and BuildConfig.java.

Bug: 984573
Change-Id: Iebeaa9970ac9c47ea13f81f05b71d5ccb5247205
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1704315
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678569}
parent ab14c6bc
...@@ -3504,6 +3504,7 @@ if (is_android) { ...@@ -3504,6 +3504,7 @@ if (is_android) {
} }
write_native_libraries_java("base_native_libraries_gen") { write_native_libraries_java("base_native_libraries_gen") {
use_final_fields = false
} }
android_library("base_java_unittest_support") { android_library("base_java_unittest_support") {
......
...@@ -10,61 +10,21 @@ NATIVE_LIBRARIES_TEMPLATE = """\ ...@@ -10,61 +10,21 @@ NATIVE_LIBRARIES_TEMPLATE = """\
package org.chromium.base.library_loader; package org.chromium.base.library_loader;
public class NativeLibraries {{ public class NativeLibraries {{
/**
* IMPORTANT NOTE: The variables defined here must _not_ be 'final'.
*
* The reason for this is very subtle:
*
* - This template is used to generate several distinct, but similar
* files used in different contexts:
*
* o .../gen/templates/org/chromium/base/library_loader/NativeLibraries.java
*
* This file is used to build base.jar, which is the library
* jar used by chromium projects. However, the
* corresponding NativeLibraries.class file will _not_ be part
* of the final base.jar.
*
* o .../$PROJECT/native_libraries_java/NativeLibraries.java
*
* This file is used to build an APK (e.g. $PROJECT
* could be 'content_shell_apk'). Its content will depend on
* this target's specific build configuration, and differ from
* the source file above.
*
* - During the final link, all .jar files are linked together into
* a single .dex file, and the second version of NativeLibraries.class
* will be put into the final output file, and used at runtime.
*
* - If the variables were defined as 'final', their value would be
* optimized out inside of 'base.jar', and could not be specialized
* for every chromium program. This, however, doesn't apply to arrays of
* strings, which can be defined as final.
*
* This exotic scheme is used to avoid injecting project-specific, or
* even build-specific, values into the base layer. E.g. this is
* how the component build is supported on Android without modifying
* the sources of each and every Chromium-based target.
*/
public static final int CPU_FAMILY_UNKNOWN = 0; public static final int CPU_FAMILY_UNKNOWN = 0;
public static final int CPU_FAMILY_ARM = 1; public static final int CPU_FAMILY_ARM = 1;
public static final int CPU_FAMILY_MIPS = 2; public static final int CPU_FAMILY_MIPS = 2;
public static final int CPU_FAMILY_X86 = 3; public static final int CPU_FAMILY_X86 = 3;
// Set to true to enable the use of the Chromium Linker. // Set to true to enable the use of the Chromium Linker.
public static boolean sUseLinker{USE_LINKER}; public static {MAYBE_FINAL}boolean sUseLinker{USE_LINKER};
public static boolean sUseLibraryInZipFile{USE_LIBRARY_IN_ZIP_FILE}; public static {MAYBE_FINAL}boolean sUseLibraryInZipFile{USE_LIBRARY_IN_ZIP_FILE};
public static boolean sEnableLinkerTests{ENABLE_LINKER_TESTS}; public static {MAYBE_FINAL}boolean sEnableLinkerTests{ENABLE_LINKER_TESTS};
// This is the list of native libraries to be loaded (in the correct order) // This is the list of native libraries to be loaded (in the correct order)
// by LibraryLoader.java. The base java library is compiled with no // by LibraryLoader.java.
// array defined, and then the build system creates a version of the file
// with the real list of libraries required (which changes based upon which
// .apk is being built).
// TODO(cjhopman): This is public since it is referenced by NativeTestActivity.java // TODO(cjhopman): This is public since it is referenced by NativeTestActivity.java
// directly. The two ways of library loading should be refactored into one. // directly. The two ways of library loading should be refactored into one.
public static final String[] LIBRARIES = {LIBRARIES}; public static {MAYBE_FINAL}String[] LIBRARIES = {LIBRARIES};
// This is the expected version of the 'main' native library, which is the one that // This is the expected version of the 'main' native library, which is the one that
// implements the initial set of base JNI functions including // implements the initial set of base JNI functions including
...@@ -72,8 +32,8 @@ public class NativeLibraries {{ ...@@ -72,8 +32,8 @@ public class NativeLibraries {{
// TODO(torne): This is public to work around classloader issues in Trichrome // TODO(torne): This is public to work around classloader issues in Trichrome
// where NativeLibraries is not in the same dex as LibraryLoader. // where NativeLibraries is not in the same dex as LibraryLoader.
// We should instead split up Java code along package boundaries. // We should instead split up Java code along package boundaries.
public static String sVersionNumber = {VERSION_NUMBER}; public static {MAYBE_FINAL}String sVersionNumber = {VERSION_NUMBER};
public static int sCpuFamily = {CPU_FAMILY}; public static {MAYBE_FINAL}int sCpuFamily = {CPU_FAMILY};
}} }}
""" """
...@@ -15,13 +15,10 @@ from native_libraries_template import NATIVE_LIBRARIES_TEMPLATE ...@@ -15,13 +15,10 @@ from native_libraries_template import NATIVE_LIBRARIES_TEMPLATE
from util import build_utils from util import build_utils
def _GetFormattedBoolean(value):
return ' = true' if value else ''
def main(): def main():
parser = argparse.ArgumentParser() parser = argparse.ArgumentParser()
parser.add_argument('--final', action='store_true', help='Use final fields.')
parser.add_argument( parser.add_argument(
'--enable-chromium-linker', '--enable-chromium-linker',
action='store_true', action='store_true',
...@@ -77,19 +74,21 @@ def main(): ...@@ -77,19 +74,21 @@ def main():
native_libraries_list = ( native_libraries_list = (
'{%s}' % ','.join(['"%s"' % s[3:-3] for s in lib_paths])) '{%s}' % ','.join(['"%s"' % s[3:-3] for s in lib_paths]))
def bool_str(value):
if value:
return ' = true'
elif options.final:
return ' = false'
return ''
format_dict = { format_dict = {
'USE_LINKER': 'MAYBE_FINAL': 'final ' if options.final else '',
_GetFormattedBoolean(options.enable_chromium_linker), 'USE_LINKER': bool_str(options.enable_chromium_linker),
'USE_LIBRARY_IN_ZIP_FILE': 'USE_LIBRARY_IN_ZIP_FILE': bool_str(options.load_library_from_apk),
_GetFormattedBoolean(options.load_library_from_apk), 'ENABLE_LINKER_TESTS': bool_str(options.enable_chromium_linker_tests),
'ENABLE_LINKER_TESTS': 'LIBRARIES': native_libraries_list,
_GetFormattedBoolean(options.enable_chromium_linker_tests), 'VERSION_NUMBER': options.version_number,
'LIBRARIES': 'CPU_FAMILY': options.cpu_family,
native_libraries_list,
'VERSION_NUMBER':
options.version_number,
'CPU_FAMILY':
options.cpu_family,
} }
with build_utils.AtomicOutput(options.output) as f: with build_utils.AtomicOutput(options.output) as f:
with zipfile.ZipFile(f.name, 'w') as srcjar_file: with zipfile.ZipFile(f.name, 'w') as srcjar_file:
......
...@@ -722,6 +722,8 @@ if (enable_java_templates) { ...@@ -722,6 +722,8 @@ if (enable_java_templates) {
# enable_chromium_linker_tests: (Optional) Whether to run tests. # enable_chromium_linker_tests: (Optional) Whether to run tests.
# dont_load_shared_libraries: (Optional) List of native libraries to exclude # dont_load_shared_libraries: (Optional) List of native libraries to exclude
# from output. # from output.
# use_final_fields: True to use final fields. When false, all other
# variables must not be set.
template("write_native_libraries_java") { template("write_native_libraries_java") {
_native_libraries_file = "$target_gen_dir/$target_name.srcjar" _native_libraries_file = "$target_gen_dir/$target_name.srcjar"
if (current_cpu == "arm" || current_cpu == "arm64") { if (current_cpu == "arm" || current_cpu == "arm64") {
...@@ -750,38 +752,39 @@ if (enable_java_templates) { ...@@ -750,38 +752,39 @@ if (enable_java_templates) {
"--cpu-family", "--cpu-family",
_cpu_family, _cpu_family,
] ]
if (defined(invoker.native_libraries_list_file)) { if (invoker.use_final_fields) {
args += [ args += [
"--final",
"--native-libraries-list", "--native-libraries-list",
rebase_path(invoker.native_libraries_list_file, root_build_dir), rebase_path(invoker.native_libraries_list_file, root_build_dir),
] ]
inputs = [ inputs = [
invoker.native_libraries_list_file, invoker.native_libraries_list_file,
] ]
} if (defined(invoker.version_number)) {
if (defined(invoker.version_number)) { args += [
args += [ "--version-number",
"--version-number", invoker.version_number,
invoker.version_number, ]
] }
} if (defined(invoker.enable_chromium_linker) &&
if (defined(invoker.enable_chromium_linker) && invoker.enable_chromium_linker) {
invoker.enable_chromium_linker) { args += [ "--enable-chromium-linker" ]
args += [ "--enable-chromium-linker" ] }
} if (defined(invoker.load_library_from_apk) &&
if (defined(invoker.load_library_from_apk) && invoker.load_library_from_apk) {
invoker.load_library_from_apk) { args += [ "--load-library-from-apk" ]
args += [ "--load-library-from-apk" ] }
} if (defined(invoker.enable_chromium_linker_tests) &&
if (defined(invoker.enable_chromium_linker_tests) && invoker.enable_chromium_linker_tests) {
invoker.enable_chromium_linker_tests) { args += [ "--enable-chromium-linker-tests" ]
args += [ "--enable-chromium-linker-tests" ] }
}
# TODO(tiborg): Remove once all usages are removed. # TODO(tiborg): Remove once all usages are removed.
if (defined(invoker.dont_load_shared_libraries)) { if (defined(invoker.dont_load_shared_libraries)) {
args += [ "--exclude-native-libraries=" + args += [ "--exclude-native-libraries=" +
invoker.dont_load_shared_libraries ] invoker.dont_load_shared_libraries ]
}
} }
} }
} }
...@@ -1940,8 +1943,8 @@ if (enable_java_templates) { ...@@ -1940,8 +1943,8 @@ if (enable_java_templates) {
# apks that depend on //base:base_java. # apks that depend on //base:base_java.
# #
# Variables: # Variables:
# use_final_fields: True to use final fields. All other variables are # use_final_fields: True to use final fields. When false, all other
# ignored when this is false. # variables must not be set.
# enable_multidex: Value for ENABLE_MULTIDEX. # enable_multidex: Value for ENABLE_MULTIDEX.
# min_sdk_version: Value for MIN_SDK_VERSION. # min_sdk_version: Value for MIN_SDK_VERSION.
# #
...@@ -2663,6 +2666,7 @@ if (enable_java_templates) { ...@@ -2663,6 +2666,7 @@ if (enable_java_templates) {
} }
enable_chromium_linker = _use_chromium_linker enable_chromium_linker = _use_chromium_linker
load_library_from_apk = _load_library_from_apk load_library_from_apk = _load_library_from_apk
use_final_fields = true
} }
_srcjar_deps += [ ":${_template_name}__native_libraries" ] _srcjar_deps += [ ":${_template_name}__native_libraries" ]
} }
......
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