Skip to content

Commit

Permalink
Fix VR module native code with isolated splits
Browse files Browse the repository at this point in the history
There were three separate issues which were preventing the VR native
code from working correctly:
- Isolated split class loaders can't load native libraries (b/171269960)
- Shared libs for modules were not being added to the module descriptor
- The GVR native methods need to be manually registered now that GVR
  code is not in the base module.

Bug: 1126301
Change-Id: I184ea61fe4ef97a598ecf47b6e3cd81f4425ec1f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2486571
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819164}
  • Loading branch information
clarkduvall authored and Commit Bot committed Oct 20, 2020
1 parent ee9f4ab commit 7b25ff7
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 10 deletions.
20 changes: 17 additions & 3 deletions base/android/java/src/org/chromium/base/BundleUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,24 @@ public static Context createIsolatedSplitContext(Context base, String splitName)

/* Returns absolute path to a native library in a feature module. */
@CalledByNative
private static String getNativeLibraryPath(String libraryName) {
public static String getNativeLibraryPath(String libraryName) {
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
return ((BaseDexClassLoader) ContextUtils.getApplicationContext().getClassLoader())
.findLibrary(libraryName);
ClassLoader classLoader = ContextUtils.getApplicationContext().getClassLoader();
String path = getNativeLibraryPathFromClassLoader(classLoader, libraryName);
// TODO(b/171269960): Isolated split class loaders have an empty library path, so check
// the parent class loader as well.
if (path == null) {
path = getNativeLibraryPathFromClassLoader(classLoader.getParent(), libraryName);
}
return path;
}
}

private static String getNativeLibraryPathFromClassLoader(
ClassLoader classLoader, String libraryName) {
if (classLoader == null) {
return null;
}
return ((BaseDexClassLoader) classLoader).findLibrary(libraryName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
import android.content.Context;
import android.view.Surface;

import dalvik.system.BaseDexClassLoader;

import org.chromium.base.BundleUtils;
import org.chromium.base.ContextUtils;
import org.chromium.base.Log;
import org.chromium.base.StrictModeContext;
import org.chromium.base.ThreadUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.JNINamespace;
Expand Down Expand Up @@ -65,10 +63,7 @@ private static ArCoreJavaUtils create(long nativeArCoreJavaUtils) {

@CalledByNative
private static String getArCoreShimLibraryPath() {
try (StrictModeContext ignored = StrictModeContext.allowDiskReads()) {
return ((BaseDexClassLoader) ContextUtils.getApplicationContext().getClassLoader())
.findLibrary("arcore_sdk_c");
}
return BundleUtils.getNativeLibraryPath("arcore_sdk_c");
}

/**
Expand Down
15 changes: 15 additions & 0 deletions chrome/android/modules/chrome_bundle_tmpl.gni
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,25 @@ template("chrome_bundle") {
_module_desc.supports_isolated_split) {
_module_descs += [ _module_desc ]
} else {
_shared_libraries = []
if (defined(_module_desc.native_deps) &&
_module_desc.native_deps != []) {
_arch = ""
_toolchain = ""
if (android_64bit_target_cpu) {
if (invoker.is_64_bit_browser) {
_arch = "_64"
} else {
_toolchain = "($android_secondary_abi_toolchain)"
}
}
_shared_libraries += [ "//chrome/android:libmonochrome${_arch}_${_module_desc.name}${_toolchain}" ]
}
_module_desc_target_name =
"${target_name}__${_module_desc.name}__module_desc_java"
module_desc_java(_module_desc_target_name) {
module_name = _module_desc.name
shared_libraries = _shared_libraries
if (defined(_module_desc.pak_deps)) {
paks = _module_desc.paks
}
Expand Down
15 changes: 15 additions & 0 deletions chrome/browser/android/vr/register_jni_monochrome.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,24 @@

#include "chrome/browser/android/vr/register_jni.h"

#include "chrome/browser/android/vr/register_gvr_jni.h"

namespace vr {

bool RegisterJni(JNIEnv* env) {
// The GVR Java code is normally in the vr DFM, which will be loaded into the
// base class loader. If the enable_chrome_module gn arg is enabled, the GVR
// Java code will be in the chrome DFM, which is loaded as an isolated split.
// This means the Java code is no longer automatically loaded in the base
// class loader. Automatic JNI registration only works for native methods
// associated with the base class loader (which loaded libmonochrome.so, so
// will look for symbols there). Most of Chrome's native methods are in
// GEN_JNI.java which is present in the base module, so do not need manual
// registration. Since GVR has native methods outside of GEN_JNI.java which
// are not present in the base module, these must be manually registered.
if (!vr::RegisterGvrJni(env)) {
return false;
}
return true;
}

Expand Down

0 comments on commit 7b25ff7

Please sign in to comment.