Commit 3e21b750 authored by Piotr Bialecki's avatar Piotr Bialecki Committed by Commit Bot

Reland "Move away from using DFM to distribute AR-related code"

This is a reland of cd842897

Original CL caused Chrome Canary to be rejected from Play Store.
Play Store validates whether ARCore SDK's manifest entries are present
only for applications that contain entry about ARCore being either
required or optional.

Additional changes needed for reland:
- ArCoreShimImpl.java should not be included in java_sources - it will
  be brought in by ar_java target on supported configurations since
  it depends on ARCore SDK which is included only for specific targets
- relaxed the change in arcore_consent_prompt.cc - the original change
  would cause builds that are not supporting ARCore (by design) to
  crash

Original change's description:
> Move away from using DFM to distribute AR-related code
>
> This change will make Chrome for Android stop using DFM to ship the
> ARCore SDK. This is done to enable us to provide AR experiences for
> apps using WebView. The changes are introduced in a way to enable us to
> switch back to using AR DFM if need be.
>
> Changes summary:
> - chrome_java target (defined in chrome/android/BUILD.gn) now also
>   compiles ArCoreShimImpl.java (see java_sources.gni) - this means that
>   java parts from ARCore SDK have to be added to its deps
> - libarcore_sdk_c.so needs to be listed in loadable modules - this is
>   done in monochrome_public_common_apk_or_module_tmpl (only if target
>   is a base module when ARCore is enabled)
> - AndroidManifest.xml no longer needs to contain the min_apk_version
>   and InstallActivity entries explicitly - they will be brought in
>   auto-magically by the build system and were needed only because
>   manifest merging did not work for DFMs
> - ArCoreInstallUtils (java) should never report that we can request
>   installation of AR DFM
> - ArCoreConsentPrompt (native) should never think it needs to request
>   installation of AR DFM
> - Added a DCHECK and graceful handling of null jstring for java_path
>   (path to ARCore SDK .so) - it can be null if loadable_modules do not
>   contain correct entry - with the DCHECK, debug builds will still
>   crash but the release builds should just report that the WebXR
>   specified session configuration (session mode == "immersive-ar") is
>   not supported
>
> Binary-Size: Tracked in crbug/1040289
> Bug: 1031623, 1040289
> Change-Id: Ia2f305d1685f1c7207cc187d253f59a6331c36fc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1988834
> Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Reviewed-by: Alexander Cooper <alcooper@chromium.org>
> Auto-Submit: Piotr Bialecki <bialpio@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#729818}

Binary-Size: Tracked in crbug/1040289
Bug: 1031623, 1040289
Change-Id: I87eac0bc3748973e22190c6dba1187e822f83082
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1995769Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarAlexander Cooper <alcooper@chromium.org>
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#731011}
parent afa1d412
...@@ -464,6 +464,7 @@ android_library("chrome_java") { ...@@ -464,6 +464,7 @@ android_library("chrome_java") {
if (enable_vr) { if (enable_vr) {
deps += [ ":chrome_vr_java_resources" ] deps += [ ":chrome_vr_java_resources" ]
} }
srcjar_deps += [ ":chrome_vr_android_java_enums_srcjar" ] srcjar_deps += [ ":chrome_vr_android_java_enums_srcjar" ]
if (enable_feed_in_chrome) { if (enable_feed_in_chrome) {
deps += [ ":chrome_feed_java_resources" ] deps += [ ":chrome_feed_java_resources" ]
...@@ -2157,10 +2158,9 @@ if (enable_arcore) { ...@@ -2157,10 +2158,9 @@ if (enable_arcore) {
] ]
# Include ArCore files directly instead of using bundles. # Include ArCore files directly instead of using bundles.
deps += [ deps +=
"//chrome/browser/android/vr:ar_java", [ "//chrome/browser/android/vr:ar_java",
"//third_party/arcore-android-sdk-client:com_google_ar_core_java", "//third_party/arcore-android-sdk-client:com_google_ar_core_java" ]
]
_libarcore_dir = get_label_info( _libarcore_dir = get_label_info(
"//third_party/arcore-android-sdk-client:com_google_ar_core_java($default_toolchain)", "//third_party/arcore-android-sdk-client:com_google_ar_core_java($default_toolchain)",
......
...@@ -318,6 +318,12 @@ template("chrome_public_common_apk_or_module_tmpl") { ...@@ -318,6 +318,12 @@ template("chrome_public_common_apk_or_module_tmpl") {
template("monochrome_public_common_apk_or_module_tmpl") { template("monochrome_public_common_apk_or_module_tmpl") {
_is_bundle_module = defined(invoker.target_type) && _is_bundle_module = defined(invoker.target_type) &&
invoker.target_type == "android_app_bundle_module" invoker.target_type == "android_app_bundle_module"
if (_is_bundle_module) {
assert(
defined(invoker.is_base_module),
"_is_bundle_module is true but the invoker does not define is_base_module!")
}
if (defined(invoker.enable_multidex)) { if (defined(invoker.enable_multidex)) {
_enable_multidex = invoker.enable_multidex _enable_multidex = invoker.enable_multidex
} else { } else {
...@@ -344,6 +350,32 @@ template("monochrome_public_common_apk_or_module_tmpl") { ...@@ -344,6 +350,32 @@ template("monochrome_public_common_apk_or_module_tmpl") {
"//components/crash/android:handler_java", "//components/crash/android:handler_java",
] ]
if (_is_bundle_module && invoker.is_base_module && enable_arcore) {
# AR DFM is disabled - bring in the ar_java target along with ARCore SDK
# and set the loadable_modules / secondary_abi_loadable_modules to what
# would be brought in by the module.
# This needs to happen for monochrome & trichrome builds of base module if ARCore is enabled.
_deps +=
[ "//chrome/browser/android/vr:ar_java",
"//third_party/arcore-android-sdk-client:com_google_ar_core_java" ]
_libarcore_dir = get_label_info(
"//third_party/arcore-android-sdk-client:com_google_ar_core_java($default_toolchain)",
"target_out_dir") + "/com_google_ar_core_java/jni"
if (android_64bit_target_cpu) {
if (invoker.is_64_bit_browser) {
loadable_modules = [ "$_libarcore_dir/arm64-v8a/libarcore_sdk_c.so" ]
} else {
secondary_abi_loadable_modules =
[ "$_libarcore_dir/armeabi-v7a/libarcore_sdk_c.so" ]
}
} else {
loadable_modules = [ "$_libarcore_dir/armeabi-v7a/libarcore_sdk_c.so" ]
}
}
if (is_monochrome) { if (is_monochrome) {
product_config_java_packages = [ product_config_java_packages = [
"org.chromium.chrome.browser", "org.chromium.chrome.browser",
......
...@@ -175,21 +175,6 @@ by a child template that "extends" this file. ...@@ -175,21 +175,6 @@ by a child template that "extends" this file.
<!-- ARCore APK integration --> <!-- ARCore APK integration -->
<!-- This tag indicates that this application optionally uses ARCore. --> <!-- This tag indicates that this application optionally uses ARCore. -->
<meta-data android:name="com.google.ar.core" android:value="optional" /> <meta-data android:name="com.google.ar.core" android:value="optional" />
<!-- This value must match value present in ARCore SDK's .aar -->
<!-- TODO(https://crbug.com/917406): modify build scripts to
automatically pull this value in - the problem exists because in
bundle mode, the ARCore SDK is packaged into AR module and
the module installation will not automatically bring in DFM's
manifest entries. -->
<meta-data android:name="com.google.ar.core.min_apk_version"
android:value="190531000"/>
<activity
android:name="com.google.ar.core.InstallActivity"
android:configChanges="keyboardHidden|orientation|screenSize"
android:excludeFromRecents="true"
android:exported="false"
android:launchMode="singleTop"
android:theme="@android:style/Theme.Material.Light.Dialog.Alert" />
{% endif %} {% endif %}
<!-- Cast support --> <!-- Cast support -->
......
...@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.vr; ...@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.vr;
import android.app.Activity; import android.app.Activity;
import org.chromium.base.BundleUtils;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.Log; import org.chromium.base.Log;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
...@@ -82,7 +81,8 @@ public class ArCoreInstallUtils implements ModuleInstallUi.FailureUiListener { ...@@ -82,7 +81,8 @@ public class ArCoreInstallUtils implements ModuleInstallUi.FailureUiListener {
@CalledByNative @CalledByNative
private boolean canRequestInstallArModule() { private boolean canRequestInstallArModule() {
// We can only try to install the AR module if we are in a bundle mode. // We can only try to install the AR module if we are in a bundle mode.
return BundleUtils.isBundle(); // Currently, AR DFM is disabled so we should never have to install it.
return false;
} }
@CalledByNative @CalledByNative
......
...@@ -60,7 +60,7 @@ if (enable_image_editor) { ...@@ -60,7 +60,7 @@ if (enable_image_editor) {
# Modules shipped in Monochrome (Android N+). # Modules shipped in Monochrome (Android N+).
monochrome_module_descs = chrome_modern_module_descs monochrome_module_descs = chrome_modern_module_descs
if (enable_arcore) { if (false) { # AR DFM is currently disabled
monochrome_module_descs += [ ar_module_desc ] monochrome_module_descs += [ ar_module_desc ]
} }
if (!disable_autofill_assistant_dfm) { if (!disable_autofill_assistant_dfm) {
......
...@@ -66,9 +66,8 @@ ArCoreConsentPrompt::ArCoreConsentPrompt() : weak_ptr_factory_(this) {} ...@@ -66,9 +66,8 @@ ArCoreConsentPrompt::ArCoreConsentPrompt() : weak_ptr_factory_(this) {}
ArCoreConsentPrompt::~ArCoreConsentPrompt() = default; ArCoreConsentPrompt::~ArCoreConsentPrompt() = default;
void ArCoreConsentPrompt::OnUserConsentResult( void ArCoreConsentPrompt::OnUserConsentResult(JNIEnv* env,
JNIEnv* env, jboolean is_granted) {
jboolean is_granted) {
jdelegate_.Reset(); jdelegate_.Reset();
if (!on_user_consent_callback_) if (!on_user_consent_callback_)
...@@ -139,9 +138,8 @@ void ArCoreConsentPrompt::RequestInstallSupportedArCore() { ...@@ -139,9 +138,8 @@ void ArCoreConsentPrompt::RequestInstallSupportedArCore() {
GetTabFromRenderer(render_process_id_, render_frame_id_)); GetTabFromRenderer(render_process_id_, render_frame_id_));
} }
void ArCoreConsentPrompt::OnRequestInstallArModuleResult( void ArCoreConsentPrompt::OnRequestInstallArModuleResult(JNIEnv* env,
JNIEnv* env, bool success) {
bool success) {
DVLOG(1) << __func__; DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
...@@ -150,9 +148,8 @@ void ArCoreConsentPrompt::OnRequestInstallArModuleResult( ...@@ -150,9 +148,8 @@ void ArCoreConsentPrompt::OnRequestInstallArModuleResult(
} }
} }
void ArCoreConsentPrompt::OnRequestInstallSupportedArCoreResult( void ArCoreConsentPrompt::OnRequestInstallSupportedArCoreResult(JNIEnv* env,
JNIEnv* env, bool success) {
bool success) {
DVLOG(1) << __func__; DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
DCHECK(on_request_arcore_install_or_update_result_callback_); DCHECK(on_request_arcore_install_or_update_result_callback_);
...@@ -162,15 +159,14 @@ void ArCoreConsentPrompt::OnRequestInstallSupportedArCoreResult( ...@@ -162,15 +159,14 @@ void ArCoreConsentPrompt::OnRequestInstallSupportedArCoreResult(
void ArCoreConsentPrompt::RequestArModule() { void ArCoreConsentPrompt::RequestArModule() {
DVLOG(1) << __func__; DVLOG(1) << __func__;
if (ShouldRequestInstallArModule()) { if (ShouldRequestInstallArModule()) {
if (!CanRequestInstallArModule()) { // AR DFM is disabled - if we think we should install AR module, then it
OnRequestArModuleResult(false); // means that we are using a build that does not support AR capabilities.
return; // Treat this as if the AR module installation failed.
} LOG(WARNING) << "AR is not supported on this build";
on_request_ar_module_result_callback_ = base::BindOnce( OnRequestArModuleResult(false);
&ArCoreConsentPrompt::OnRequestArModuleResult, GetWeakPtr());
RequestInstallArModule();
return; return;
} }
......
...@@ -19,10 +19,6 @@ using base::android::ScopedJavaLocalRef; ...@@ -19,10 +19,6 @@ using base::android::ScopedJavaLocalRef;
namespace vr { namespace vr {
namespace {
} // namespace
ArCoreJavaUtils::ArCoreJavaUtils() { ArCoreJavaUtils::ArCoreJavaUtils() {
JNIEnv* env = AttachCurrentThread(); JNIEnv* env = AttachCurrentThread();
if (!env) if (!env)
...@@ -109,6 +105,21 @@ bool ArCoreJavaUtils::EnsureLoaded() { ...@@ -109,6 +105,21 @@ bool ArCoreJavaUtils::EnsureLoaded() {
// absolute path. // absolute path.
ScopedJavaLocalRef<jstring> java_path = ScopedJavaLocalRef<jstring> java_path =
Java_ArCoreJavaUtils_getArCoreShimLibraryPath(env); Java_ArCoreJavaUtils_getArCoreShimLibraryPath(env);
// Crash in debug builds if `java_path` is a null pointer but handle this
// situation in release builds. This is done by design - the `java_path` will
// be null only if there was a regression introduced to our gn/gni files w/o
// causing a build break. In release builds, this approach will result in the
// site not being able to request an AR session.
DCHECK(java_path)
<< "Unable to find path to ARCore SDK library - please ensure that "
"loadable_modules and secondary_abi_loadable_modules are set "
"correctly when building";
if (!java_path) {
LOG(ERROR) << "Unable to find path to ARCore SDK library";
return false;
}
return LoadArCoreSdk(base::android::ConvertJavaStringToUTF8(env, java_path)); return LoadArCoreSdk(base::android::ConvertJavaStringToUTF8(env, java_path));
} }
......
...@@ -232,6 +232,7 @@ void XR::PendingSupportsSessionQuery::Resolve(bool supported, ...@@ -232,6 +232,7 @@ void XR::PendingSupportsSessionQuery::Resolve(bool supported,
if (supported) { if (supported) {
resolver_->Resolve(); resolver_->Resolve();
} else { } else {
DVLOG(2) << __func__ << ": session is unsupported - throwing exception";
RejectWithDOMException(DOMExceptionCode::kNotSupportedError, RejectWithDOMException(DOMExceptionCode::kNotSupportedError,
kSessionNotSupported, exception_state); kSessionNotSupported, exception_state);
} }
...@@ -647,6 +648,9 @@ ScriptPromise XR::InternalIsSessionSupported(ScriptState* script_state, ...@@ -647,6 +648,9 @@ ScriptPromise XR::InternalIsSessionSupported(ScriptState* script_state,
if (session_mode == device::mojom::blink::XRSessionMode::kImmersiveAr && if (session_mode == device::mojom::blink::XRSessionMode::kImmersiveAr &&
!RuntimeEnabledFeatures::WebXRARModuleEnabled(doc)) { !RuntimeEnabledFeatures::WebXRARModuleEnabled(doc)) {
DVLOG(2) << __func__
<< ": Immersive AR session is only supported if WebXRARModule "
"feature is enabled";
query->Resolve(false); query->Resolve(false);
return promise; return promise;
} }
......
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