JNI: make JNI Multiplexing work (behind flag)

Now, the gn arg "allow_jni_multiplexing" turns on a functional, if not ideally optimized, jni multiplexing implementation. There is still some work that is needed to improve the multiplexing before we enable globally. Bug: 929682 Change-Id: Ia4275bc32fd2bbca58fb3c8ff4a712b0883446a8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3872586 Reviewed-by: Nate Fischer <ntfschr@chromium.org> Commit-Queue: Sam Maier <smaier@chromium.org> Reviewed-by: Andrew Grieve <agrieve@chromium.org> Auto-Submit: Sam Maier <smaier@chromium.org> Commit-Queue: Nate Fischer <ntfschr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1100141}
qweri0p · Feb 1, 2023 · 49c3876 · 49c3876
1 parent f92ef38
commit 49c3876
Show file tree

Hide file tree

Showing 18 changed files with 122 additions and 25 deletions.
diff --git a/android_webview/BUILD.gn b/android_webview/BUILD.gn
@@ -90,7 +90,9 @@ standalone_system_webview_apk_tmpl("system_webview_no_weblayer_apk") {
       [ "//third_party/androidx:androidx_recyclerview_recyclerview_java" ]
 }
 
+_main_webview_bundle_target = ":system_webview_bundle"
 if (android_64bit_target_cpu && skip_secondary_abi_for_cq) {
+  _main_webview_bundle_target = ":system_webview_64_bundle"
   group("system_webview_bundle") {
     deps = [ ":system_webview_64_bundle" ]
   }
@@ -389,6 +391,13 @@ if (android_64bit_target_cpu && !skip_secondary_abi_for_cq) {
   }
 }
 
+if (allow_jni_multiplexing && current_toolchain == default_toolchain) {
+  generate_jni_registration("webview_jni_registration") {
+    targets = [ _main_webview_bundle_target ]
+    enable_jni_multiplexing = true
+  }
+}
+
 # The shared library used by standalone WebView.
 template("libwebviewchromium_tmpl") {
   shared_library(target_name) {
@@ -399,6 +408,9 @@ template("libwebviewchromium_tmpl") {
       "//android_webview/nonembedded",
       "//third_party/blink/public:blink",
     ]
+    if (allow_jni_multiplexing) {
+      deps += [ ":webview_jni_registration($default_toolchain)" ]
+    }
     configs -= [ "//build/config/android:hide_all_but_jni_onload" ]
     configs += [
       "//build/config/android:hide_all_but_jni",
@@ -430,6 +442,10 @@ template("webview_alternate_library") {
       "//android_webview/lib:webview_entry_point",
       "//android_webview/nonembedded",
     ]
+    if (allow_jni_multiplexing) {
+      deps += [ ":webview_jni_registration($default_toolchain)" ]
+      enable_jni_multiplexing = true
+    }
     is_webview = true
   }
 }

diff --git a/android_webview/lib/BUILD.gn b/android_webview/lib/BUILD.gn
@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/android/config.gni")
 import("//components/gwp_asan/buildflags/buildflags.gni")
 import("//components/spellcheck/spellcheck_build_features.gni")
 import("//weblayer/variables.gni")
@@ -68,9 +69,15 @@ source_set("webview_entry_point") {
     "//base",
   ]
   sources = [ "webview_entry_point.cc" ]
+  defines = []
+
+  if (allow_jni_multiplexing) {
+    defines += [ "JNI_REGISTRATION_REQUIRED" ]
+    deps += [ "//android_webview:webview_jni_registration($default_toolchain)" ]
+  }
 
   if (webview_includes_weblayer) {
-    defines = [ "WEBVIEW_INCLUDES_WEBLAYER" ]
+    defines += [ "WEBVIEW_INCLUDES_WEBLAYER" ]
     deps += [ "//weblayer:weblayer_lib_webview" ]
   }
 }
diff --git a/android_webview/lib/webview_entry_point.cc b/android_webview/lib/webview_entry_point.cc
@@ -7,6 +7,9 @@
 #include "base/android/jni_android.h"
 #include "base/android/library_loader/library_loader_hooks.h"
 
+#if defined(JNI_REGISTRATION_REQUIRED)
+#include "android_webview/webview_jni_registration_generated.h"
+#endif
 #if defined(WEBVIEW_INCLUDES_WEBLAYER)
 #include "weblayer/app/jni_onload.h"
 #include "weblayer/browser/web_view_compatibility_helper_impl.h"

diff --git a/android_webview/system_webview_apk_tmpl.gni b/android_webview/system_webview_apk_tmpl.gni
@@ -101,6 +101,9 @@ template("system_webview_apk_or_module_tmpl") {
     if (!_omit_dex) {
       product_config_java_packages =
           [ invoker.webview_product_config_java_package ]
+      if (allow_jni_multiplexing) {
+        enable_jni_multiplexing = true
+      }
     }
 
     if (_webview_includes_weblayer) {

diff --git a/base/android/jni_generator/BUILD.gn b/base/android/jni_generator/BUILD.gn
@@ -48,12 +48,17 @@ source_set("jni_sample_native_side") {
   ]
 }
 
+generate_jni_registration("jni_registration") {
+  targets = [ ":jni_sample_java" ]
+  manual_jni_registration = true
+}
+
 shared_library("jni_sample_lib") {
   sources = [ "sample_entry_point.cc" ]
 
   deps = [
+    ":jni_registration",
     ":jni_sample_native_side",
-    ":sample_jni_apk__final_jni",  # For registration_header
     "//base",
   ]
 }
@@ -63,7 +68,6 @@ android_apk("sample_jni_apk") {
   android_manifest = "AndroidManifest.xml"
   deps = [ ":jni_sample_java" ]
   shared_libraries = [ ":jni_sample_lib" ]
-  manual_jni_registration = true
 }
 
 # Serves to test that generated bindings compile properly.

diff --git a/base/android/jni_generator/jni_registration_generator.py b/base/android/jni_generator/jni_registration_generator.py
@@ -706,6 +706,8 @@ def _AddCases(self):
       params = _GetParamsListForMultiplex(signature[1], with_types=False)
       values = {
           'SWITCH_NUM': native.switch_num,
+          # We are forced to call the generated stub instead of the impl because
+          # the impl is not guaranteed to have a globally unique name.
           'STUB_NAME': self.helper.GetStubName(native),
           'PARAMS': params,
       }

diff --git a/base/android/jni_generator/sample_entry_point.cc b/base/android/jni_generator/sample_entry_point.cc
@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/android/jni_android.h"
-#include "base/android/jni_generator/sample_jni_apk__final_jni_generated.h"
+#include "base/android/jni_generator/jni_registration_generated.h"
 #include "base/android/jni_utils.h"
 
 // This is called by the VM when the shared library is first loaded.

diff --git a/build/android/gyp/generate_linker_version_script.py b/build/android/gyp/generate_linker_version_script.py
@@ -35,6 +35,10 @@ def main():
       '--export-java-symbols',
       action='store_true',
       help='Export Java_* JNI methods')
+  parser.add_argument(
+      '--jni-multiplexing',
+      action='store_true',
+      help='Export only the JNI methods generated by multiplexing')
   parser.add_argument('--export-fortesting-java-symbols',
                       action='store_true',
                       help='Export Java_*_ForTesting JNI methods')
@@ -61,7 +65,9 @@ def main():
   symbol_list = ['CrashpadHandlerMain', 'JNI_OnLoad']
 
   if options.export_java_symbols:
-    if options.export_fortesting_java_symbols:
+    if options.jni_multiplexing:
+      symbol_list.append('Java_*_resolve_1for_*')
+    elif options.export_fortesting_java_symbols:
       symbol_list.append('Java_*')
     else:
       # The linker uses unix shell globbing patterns, not regex. So, we have to

diff --git a/build/config/android/config.gni b/build/config/android/config.gni
@@ -291,7 +291,7 @@ if (is_android || is_chromeos) {
     use_hashed_jni_names = !is_java_debug
 
     # Enables JNI multiplexing to reduce JNI native methods overhead.
-    enable_jni_multiplexing = false
+    allow_jni_multiplexing = false
 
     # Enables trace event injection on Android views with bytecode rewriting.
     # This adds an additional step on android_app_bundle_module targets that
@@ -300,10 +300,6 @@ if (is_android || is_chromeos) {
         !is_java_debug && android_channel != "stable"
   }
 
-  if (enable_jni_multiplexing) {
-    use_hashed_jni_names = false
-  }
-
   # Host stuff -----------------------------------------------------------------
 
   # Defines the name the Android build gives to the current host CPU

diff --git a/build/config/android/linker_version_script.gni b/build/config/android/linker_version_script.gni
@@ -2,6 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import("//build/config/android/config.gni")
 import("//build/config/python.gni")
 
 # Generate a custom linker version script that can later be used with
@@ -27,6 +28,9 @@ template("generate_linker_version_script") {
       if (defined(invoker.testonly) && invoker.testonly) {
         args += [ "--export-fortesting-java-symbols" ]
       }
+      if (allow_jni_multiplexing) {
+        args += [ "--jni-multiplexing" ]
+      }
     }
 
     if (defined(invoker.export_feature_registrations) &&

diff --git a/build/config/android/rules.gni b/build/config/android/rules.gni
@@ -222,7 +222,9 @@ if (enable_java_templates) {
         if (!is_robolectric && use_hashed_jni_names) {
           args += [ "--use_proxy_hash" ]
         }
-        if (!is_robolectric && enable_jni_multiplexing) {
+
+        if (!is_robolectric && defined(invoker.enable_jni_multiplexing) &&
+            invoker.enable_jni_multiplexing) {
           args += [ "--enable_jni_multiplexing" ]
         }
         if (defined(invoker.namespace)) {
@@ -450,16 +452,20 @@ if (enable_java_templates && is_android) {
 
       _manual_jni_registration = defined(invoker.manual_jni_registration) &&
                                  invoker.manual_jni_registration
+      _enable_jni_multiplexing = defined(invoker.enable_jni_multiplexing) &&
+                                 invoker.enable_jni_multiplexing
+      if (_manual_jni_registration) {
+        args += [ "--manual-jni-registration" ]
+      }
+      if (_enable_jni_multiplexing) {
+        args += [ "--enable-jni-multiplexing" ]
+      }
 
-      if (_manual_jni_registration || enable_jni_multiplexing) {
+      if ((!defined(invoker.prevent_header_output) ||
+           !invoker.prevent_header_output) &&
+          (_manual_jni_registration || _enable_jni_multiplexing)) {
         assert(current_toolchain == default_toolchain,
                "We do not need >1 toolchain copies of the same header.")
-        if (_manual_jni_registration) {
-          args += [ "--manual-jni-registration" ]
-        }
-        if (enable_jni_multiplexing) {
-          args += [ "--enable-jni-multiplexing" ]
-        }
 
         _subdir = rebase_path(target_gen_dir, root_gen_dir)
         _jni_header_output =
@@ -2389,8 +2395,6 @@ if (enable_java_templates && is_android) {
   #     generated is solely controlled by this flag. Otherwise, the default behavior
   #     is NativeLibraries.java will only be generated for the base module/apk when
   #     its `shared_libraries` is not empty.
-  #   manual_jni_registration: If true, causes the ${target_name}__final_jni target
-  #     to additionally output a header file for use with manual JNI registration.
   #   jni_file_exclusions: List of source path to exclude from the
   #     final_jni step.
   #   aapt_locale_allowlist: If set, all locales not in this list will be
@@ -3028,8 +3032,8 @@ if (enable_java_templates && is_android) {
       generate_jni_registration("${_template_name}__final_jni") {
         forward_variables_from(invoker,
                                [
+                                 "enable_jni_multiplexing",
                                  "enable_native_mocks",
-                                 "manual_jni_registration",
                                  "require_native_mocks",
                                ])
         if (defined(invoker.bundle_target)) {
@@ -3040,6 +3044,7 @@ if (enable_java_templates && is_android) {
         if (defined(invoker.jni_file_exclusions)) {
           file_exclusions = invoker.jni_file_exclusions
         }
+        prevent_header_output = true
       }
       _srcjar_deps += [ ":${_template_name}__final_jni" ]
     } else {
@@ -3561,6 +3566,7 @@ if (enable_java_templates && is_android) {
                                "data_deps",
                                "deps",
                                "enable_lint",
+                               "enable_jni_multiplexing",
                                "enable_multidex",
                                "enable_native_mocks",
                                "enable_proguard_checks",
@@ -3586,7 +3592,6 @@ if (enable_java_templates && is_android) {
                                "lint_suppressions_file",
                                "loadable_modules",
                                "manifest_package",
-                               "manual_jni_registration",
                                "max_sdk_version",
                                "mergeable_android_manifests",
                                "product_config_java_packages",
@@ -3698,6 +3703,7 @@ if (enable_java_templates && is_android) {
                                "data",
                                "data_deps",
                                "deps",
+                               "enable_jni_multiplexing",
                                "enable_multidex",
                                "expected_android_manifest",
                                "expected_android_manifest_base",
@@ -3717,7 +3723,6 @@ if (enable_java_templates && is_android) {
                                "product_config_java_packages",
                                "main_component_library",
                                "manifest_package",
-                               "manual_jni_registration",
                                "max_sdk_version",
                                "min_sdk_version",
                                "mergeable_android_manifests",

diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
@@ -2128,6 +2128,9 @@ if (current_toolchain == default_toolchain) {
     targets = [ ":chrome_public_base_module_java" ]
     manual_jni_registration = true
     file_exclusions = chrome_jni_file_exclusions
+    if (allow_jni_multiplexing) {
+      enable_jni_multiplexing = true
+    }
   }
 
   # The test apks do not use chromium linker, but using manual JNI registration
@@ -2137,6 +2140,20 @@ if (current_toolchain == default_toolchain) {
     targets = [ ":chrome_public_base_module_java_for_test" ]
     manual_jni_registration = true
     file_exclusions = chrome_jni_file_exclusions
+    if (allow_jni_multiplexing) {
+      enable_jni_multiplexing = true
+    }
+  }
+
+  if (allow_jni_multiplexing) {
+    generate_jni_registration("monochrome_jni_registration") {
+      targets = [
+        ":monochrome_java",
+        ":chrome_all_java",
+      ]
+      file_exclusions = chrome_jni_file_exclusions
+      enable_jni_multiplexing = true
+    }
   }
 
   # This template instantiates targets responsible for generating pak
@@ -4006,6 +4023,9 @@ template("libchrome_impl") {
   chrome_common_shared_library(target_name) {
     sources = [ "../browser/android/chrome_entry_point.cc" ]
     deps = [ ":chrome_jni_registration($default_toolchain)" ]
+    if (allow_jni_multiplexing) {
+      enable_jni_multiplexing = true
+    }
     if (defined(invoker.deps)) {
       deps += invoker.deps
     }
@@ -4063,6 +4083,9 @@ chrome_common_shared_library("libchromefortest") {
   if (enable_vr) {
     deps += [ "//chrome/browser/android/vr:test_support" ]
   }
+  if (allow_jni_multiplexing) {
+    enable_jni_multiplexing = true
+  }
 
   # Make this a partitioned library, since some partitioned code is linked in
   # (otherwise, the library will warn at build time that it contains multiple
@@ -4085,11 +4108,18 @@ template("libmonochrome_apk_or_bundle_tmpl") {
       deps += invoker.deps
     }
 
+    defines = []
     if (webview_includes_weblayer) {
-      defines = [ "WEBVIEW_INCLUDES_WEBLAYER" ]
+      defines += [ "WEBVIEW_INCLUDES_WEBLAYER" ]
       deps += [ "//weblayer:weblayer_lib" ]
     }
 
+    if (allow_jni_multiplexing) {
+      defines += [ "JNI_REGISTRATION_REQUIRED" ]
+      deps += [ ":monochrome_jni_registration($default_toolchain)" ]
+      enable_jni_multiplexing = true
+    }
+
     if (enable_vr) {
       deps += [ "//chrome/browser/android/vr:module_factory" ]
     }

diff --git a/chrome/android/chrome_common_shared_library.gni b/chrome/android/chrome_common_shared_library.gni
@@ -120,7 +120,8 @@ template("chrome_common_shared_library") {
 
     # Handle VR JNI registration and dependencies.
     if (!_is_webview && enable_vr) {
-      if (_export_java_symbols) {
+      if (_export_java_symbols && (!defined(invoker.enable_jni_multiplexing) ||
+                                   !invoker.enable_jni_multiplexing)) {
         # NOTE: While this file is named *_monochrome.cc, it just contains an
         # empty vr::RegisterJni() function that returns true.
         sources += [ "../browser/android/vr/register_jni_monochrome.cc" ]
@@ -137,6 +138,9 @@ template("chrome_common_shared_library") {
       }
     }
 
+    # We check this variable last, so sometimes it isn't used to decide which registration to use.
+    not_needed([ "enable_jni_multiplexing" ])
+
     if (_generate_partitions) {
       partitions = []
       foreach(_module_desc, _module_descs) {

diff --git a/chrome/android/chrome_public_apk_tmpl.gni b/chrome/android/chrome_public_apk_tmpl.gni
@@ -278,6 +278,10 @@ template("chrome_public_common_apk_or_module_tmpl") {
       custom_assertion_handler = crash_reporting_assertion_handler
     }
 
+    if (allow_jni_multiplexing) {
+      enable_jni_multiplexing = true
+    }
+
     if (!defined(version_name)) {
       version_name = chrome_version_name
     }