Tweak OpenXr end session flow on Android
On Android, when blink ends a session, everything appears to become headlocked if we simply end the way we do now (which is to call xrDestroySession and then shut down the activity). This starts by shutting down the activity (mimicking what the back button does), and then only stops the session once that removal has finished. While the page's content *does* stall in place with this approach, turning does update the position and exposes the edges of the content. This is a platform bug that we will follow up on, but this makes the experience better for the user in the mean time. Followup investigations will also explore delaying the "stop" in blink, but previous research indicated that no change occurred. Bug: 380499384 Change-Id: I0688889977941e9c0f11a5eeb4a6d56ef8111372 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6236184 Reviewed-by: Brandon Jones <bajones@chromium.org> Commit-Queue: Alexander Cooper <alcooper@chromium.org> Cr-Commit-Position: refs/heads/main@{#1416515}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
648f054bac
commit
d765257fa0
chrome/browser/android/vr/arcore_device
components/webxr/android
java
src
org
chromium
components
device/vr
@ -153,6 +153,7 @@ class StubXrJavaCoordinator : public XrJavaCoordinator {
|
||||
NOTREACHED();
|
||||
}
|
||||
void EndSession() override {}
|
||||
void EndSession(JavaShutdownCallback destroyed_callback) override {}
|
||||
|
||||
bool EnsureARCoreLoaded() override { return true; }
|
||||
|
||||
|
@ -10,6 +10,8 @@ import android.content.Intent;
|
||||
import android.os.Bundle;
|
||||
import android.view.SurfaceView;
|
||||
|
||||
import org.chromium.base.Log;
|
||||
|
||||
// TODO(crbug.com/40904930): Investigate if this would benefit from
|
||||
// extending ChromeBaseAppCompatActivity
|
||||
|
||||
@ -19,12 +21,17 @@ import android.view.SurfaceView;
|
||||
* browser when done cleaner.
|
||||
*/
|
||||
public class XrHostActivity extends Activity {
|
||||
private static final String TAG = "XrHostActivity";
|
||||
private static final boolean DEBUG_LOGS = false;
|
||||
|
||||
/**
|
||||
* Creates an Intent to start the {@link XrHostActivity}.
|
||||
* @param context Context to use when constructing the Intent.
|
||||
*
|
||||
* @param context Context to use when constructing the Intent.
|
||||
* @return Intent that can be used to begin presenting with OpenXR.
|
||||
*/
|
||||
public static Intent createIntent(Context context) {
|
||||
if (DEBUG_LOGS) Log.i(TAG, "createIntent");
|
||||
Intent intent = new Intent();
|
||||
intent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
|
||||
intent.setClassName(context.getPackageName(), XrHostActivity.class.getName());
|
||||
@ -33,6 +40,7 @@ public class XrHostActivity extends Activity {
|
||||
|
||||
@Override
|
||||
public void onCreate(Bundle savedInstanceState) {
|
||||
if (DEBUG_LOGS) Log.i(TAG, "onCreate");
|
||||
super.onCreate(savedInstanceState);
|
||||
|
||||
// The activity should only be created by the XrSessionCoordinator during an active session.
|
||||
@ -48,6 +56,7 @@ public class XrHostActivity extends Activity {
|
||||
|
||||
@Override
|
||||
public void onAttachedToWindow() {
|
||||
if (DEBUG_LOGS) Log.i(TAG, "onAttachedToWindow");
|
||||
super.onAttachedToWindow();
|
||||
|
||||
boolean result = XrSessionCoordinator.onXrHostActivityReady(this);
|
||||
@ -56,6 +65,7 @@ public class XrHostActivity extends Activity {
|
||||
|
||||
@Override
|
||||
public void onStop() {
|
||||
if (DEBUG_LOGS) Log.i(TAG, "onStop");
|
||||
super.onStop();
|
||||
|
||||
XrSessionCoordinator.endActiveSessionFromXrHost();
|
||||
@ -65,6 +75,7 @@ public class XrHostActivity extends Activity {
|
||||
|
||||
@Override
|
||||
public void onBackPressed() {
|
||||
if (DEBUG_LOGS) Log.i(TAG, "onBackPressed");
|
||||
super.onBackPressed();
|
||||
|
||||
XrSessionCoordinator.endActiveSessionFromXrHost();
|
||||
|
@ -216,6 +216,15 @@ public class XrSessionCoordinator {
|
||||
if (sActiveSessionInstance == null) return;
|
||||
assert (sActiveSessionInstance == this);
|
||||
|
||||
// If we have a host activity, shut it down first. Once it actually enters `onStop` we'll
|
||||
// get called again, but this time since our activity is null we'll run the rest of the
|
||||
// function.
|
||||
if (mXrHostActivity != null && mXrHostActivity.get() != null) {
|
||||
mXrHostActivity.get().finish();
|
||||
mXrHostActivity = null;
|
||||
return;
|
||||
}
|
||||
|
||||
if (mImmersiveOverlay != null) {
|
||||
mImmersiveOverlay.cleanupAndExit();
|
||||
mImmersiveOverlay = null;
|
||||
@ -227,10 +236,6 @@ public class XrSessionCoordinator {
|
||||
mWebContents = null;
|
||||
sActiveSessionInstance = null;
|
||||
sActiveSessionAvailableSupplier.set(SessionType.NONE);
|
||||
if (mXrHostActivity != null && mXrHostActivity.get() != null) {
|
||||
mXrHostActivity.get().finish();
|
||||
mXrHostActivity = null;
|
||||
}
|
||||
}
|
||||
|
||||
// Called from XrDelegateImpl and XRHostActivity
|
||||
|
@ -43,6 +43,11 @@ void OpenXrPlatformHelperAndroid::GetPlatformCreateInfo(
|
||||
std::move(shutdown_callback));
|
||||
}
|
||||
|
||||
void OpenXrPlatformHelperAndroid::PrepareForSessionShutdown(
|
||||
base::OnceClosure shutdown_ready_callback) {
|
||||
session_coordinator_->EndSession(std::move(shutdown_ready_callback));
|
||||
}
|
||||
|
||||
void OpenXrPlatformHelperAndroid::OnXrActivityReady(
|
||||
PlatformCreateInfoReadyCallback callback,
|
||||
const base::android::JavaParamRef<jobject>& activity) {
|
||||
|
@ -41,6 +41,9 @@ class OpenXrPlatformHelperAndroid : public device::OpenXrPlatformHelper {
|
||||
|
||||
XrResult DestroyInstance(XrInstance& instance) override;
|
||||
|
||||
void PrepareForSessionShutdown(
|
||||
base::OnceClosure shutdown_ready_callback) override;
|
||||
|
||||
private:
|
||||
// Creates a temporary XrInstance that's only used for querying information
|
||||
// about the OpenXR system, and not able to present session content.
|
||||
|
@ -112,9 +112,19 @@ void XrSessionCoordinator::RequestXrSession(
|
||||
}
|
||||
|
||||
void XrSessionCoordinator::EndSession() {
|
||||
// A default constructed callback is null.
|
||||
EndSession(device::JavaShutdownCallback());
|
||||
}
|
||||
|
||||
void XrSessionCoordinator::EndSession(
|
||||
device::JavaShutdownCallback shutdown_callback) {
|
||||
DVLOG(1) << __func__;
|
||||
JNIEnv* env = AttachCurrentThread();
|
||||
|
||||
if (shutdown_callback) {
|
||||
java_shutdown_callback_ = std::move(shutdown_callback);
|
||||
}
|
||||
|
||||
Java_XrSessionCoordinator_endSession(env, j_xr_session_coordinator_);
|
||||
}
|
||||
|
||||
|
@ -48,6 +48,7 @@ class XrSessionCoordinator : public device::XrJavaCoordinator {
|
||||
device::JavaShutdownCallback destroyed_callback,
|
||||
device::XrSessionButtonTouchedCallback button_touched_callback) override;
|
||||
void EndSession() override;
|
||||
void EndSession(device::JavaShutdownCallback destroyed_callback) override;
|
||||
bool EnsureARCoreLoaded() override;
|
||||
base::android::ScopedJavaLocalRef<jobject> GetCurrentActivityContext()
|
||||
override;
|
||||
|
@ -77,6 +77,11 @@ class XrJavaCoordinator {
|
||||
SurfaceTouchCallback touch_callback,
|
||||
JavaShutdownCallback destroyed_callback,
|
||||
XrSessionButtonTouchedCallback button_touched_callback) = 0;
|
||||
|
||||
// `shutdown_callback` may optionally be provided to override the previously
|
||||
// supplied `destroyed_callback`. Default constructed callbacks are considered
|
||||
// null, so this is not wrapped in std::optional.
|
||||
virtual void EndSession(JavaShutdownCallback shutdown_callback) = 0;
|
||||
virtual void EndSession() = 0;
|
||||
};
|
||||
|
||||
|
@ -118,6 +118,10 @@ OpenXrDevice::~OpenXrDevice() {
|
||||
if (request_session_callback_) {
|
||||
std::move(request_session_callback_).Run(nullptr);
|
||||
}
|
||||
|
||||
if (shutdown_request_callback_) {
|
||||
std::move(shutdown_request_callback_).Run();
|
||||
}
|
||||
}
|
||||
|
||||
void OpenXrDevice::RequestSession(
|
||||
@ -241,6 +245,10 @@ void OpenXrDevice::ForceEndSession(ExitXrPresentReason reason) {
|
||||
if (instance_ != XR_NULL_HANDLE) {
|
||||
platform_helper_->DestroyInstance(instance_);
|
||||
}
|
||||
|
||||
if (shutdown_request_callback_) {
|
||||
std::move(shutdown_request_callback_).Run();
|
||||
}
|
||||
}
|
||||
|
||||
void OpenXrDevice::OnPresentingControllerMojoConnectionError() {
|
||||
@ -249,8 +257,16 @@ void OpenXrDevice::OnPresentingControllerMojoConnectionError() {
|
||||
|
||||
void OpenXrDevice::ShutdownSession(
|
||||
mojom::XRRuntime::ShutdownSessionCallback callback) {
|
||||
ForceEndSession(ExitXrPresentReason::kBrowserShutdown);
|
||||
std::move(callback).Run();
|
||||
DVLOG(1) << __func__;
|
||||
if (!HasExclusiveSession()) {
|
||||
std::move(callback).Run();
|
||||
return;
|
||||
}
|
||||
|
||||
shutdown_request_callback_ = std::move(callback);
|
||||
platform_helper_->PrepareForSessionShutdown(base::BindOnce(
|
||||
&OpenXrDevice::ForceEndSession, weak_ptr_factory_.GetWeakPtr(),
|
||||
ExitXrPresentReason::kBrowserShutdown));
|
||||
}
|
||||
|
||||
void OpenXrDevice::SetFrameDataRestricted(bool restricted) {
|
||||
|
@ -59,6 +59,8 @@ class DEVICE_VR_EXPORT OpenXrDevice : public VRDeviceBase,
|
||||
std::unique_ptr<OpenXrExtensionHelper> extension_helper_;
|
||||
std::unique_ptr<OpenXrRenderLoop> render_loop_;
|
||||
|
||||
mojom::XRRuntime::ShutdownSessionCallback shutdown_request_callback_;
|
||||
|
||||
mojo::Receiver<mojom::XRSessionController> exclusive_controller_receiver_{
|
||||
this};
|
||||
|
||||
|
@ -86,6 +86,15 @@ class DEVICE_VR_EXPORT OpenXrPlatformHelper {
|
||||
// that require additional information via this mechanism will fail creation.
|
||||
XrResult CreateInstance(XrInstance* instance);
|
||||
|
||||
// Run any platform-specific shutdown that has to happen before the OpenXr
|
||||
// session can be ended. E.g. On Android, if there is a separate activity, it
|
||||
// needs to be destroyed before the session is shutdown so that the system
|
||||
// rendering takes over.
|
||||
// If a `shutdown_callback` was previously passed in, it will not be run, in
|
||||
// favor of this callback.
|
||||
virtual void PrepareForSessionShutdown(
|
||||
base::OnceClosure shutdown_ready_callback) = 0;
|
||||
|
||||
void CreateInstanceWithCreateInfo(
|
||||
std::optional<OpenXrCreateInfo> create_info,
|
||||
CreateInstanceCallback instance_ready_callback,
|
||||
|
@ -141,10 +141,16 @@ bool OpenXrPlatformHelperWindows::IsApiAvailable() {
|
||||
}
|
||||
|
||||
bool OpenXrPlatformHelperWindows::Initialize() {
|
||||
// Nothing to do;
|
||||
// Nothing to do.
|
||||
return true;
|
||||
}
|
||||
|
||||
void OpenXrPlatformHelperWindows::PrepareForSessionShutdown(
|
||||
base::OnceClosure shutdown_ready_callback) {
|
||||
// Nothing to do.
|
||||
std::move(shutdown_ready_callback).Run();
|
||||
}
|
||||
|
||||
XrResult OpenXrPlatformHelperWindows::CreateInstance(XrInstance* instance,
|
||||
void* create_info) {
|
||||
CHECK(instance);
|
||||
|
@ -32,6 +32,9 @@ class DEVICE_VR_EXPORT OpenXrPlatformHelperWindows
|
||||
device::mojom::XRDeviceData GetXRDeviceData() override;
|
||||
bool Initialize() override;
|
||||
|
||||
void PrepareForSessionShutdown(
|
||||
base::OnceClosure shutdown_ready_callback) override;
|
||||
|
||||
// Note that we treat the XrInstance as a singleton on Windows, so we must
|
||||
// override CreateInstance/DestroyInstance. See `OpenXrInstanceWrapper` for
|
||||
// more context.
|
||||
|
Reference in New Issue
Block a user