[ar] Add secondary abi support for loadable modules and use for AR

With this change, it should be possible to support enabling ARCore
integration on arm64.

I've added a secondary abi analog to loadable_modules and fixed a
typo in the __secondary_abi__runtime_deps target that was preventing
monochrome_public_test_ar_apk from fulfilling its dependencies.

Bug: 841389
Cq-Include-Trybots: luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I48182a861e779417a610c20be45cdbe0c52e5b82
Reviewed-on: https://chromium-review.googlesource.com/1145470Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Ian Vollick <vollick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577270}

build/android/gyp/write_build_config.py

@@ -828,6 +828,11 @@ def main(argv):
   parser.add_option('--secondary-abi-shared-libraries-runtime-deps',
                     help='Path to file containing runtime deps for secondary '
                          'abi shared libraries.')
+  parser.add_option('--secondary-native-libs',
+                    action='append',
+                    help='GYP-list of native libraries for secondary '
+                         'android-abi. Can be specified multiple times.',
+                    default=[])
   parser.add_option('--uncompress-shared-libraries', default=False,
                     action='store_true',
                     help='Whether to store native libraries uncompressed')
@@ -1324,6 +1329,9 @@ def main(argv):
       secondary_abi_java_libraries_list = _CreateJavaLibrariesList(
           secondary_abi_library_paths)
 
+    for gyp_list in options.secondary_native_libs:
+      secondary_abi_library_paths.extend(build_utils.ParseGnList(gyp_list))
+
     extra_shared_libraries = build_utils.ParseGnList(
         options.extra_shared_libraries)
 

build/config/android/internal_rules.gni

@@ -306,6 +306,13 @@ template("write_build_config") {
       ]
     }
 
+    if (defined(invoker.secondary_abi_loadable_modules) &&
+        invoker.secondary_abi_loadable_modules != []) {
+      _rebased_modules =
+          rebase_path(invoker.secondary_abi_loadable_modules, root_build_dir)
+      args += [ "--secondary-native-libs=$_rebased_modules" ]
+    }
+
     if (defined(invoker.uncompress_shared_libraries) &&
         invoker.uncompress_shared_libraries) {
       args += [ "--uncompress-shared-libraries" ]
@@ -2084,7 +2091,7 @@ if (enable_java_templates) {
   #   native_libs_filearg: @FileArg() of additionally native libraries.
   #   secondary_abi_native_libs: (optional) List of native libraries for
   #     secondary ABI.
-  #   secondary_abi_native_libs_filearg: (optiona). @FileArg() of additional
+  #   secondary_abi_native_libs_filearg: (optional). @FileArg() of additional
   #     secondary ABI native libs.
   #   write_asset_list: Adds an extra file to the assets, which contains a list of
   #     all other asset files.
@@ -2198,8 +2205,8 @@ if (enable_java_templates) {
         args += [ "--android-abi=$android_app_abi" ]
       }
       if (defined(invoker.secondary_abi_native_libs_filearg) ||
-          (defined(invoker.secondary_native_libs) &&
-           invoker.secondary_native_libs != []) ||
+          (defined(invoker.secondary_abi_loadable_modules) &&
+           invoker.secondary_abi_loadable_modules != []) ||
           _secondary_native_lib_placeholders != []) {
         assert(defined(android_app_secondary_abi))
         args += [ "--secondary-android-abi=$android_app_secondary_abi" ]
@@ -2218,14 +2225,8 @@ if (enable_java_templates) {
         args += [ "--secondary-native-lib-placeholders=$_secondary_native_lib_placeholders" ]
       }
 
-      # TODO (michaelbai): Remove the secondary_native_libs variable.
       if (defined(invoker.secondary_abi_native_libs_filearg)) {
         args += [ "--secondary-native-libs=${invoker.secondary_abi_native_libs_filearg}" ]
-      } else if (defined(invoker.secondary_native_libs) &&
-                 invoker.secondary_native_libs != []) {
-        inputs += invoker.secondary_native_libs
-        _secondary_native_libs = rebase_path(invoker.secondary_native_libs)
-        args += [ "--secondary-native-libs=$_secondary_native_libs" ]
       }
 
       if (defined(invoker.uncompress_shared_libraries) &&
@@ -2262,7 +2263,8 @@ if (enable_java_templates) {
       _native_libs = invoker.native_libs
     }
     _native_libs_even_when_incremental = []
-    if (defined(invoker.native_libs_even_when_incremental)) {
+    if (defined(invoker.native_libs_even_when_incremental) &&
+        invoker.native_libs_even_when_incremental != []) {
       _native_libs_even_when_incremental =
           invoker.native_libs_even_when_incremental
     }
@@ -2285,7 +2287,7 @@ if (enable_java_templates) {
                                "packaged_resources_path",
                                "secondary_native_lib_placeholders",
                                "secondary_abi_native_libs_filearg",
-                               "secondary_native_libs",
+                               "secondary_abi_loadable_modules",
                                "uncompress_shared_libraries",
                                "write_asset_list",
                              ])
@@ -2366,7 +2368,7 @@ if (enable_java_templates) {
         forward_variables_from(invoker,
                                [
                                  "assets_build_config",
-                                 "secondary_native_libs",
+                                 "secondary_abi_loadable_modules",
                                  "uncompress_shared_libraries",
                                ])
         if (!defined(uncompress_shared_libraries)) {
@@ -2947,6 +2949,7 @@ if (enable_java_templates) {
                                "proguard_configs",
                                "proguard_enabled",
                                "proguard_output_jar_path",
+                               "secondary_abi_loadable_modules",
                                "type",
                              ])
       if (type == "android_apk") {

build/config/android/rules.gni

@@ -1868,6 +1868,8 @@ if (enable_java_templates) {
   #         and enable_relocation_packing do not apply
   #     Use this instead of shared_libraries when you are going to load the library
   #     conditionally, and only when shared_libraries doesn't work for you.
+  #   secondary_abi_loadable_modules: This is the loadable_modules analog to
+  #     secondary_abi_shared_libraries.
   #   shared_libraries: List shared_library targets to bundle. If these
   #     libraries depend on other shared_library targets, those dependencies will
   #     also be included in the apk (e.g. for is_component_build).
@@ -1888,8 +1890,6 @@ if (enable_java_templates) {
   #   requires_sdk_api_level_23: If defined and true, the apk is intended for
   #     installation only on Android M or later. In these releases the system
   #     linker does relocation unpacking, so we can enable it unconditionally.
-  #   secondary_native_libs (deprecated): The path of native libraries for secondary
-  #     app abi.
   #   aapt_locale_whitelist: If set, all locales not in this list will be
   #     stripped from resources.arsc.
   #   exclude_xxxhdpi: Causes all drawable-xxxhdpi images to be excluded
@@ -2088,7 +2088,7 @@ if (enable_java_templates) {
       }
     } else {
       # Must exist for instrumentation_test_apk() to depend on.
-      group("${_template_name}__secondary_abi_runtime_deps") {
+      group("${_template_name}__secondary_abi__runtime_deps") {
       }
     }
 
@@ -2312,8 +2312,9 @@ if (enable_java_templates) {
                                "classpath_deps",
                                "emma_never_instrument",
                                "java_files",
-                               "no_build_hooks",
                                "javac_args",
+                               "no_build_hooks",
+                               "secondary_abi_loadable_modules",
                                "uncompress_shared_libraries",
                              ])
       type = "android_apk"
@@ -2521,7 +2522,6 @@ if (enable_java_templates) {
         forward_variables_from(invoker,
                                [
                                  "public_deps",
-                                 "secondary_native_libs",
                                  "shared_resources",
                                  "uncompress_shared_libraries",
                                  "write_asset_list",
@@ -2816,9 +2816,9 @@ if (enable_java_templates) {
                                "proguard_enabled",
                                "proguard_jar_path",
                                "requires_sdk_api_level_23",
+                               "secondary_abi_loadable_modules",
                                "secondary_abi_shared_libraries",
                                "secondary_native_lib_placeholders",
-                               "secondary_native_libs",
                                "shared_libraries",
                                "shared_resources",
                                "shared_resources_whitelist_target",
@@ -2894,9 +2894,9 @@ if (enable_java_templates) {
                                "proguard_configs",
                                "proguard_enabled",
                                "requires_sdk_api_level_23",
+                               "secondary_abi_loadable_modules",
                                "secondary_abi_shared_libraries",
                                "secondary_native_lib_placeholders",
-                               "secondary_native_libs",
                                "shared_libraries",
                                "shared_resources",
                                "shared_resources_whitelist_target",
@@ -3086,14 +3086,14 @@ if (enable_java_templates) {
       # symbolization can be done.
       deps = [
         ":${_apk_target_name}__runtime_deps",
-        ":${_apk_target_name}__secondary_abi_runtime_deps",
+        ":${_apk_target_name}__secondary_abi__runtime_deps",
       ]
       if (defined(invoker.apk_under_test)) {
         _under_test_label =
             get_label_info(invoker.apk_under_test, "label_no_toolchain")
         deps += [
           "${_under_test_label}__runtime_deps",
-          "${_under_test_label}__secondary_abi_runtime_deps",
+          "${_under_test_label}__secondary_abi__runtime_deps",
         ]
       }
     }
@@ -3181,7 +3181,7 @@ if (enable_java_templates) {
       shared_libraries = [ invoker.shared_library ]
       deps += [
         ":${target_name}__runtime_deps",
-        ":${target_name}__secondary_abi_runtime_deps",
+        ":${target_name}__secondary_abi__runtime_deps",
         "//base:base_java",
         "//testing/android/reporter:reporter_java",
       ]

chrome/android/chrome_public_apk_tmpl.gni

@@ -210,10 +210,20 @@ template("monochrome_public_apk_tmpl") {
       deps += [ "//third_party/arcore-android-sdk:libdynamite_client_java" ]
     }
     if (package_arcore) {
-      deps += [ "//third_party/arcore-android-sdk:libarcore_library" ]
-
       # We store this as a separate .so in the APK and only load as needed.
+      if (android_64bit_target_cpu && build_apk_secondary_abi) {
+        deps += [
+          "//third_party/arcore-android-sdk:libarcore_library_secondary_abi",
+        ]
+        toolchain_out_dir = get_label_info(
+                "//third_party/arcore-android-sdk:libarcore_library($android_secondary_abi_toolchain)",
+                "root_out_dir")
+        secondary_abi_loadable_modules =
+            [ "${toolchain_out_dir}/libarcore_sdk_c_minimal.so" ]
+      } else {
+        deps += [ "//third_party/arcore-android-sdk:libarcore_library" ]
         loadable_modules = [ "${root_out_dir}/libarcore_sdk_c_minimal.so" ]
       }
     }
+  }
 }

chrome/browser/android/vr/arcore_device/arcore_shim.cc

@@ -72,8 +72,12 @@ bool LoadArCoreSdk() {
 
   sdk_handle = dlopen("libarcore_sdk_c_minimal.so", RTLD_GLOBAL | RTLD_NOW);
   if (!sdk_handle) {
-    DLOG(ERROR) << "could not open libarcore_sdk_c_minimal.so";
+    char* error_string = nullptr;
+    error_string = dlerror();
+    LOG(ERROR) << "Could not open libarcore_sdk_c_minimal.so: " << error_string;
     return false;
+  } else {
+    VLOG(2) << "Opened shim shared library.";
   }
 
   // TODO(vollick): check SDK version.

device/vr/buildflags/buildflags.gni

@@ -37,10 +37,10 @@ declare_args() {
   # we are limiting to canary and dev until binary size issues are resolved.
   # TODO(crbug.com/836524): once we've refactored AR code out from vr
   # directories, we can stop requiring |enable_vr| here.
-  package_arcore =
-      enable_vr && is_android && !is_chromecast && current_cpu == "arm" &&
-      (android_channel == "default" || android_channel == "canary" ||
-       android_channel == "dev")
+  package_arcore = enable_vr && is_android && !is_chromecast &&
+                   (current_cpu == "arm" || current_cpu == "arm64") &&
+                   (android_channel == "default" ||
+                    android_channel == "canary" || android_channel == "dev")
 
   # TODO(crbug.com/841389): We should eventually have a single flag for
   # enabling arcore, but we currently don't support ARCore in 64bit, and we do

third_party/arcore-android-sdk/BUILD.gn

@@ -10,14 +10,9 @@ java_prebuilt("libdynamite_client_java") {
   jar_path = "libarcore_client_c.jar"
 }
 
-source_set("libarcore_sdk") {
-  deps = [
-    ":libarcore_library",
-  ]
-  libs = [ "${root_out_dir}/libarcore_sdk_c_minimal.so" ]
-}
-
-copy("libarcore_library") {
+if (!android_64bit_target_cpu || !build_apk_secondary_abi ||
+    current_toolchain == android_secondary_abi_toolchain) {
+  copy("libarcore_library") {
     if (current_cpu == "arm") {
       sources = [
         "libraries/android_arm/libarcore_sdk_c_minimal.so",
@@ -30,6 +25,13 @@ copy("libarcore_library") {
     outputs = [
       "${root_out_dir}/libarcore_sdk_c_minimal.so",
     ]
+  }
+} else {
+  group("libarcore_library_secondary_abi") {
+    public_deps = [
+      ":libarcore_library($android_secondary_abi_toolchain)",
+    ]
+  }
 }
 
 config("libarcore_config") {

third_party/arcore-android-sdk/README.chromium

@@ -26,4 +26,7 @@ The LICENSE file is taken from
  * https://github.com/google-ar/arcore-unity-sdk/blob/master/LICENSE
 
 Changes:
+2018-07-19 - Updated BUILD.gn to support secondary abi (the previous change for
+             arm64 support turned out to be insufficient) and removed the unused
+             libarcore_sdk target.
 2018-05-04 - Updated BUILD.gn to work properly for arm64.