From af70b3004cb90622809fa3306dfa7d12e75a2f07 Mon Sep 17 00:00:00 2001 From: Alexander Cooper Date: Wed, 5 May 2021 01:33:59 +0000 Subject: [PATCH] Simplify ArImmersiveOverlay to only use SurfaceView ArImmersiveOverlay previously chose between using a SurfaceView or using a Dialog to show AR content based on whether or not the DOMOverlay feature was enabled. This changes it to use only a SurfaceView; however, this now requires the page to always enter fullscreen for Android AR sessions so that the SurfaceView does not appear behind any system UI. Not only does this simplify the rendering path and choices for AR, but it is required to actually show InfoBars or Prompts in front of AR Content (though there is still some outstanding work to be done before that can actually happen). Though the events weren't being forwarded to the page, this adds two further checks that the DOM Overlay feature is enabled before both sending or processing data that is used to generate the beforexrselect event. Bug: 1203490 Change-Id: I28c5bb6a8c9f2d84ecc03bc0a2084bf84c6e2f7c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2857524 Commit-Queue: Alexander Cooper Commit-Queue: Daniel Cheng Auto-Submit: Alexander Cooper Reviewed-by: Daniel Cheng Reviewed-by: Klaus Weidner Cr-Commit-Position: refs/heads/master@{#879170} --- .../components/webxr/ArImmersiveOverlay.java | 137 +++--------------- .../arcore/ar_compositor_frame_sink.cc | 8 +- device/vr/android/arcore/arcore_gl.cc | 4 +- .../renderer/core/fullscreen/fullscreen.cc | 23 ++- .../core/fullscreen/scoped_allow_fullscreen.h | 2 +- .../xr/xr_enter_fullscreen_observer.cc | 14 +- .../modules/xr/xr_enter_fullscreen_observer.h | 1 + .../blink/renderer/modules/xr/xr_session.cc | 3 +- .../blink/renderer/modules/xr/xr_system.cc | 104 +++++++------ .../blink/renderer/modules/xr/xr_system.h | 4 +- 10 files changed, 126 insertions(+), 174 deletions(-) diff --git a/components/webxr/android/java/src/org/chromium/components/webxr/ArImmersiveOverlay.java b/components/webxr/android/java/src/org/chromium/components/webxr/ArImmersiveOverlay.java index 05a84a8f53813c..88e4966bb0dc9c 100644 --- a/components/webxr/android/java/src/org/chromium/components/webxr/ArImmersiveOverlay.java +++ b/components/webxr/android/java/src/org/chromium/components/webxr/ArImmersiveOverlay.java @@ -6,20 +6,15 @@ import android.annotation.SuppressLint; import android.app.Activity; -import android.app.Dialog; -import android.content.Context; import android.content.pm.ActivityInfo; import android.graphics.PixelFormat; import android.graphics.Point; -import android.os.Build; import android.view.Display; -import android.view.Gravity; import android.view.MotionEvent; import android.view.SurfaceHolder; import android.view.SurfaceView; import android.view.View; import android.view.ViewGroup; -import android.view.WindowManager; import androidx.annotation.NonNull; @@ -29,7 +24,6 @@ import org.chromium.content_public.browser.WebContents; import org.chromium.content_public.browser.WebContentsObserver; import org.chromium.ui.display.DisplayAndroidManager; -import org.chromium.ui.widget.Toast; import java.util.HashMap; import java.util.Map; @@ -48,8 +42,9 @@ public class ArImmersiveOverlay private boolean mSurfaceReportedReady; private Integer mRestoreOrientation; private boolean mCleanupInProgress; - private SurfaceUiWrapper mSurfaceUi; + private ArSurfaceView mArSurfaceView; private WebContents mWebContents; + private boolean mUseOverlay; // Set containing all currently touching pointers. private HashMap mPointerIdToData; @@ -70,95 +65,11 @@ public void show(@NonNull ArCompositorDelegate compositorDelegate, mPointerIdToData = new HashMap(); mPrimaryPointerId = null; + mUseOverlay = useOverlay; + // Choose a concrete implementation to create a drawable Surface and make it fullscreen. // It forwards SurfaceHolder callbacks and touch events to this ArImmersiveOverlay object. - if (useOverlay) { - mSurfaceUi = new SurfaceUiCompositor(canRenderDomContent); - } else { - mSurfaceUi = new SurfaceUiDialog(); - } - } - - private interface SurfaceUiWrapper { - public void onSurfaceVisible(); - public void forwardMotionEvent(MotionEvent ev); - public void destroy(); - } - - // The default Dialog cancellation behavior destroys the Surface before we get notified via the - // Cancelation callback. This is unfortunate, because we need to ensure that the compositor is - // stopped before the surface is destroyed. This class allows us to override the default - // cancellation behavior to properly shutdown the compositor before the surface is destroyed. It - // is unclear why the SurfaceHolder callbacks are not triggered. - private class ArDialog extends Dialog { - public ArDialog(Context context, int themeResId) { - super(context, themeResId); - } - - @Override - public void cancel() { - ArCoreJavaUtils.onBackPressed(); - super.cancel(); - } - } - - private class SurfaceUiDialog implements SurfaceUiWrapper { - private Toast mNotificationToast; - private ArDialog mDialog; - // Android supports multiple variants of fullscreen applications. Use fully-immersive - // "sticky" mode without navigation or status bars, and show a toast with a "pull from top - // and press back button to exit" prompt. - private static final int VISIBILITY_FLAGS_IMMERSIVE = View.SYSTEM_UI_FLAG_LAYOUT_STABLE - | View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION - | View.SYSTEM_UI_FLAG_FULLSCREEN | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION - | View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY; - - public SurfaceUiDialog() { - // Create a fullscreen dialog and use its backing Surface for drawing. - mDialog = new ArDialog(mActivity, android.R.style.Theme_NoTitleBar_Fullscreen); - mDialog.getWindow().setBackgroundDrawable(null); - mDialog.getWindow().takeSurface(ArImmersiveOverlay.this); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { - // Use maximum fullscreen, ignoring a notch if present. This code path is used - // for non-DOM-Overlay mode where the browser compositor view isn't visible. - // In DOM Overlay mode (SurfaceUiCompositor), Blink configures this separately - // via ViewportData::SetExpandIntoDisplayCutout. - mDialog.getWindow().setFlags(WindowManager.LayoutParams.FLAG_LAYOUT_NO_LIMITS, - WindowManager.LayoutParams.FLAG_LAYOUT_NO_LIMITS); - mDialog.getWindow().getAttributes().layoutInDisplayCutoutMode = - WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_SHORT_EDGES; - } - View view = mDialog.getWindow().getDecorView(); - view.setSystemUiVisibility(VISIBILITY_FLAGS_IMMERSIVE); - view.setOnTouchListener(ArImmersiveOverlay.this); - view.setKeepScreenOn(true); - mDialog.getWindow().setLayout( - ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT); - mDialog.show(); - } - - @Override // SurfaceUiWrapper - public void onSurfaceVisible() { - if (mNotificationToast != null) { - mNotificationToast.cancel(); - } - int resId = R.string.immersive_fullscreen_api_notification; - mNotificationToast = Toast.makeText(mActivity, resId, Toast.LENGTH_LONG); - mNotificationToast.setGravity(Gravity.TOP | Gravity.CENTER, 0, 0); - mNotificationToast.show(); - } - - @Override // SurfaceUiWrapper - public void forwardMotionEvent(MotionEvent ev) {} - - @Override // SurfaceUiWrapper - public void destroy() { - if (mNotificationToast != null) { - mNotificationToast.cancel(); - mNotificationToast = null; - } - mDialog.dismiss(); - } + mArSurfaceView = new ArSurfaceView(canRenderDomContent); } private class PointerData { @@ -173,17 +84,18 @@ public PointerData(float x, float y, boolean touching) { } } - private class SurfaceUiCompositor implements SurfaceUiWrapper { + private class ArSurfaceView { private SurfaceView mSurfaceView; private WebContentsObserver mWebContentsObserver; private boolean mDomSurfaceNeedsConfiguring; @SuppressLint("ClickableViewAccessibility") - public SurfaceUiCompositor(boolean canRenderDomContent) { - // If we can't render the dom content on top of the camera/gl layers manually, then - // we need to configure the DOM content's surface view to overlay ours. We need to - // track this so that we ensure we teardown everything we need to teardown as well. - mDomSurfaceNeedsConfiguring = !canRenderDomContent; + public ArSurfaceView(boolean canRenderDomContent) { + // If we need to show the dom content, but can't render it on top of the camera/gl + // layers manually, then we need to configure the DOM content's surface view to + // overlay ours. We need to track this so that we ensure we teardown everything + // we need to teardown as well. + mDomSurfaceNeedsConfiguring = mUseOverlay && !canRenderDomContent; // Enable alpha channel for the compositor and make the background transparent. // Note that this needs to happen before we create and parent our SurfaceView, so that @@ -191,6 +103,10 @@ public SurfaceUiCompositor(boolean canRenderDomContent) { if (DEBUG_LOGS) { Log.i(TAG, "calling mArCompositorDelegate.setOverlayImmersiveArMode(true)"); } + + // While it's fine to omit if the page does not use DOMOverlay, once the page does + // use DOMOverlay, something appears to have changed such that it becomes required, + // otherwies the DOM SurfaceView will be in front of the XR content. mArCompositorDelegate.setOverlayImmersiveArMode(true, mDomSurfaceNeedsConfiguring); mSurfaceView = new SurfaceView(mActivity); @@ -226,15 +142,6 @@ public void didToggleFullscreenModeForTab( mWebContents.addObserver(mWebContentsObserver); } - @Override // SurfaceUiWrapper - public void onSurfaceVisible() {} - - @Override // SurfaceUiWrapper - public void forwardMotionEvent(MotionEvent ev) { - mArCompositorDelegate.dispatchTouchEvent(ev); - } - - @Override // SurfaceUiWrapper public void destroy() { mWebContents.removeObserver(mWebContentsObserver); View content = mActivity.getWindow().findViewById(android.R.id.content); @@ -400,7 +307,9 @@ public boolean onTouch(View v, MotionEvent ev) { // We need to consume the touch (returning true) to ensure that we get // followup events such as MOVE and UP. DOM Overlay mode needs to forward // the touch to the content view so that its UI elements keep working. - mSurfaceUi.forwardMotionEvent(ev); + if (mUseOverlay) { + mArCompositorDelegate.dispatchTouchEvent(ev); + } return true; } @@ -480,7 +389,7 @@ public void surfaceChanged(SurfaceHolder holder, int format, int width, int heig // // While it would be preferable to wait until the surface is at the desired fullscreen // resolution, i.e. via mActivity.getFullscreenManager().getPersistentFullscreenMode(), that - // causes a chicken-and-egg problem for SurfaceUiCompositor mode as used for DOM overlay. + // causes a chicken-and-egg problem for ArSurfaceView mode as used for DOM overlay. // Chrome's fullscreen mode is triggered by the Blink side setting an element fullscreen // after the session starts, but the session doesn't start until we report the drawing // surface being ready (including a configured size), so we use this reported size assuming @@ -505,10 +414,6 @@ public void surfaceChanged(SurfaceHolder holder, int format, int width, int heig mArCoreJavaUtils.onDrawingSurfaceReady(holder.getSurface(), mWebContents.getTopLevelNativeWindow(), rotation, width, height); mSurfaceReportedReady = true; - - // Show the toast with instructions how to exit fullscreen mode now if necessary. - // Not needed in DOM overlay mode which uses FullscreenHtmlApiHandler to do so. - mSurfaceUi.onSurfaceVisible(); } @Override // SurfaceHolder.Callback2 @@ -533,7 +438,7 @@ public void cleanupAndExit() { // the destroy callbacks to ensure consistent state after non-exiting lifecycle events. mArCoreJavaUtils.onDrawingSurfaceDestroyed(); - mSurfaceUi.destroy(); + mArSurfaceView.destroy(); // The JS app may have put an element into fullscreen mode during the immersive session, // even if this wasn't visible to the user. Ensure that we fully exit out of any active diff --git a/device/vr/android/arcore/ar_compositor_frame_sink.cc b/device/vr/android/arcore/ar_compositor_frame_sink.cc index a54b3b09fac42c..f3c435db97f09a 100644 --- a/device/vr/android/arcore/ar_compositor_frame_sink.cc +++ b/device/vr/android/arcore/ar_compositor_frame_sink.cc @@ -357,7 +357,13 @@ viz::CompositorFrame ArCompositorFrameSink::CreateFrame(WebXrFrame* xr_frame, // First the DOM, if it's enabled if (should_composite_dom_overlay_) { auto dom_surface_id = xr_frame_sink_client_->GetDOMSurface(); - if (dom_surface_id && dom_surface_id->is_valid()) { + bool can_composite_dom_overlay = + dom_surface_id && dom_surface_id->is_valid(); + DVLOG(3) + << __func__ + << " Attempting to composite DOMOverlay, can_composite_dom_overlay=" + << can_composite_dom_overlay; + if (can_composite_dom_overlay) { viz::SharedQuadState* dom_quad_state = render_pass->CreateAndAppendSharedQuadState(); dom_quad_state->SetAll( diff --git a/device/vr/android/arcore/arcore_gl.cc b/device/vr/android/arcore/arcore_gl.cc index 6eee07b7ef9fbe..bda6598ee9d6ed 100644 --- a/device/vr/android/arcore/arcore_gl.cc +++ b/device/vr/android/arcore/arcore_gl.cc @@ -1654,7 +1654,9 @@ std::vector ArCoreGl::GetInputSourceStates() { } // Save the touch point for use in Blink's XR input event deduplication. - state->overlay_pointer_position = screen_last_touch; + if (IsFeatureEnabled(device::mojom::XRSessionFeature::DOM_OVERLAY)) { + state->overlay_pointer_position = screen_last_touch; + } state->description = device::mojom::XRInputSourceDescription::New(); diff --git a/third_party/blink/renderer/core/fullscreen/fullscreen.cc b/third_party/blink/renderer/core/fullscreen/fullscreen.cc index bd3ca9b6af4dba..7360bbeeff7e06 100644 --- a/third_party/blink/renderer/core/fullscreen/fullscreen.cc +++ b/third_party/blink/renderer/core/fullscreen/fullscreen.cc @@ -283,16 +283,25 @@ bool AllowedToRequestFullscreen(Document& document) { // // The current implementation of WebXR's "dom-overlay" mode internally uses // the Fullscreen API to show a single DOM element based on configuration at - // XR session start. The WebXR API doesn't support changing elements during - // the session, so to avoid inconsistencies between implementations we need - // to block changes via Fullscreen API while the XR session is active, while - // still allowing the XR code to set up fullscreen mode on session start. + // XR session start. In addition, for WebXR sessions without "dom-overlay" + // the renderer may need to force the page to fullscreen to ensure that + // browser UI hides/responds accordingly. In either case, requesting a WebXR + // Session does require a user gesture, but it has likely expired by the time + // the renderer actually gets the XR session from the device and attempts + // to fullscreen the page. if (ScopedAllowFullscreen::FullscreenAllowedReason() == - ScopedAllowFullscreen::kXrOverlay) { - DVLOG(1) << __func__ - << ": allowing fullscreen element setup for XR DOM overlay"; + ScopedAllowFullscreen::kXrOverlay || + ScopedAllowFullscreen::FullscreenAllowedReason() == + ScopedAllowFullscreen::kXrSession) { + DVLOG(1) << __func__ << ": allowing fullscreen element setup for XR"; return true; } + + // The WebXR API doesn't support changing elements during the session if the + // dom-overlay feature is in use (indicated by the IsXrOverlay property). To + // avoid inconsistencies between implementations we need to block changes via + // Fullscreen API while the XR session is active, while still allowing the XR + // code to set up fullscreen mode on session start. if (document.IsXrOverlay()) { DVLOG(1) << __func__ << ": rejecting change of fullscreen element for XR DOM overlay"; diff --git a/third_party/blink/renderer/core/fullscreen/scoped_allow_fullscreen.h b/third_party/blink/renderer/core/fullscreen/scoped_allow_fullscreen.h index fb2b80f90ddd24..caf10561e9573f 100644 --- a/third_party/blink/renderer/core/fullscreen/scoped_allow_fullscreen.h +++ b/third_party/blink/renderer/core/fullscreen/scoped_allow_fullscreen.h @@ -16,7 +16,7 @@ class CORE_EXPORT ScopedAllowFullscreen { STACK_ALLOCATED(); public: - enum Reason { kOrientationChange, kXrOverlay }; + enum Reason { kOrientationChange, kXrOverlay, kXrSession }; static base::Optional FullscreenAllowedReason(); explicit ScopedAllowFullscreen(Reason); diff --git a/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.cc b/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.cc index 080228335dc503..fde4fa76c787a7 100644 --- a/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.cc +++ b/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.cc @@ -44,6 +44,7 @@ void XrEnterFullscreenObserver::Invoke(ExecutionContext* execution_context, void XrEnterFullscreenObserver::RequestFullscreen( Element* fullscreen_element, + bool setup_for_dom_overlay, base::OnceCallback on_completed) { DCHECK(!on_completed_); DCHECK(fullscreen_element); @@ -84,11 +85,16 @@ void XrEnterFullscreenObserver::RequestFullscreen( // immersive session had required a user activation state, but that may have // expired by now due to the user taking time to respond to the consent // prompt. - ScopedAllowFullscreen scope(ScopedAllowFullscreen::kXrOverlay); + ScopedAllowFullscreen scope(setup_for_dom_overlay + ? ScopedAllowFullscreen::kXrOverlay + : ScopedAllowFullscreen::kXrSession); - Fullscreen::RequestFullscreen(*fullscreen_element_, options, - FullscreenRequestType::kUnprefixed | - FullscreenRequestType::kForXrOverlay); + FullscreenRequestType request_type = FullscreenRequestType::kUnprefixed; + if (setup_for_dom_overlay) { + request_type = request_type | FullscreenRequestType::kForXrOverlay; + } + + Fullscreen::RequestFullscreen(*fullscreen_element_, options, request_type); if (!wait_for_fullscreen_change) { // Element was already fullscreen, proceed with session creation. diff --git a/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.h b/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.h index 8b8756c05c8239..d687dc6259313c 100644 --- a/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.h +++ b/third_party/blink/renderer/modules/xr/xr_enter_fullscreen_observer.h @@ -26,6 +26,7 @@ class XrEnterFullscreenObserver : public NativeEventListener { // Attempt to enter fullscreen with |element| as the root. |on_completed| will // be notified with whether or not fullscreen was successfully entered. void RequestFullscreen(Element* element, + bool setup_for_dom_overlay, base::OnceCallback on_completed); void Trace(Visitor*) const override; diff --git a/third_party/blink/renderer/modules/xr/xr_session.cc b/third_party/blink/renderer/modules/xr/xr_session.cc index cf78b72640b5e0..eec8382f797335 100644 --- a/third_party/blink/renderer/modules/xr/xr_session.cc +++ b/third_party/blink/renderer/modules/xr/xr_session.cc @@ -2018,7 +2018,8 @@ void XRSession::OnInputStateChangeInternal( // cross-origin content. If that's the case, the input source is set as // invisible, and must not return poses or hit test results. bool hide_input_source = false; - if (overlay_element_ && input_state->overlay_pointer_position) { + if (IsFeatureEnabled(device::mojom::XRSessionFeature::DOM_OVERLAY) && + overlay_element_ && input_state->overlay_pointer_position) { input_source->ProcessOverlayHitTest(overlay_element_, input_state); if (!stored_input_source && !input_source->IsVisible()) { DVLOG(2) << __func__ << ": (new) hidden_input_source"; diff --git a/third_party/blink/renderer/modules/xr/xr_system.cc b/third_party/blink/renderer/modules/xr/xr_system.cc index 7dc5b990e5896d..143424a145e7ff 100644 --- a/third_party/blink/renderer/modules/xr/xr_system.cc +++ b/third_party/blink/renderer/modules/xr/xr_system.cc @@ -6,6 +6,7 @@ #include +#include "build/build_config.h" #include "device/vr/public/mojom/vr_service.mojom-blink.h" #include "services/metrics/public/cpp/ukm_builders.h" #include "third_party/blink/public/common/browser_interface_broker_proxy.h" @@ -1116,17 +1117,10 @@ void XRSystem::RequestImmersiveSession(PendingRequestSessionQuery* query, void XRSystem::DoRequestSession( PendingRequestSessionQuery* query, device::mojom::blink::XRSessionOptionsPtr session_options) { - // In DOM overlay mode, there's an additional step before an immersive-ar - // session can start, we need to enter fullscreen mode by setting the - // appropriate element as fullscreen from the Renderer, then waiting for the - // browser side to send an event indicating success or failure. - auto callback = - query->DOMOverlayElement() - ? WTF::Bind(&XRSystem::OnRequestSessionSetupForDomOverlay, - WrapWeakPersistent(this), WrapPersistent(query)) - : WTF::Bind(&XRSystem::OnRequestSessionReturned, - WrapWeakPersistent(this), WrapPersistent(query)); - service_->RequestSession(std::move(session_options), std::move(callback)); + service_->RequestSession( + std::move(session_options), + WTF::Bind(&XRSystem::OnRequestSessionReturned, WrapWeakPersistent(this), + WrapPersistent(query))); } void XRSystem::RequestInlineSession(PendingRequestSessionQuery* query, @@ -1455,39 +1449,72 @@ void XRSystem::OnSupportsSessionReturned(PendingSupportsSessionQuery* query, query->Resolve(supports_session); } -void XRSystem::OnRequestSessionSetupForDomOverlay( +void XRSystem::OnRequestSessionReturned( PendingRequestSessionQuery* query, device::mojom::blink::RequestSessionResultPtr result) { - DCHECK(query->DOMOverlayElement()); - if (result->is_success()) { - // Success. Now request fullscreen mode and continue with - // OnRequestSessionReturned once that completes. - fullscreen_enter_observer_ = - MakeGarbageCollected(); - fullscreen_enter_observer_->RequestFullscreen( - query->DOMOverlayElement(), - WTF::Bind(&XRSystem::OnFullscreenConfigured, WrapPersistent(this), - WrapPersistent(query), std::move(result))); - } else { - // Session request failed, continue processing that normally. - OnRequestSessionReturned(query, std::move(result)); + // If session creation failed, move straight on to processing that. + if (!result->is_success()) { + FinishSessionCreation(query, std::move(result)); + return; + } + + Element* fullscreen_element = nullptr; + const auto& enabled_features = + result->get_success()->session->enabled_features; + if (base::Contains(enabled_features, + device::mojom::XRSessionFeature::DOM_OVERLAY)) { + fullscreen_element = query->DOMOverlayElement(); } + + // Only setup for dom_overlay if the query actually had a DOMOverlayElement + // and the session enabled dom_overlay. (Note that fullscreen_element will be + // null if the feature was not enabled). + bool setup_for_dom_overlay = !!fullscreen_element; + +// On Android, due to the way the device renderer is configured, we always need +// to enter fullscreen if we're starting an AR session, so if we aren't supposed +// to enter DOMOverlay, we simply fullscreen the document body. +#if defined(OS_ANDROID) + if (!fullscreen_element && + query->mode() == device::mojom::blink::XRSessionMode::kImmersiveAr) { + fullscreen_element = DomWindow()->document()->body(); + } +#endif + + // If we don't need to enter fullscreen continue with session setup. + if (!fullscreen_element) { + FinishSessionCreation(query, std::move(result)); + return; + } + + // At this point, we know that we have an element that we need to make + // fullscreen, so we do that before we continue setting up the session. + fullscreen_enter_observer_ = + MakeGarbageCollected(); + fullscreen_enter_observer_->RequestFullscreen( + fullscreen_element, setup_for_dom_overlay, + WTF::Bind(&XRSystem::OnFullscreenConfigured, WrapPersistent(this), + WrapPersistent(query), std::move(result))); } void XRSystem::OnFullscreenConfigured( PendingRequestSessionQuery* query, device::mojom::blink::RequestSessionResultPtr result, bool fullscreen_succeeded) { + // At this point we no longer need the enter observer, so go ahead and destroy + // it. + fullscreen_enter_observer_ = nullptr; + if (fullscreen_succeeded) { - OnRequestSessionReturned(query, std::move(result)); + FinishSessionCreation(query, std::move(result)); } else { - OnRequestSessionReturned( + FinishSessionCreation( query, device::mojom::blink::RequestSessionResult::NewFailureReason( device::mojom::RequestSessionError::FULLSCREEN_ERROR)); } } -void XRSystem::OnRequestSessionReturned( +void XRSystem::FinishSessionCreation( PendingRequestSessionQuery* query, device::mojom::blink::RequestSessionResultPtr result) { DVLOG(2) << __func__; @@ -1501,10 +1528,6 @@ void XRSystem::OnRequestSessionReturned( has_outstanding_immersive_request_ = false; } - // Clean up the fullscreen event manager which may have been added for - // DOM overlay setup. - fullscreen_enter_observer_ = nullptr; - if (!result->is_success()) { // |service_| does not support the requested mode. Attempt to create a // sensorless session. @@ -1567,15 +1590,14 @@ void XRSystem::OnRequestSessionReturned( session->OnEnvironmentProviderCreated(); } - if (query->mode() == device::mojom::blink::XRSessionMode::kImmersiveAr) { - DCHECK(DomWindow()); - if (query->HasFeature(device::mojom::XRSessionFeature::DOM_OVERLAY)) { - DCHECK(query->DOMOverlayElement()); - // The session is using DOM overlay mode. At this point the overlay - // element is already in fullscreen mode, and the session can - // proceed. - session->SetDOMOverlayElement(query->DOMOverlayElement()); - } + auto dom_overlay_feature = device::mojom::XRSessionFeature::DOM_OVERLAY; + if (query->mode() == device::mojom::blink::XRSessionMode::kImmersiveAr && + query->HasFeature(dom_overlay_feature) && + base::Contains(enabled_features, dom_overlay_feature)) { + DCHECK(query->DOMOverlayElement()); + // The session is using DOM overlay mode. At this point the overlay + // element is already in fullscreen mode, and the session can proceed. + session->SetDOMOverlayElement(query->DOMOverlayElement()); } if (query->mode() == device::mojom::blink::XRSessionMode::kImmersiveVr && diff --git a/third_party/blink/renderer/modules/xr/xr_system.h b/third_party/blink/renderer/modules/xr/xr_system.h index 6f52a7d5195409..d4af864988c085 100644 --- a/third_party/blink/renderer/modules/xr/xr_system.h +++ b/third_party/blink/renderer/modules/xr/xr_system.h @@ -359,14 +359,14 @@ class XRSystem final : public EventTargetWithInlineData, void DoRequestSession( PendingRequestSessionQuery* query, device::mojom::blink::XRSessionOptionsPtr session_options); - void OnRequestSessionSetupForDomOverlay( + void OnRequestSessionReturned( PendingRequestSessionQuery*, device::mojom::blink::RequestSessionResultPtr result); void OnFullscreenConfigured( PendingRequestSessionQuery* query, device::mojom::blink::RequestSessionResultPtr result, bool fullscreen_succeeded); - void OnRequestSessionReturned( + void FinishSessionCreation( PendingRequestSessionQuery*, device::mojom::blink::RequestSessionResultPtr result); void OnSupportsSessionReturned(PendingSupportsSessionQuery*,