Skip to content

GitLab

T
tangled
Commit acae2efc authored by Piotr Bialecki's avatar Piotr Bialecki Committed by Commit Bot
Browse files
Options

WebXR: fix crash when hittestsource is cancelled post-session end 

The regression (crash) was introduced when fixing an issue with hit test
source cancellation not getting propagated to the device.

In addition, already-cancelled hit test sources should throw when they
are cancelled yet again - this was also regressed.

Changes:
- fixes the crash when cancelling a hit test source on a session that
has already ended
- ensures that double-cancelling a hit test source throws
InvalidStateError (as per spec)
- add instrumentation test that would have caught the browser crash
- add WPT that would have caught the double-cancel issue

Bug: 1113461
Change-Id: I174c48d096faa902e2210e3660874dd48ca88545
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2341179Reviewed-by: default avatarBrian Sheedy <bsheedy@chromium.org>
Reviewed-by: default avatarAlexander Cooper <alcooper@chromium.org>
Commit-Queue: Piotr Bialecki <bialpio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796168}
parent 12bca027
Hide whitespace changes
Inline Side-by-side
Showing with 267 additions and 5 deletions
chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrArHitTestTest.java
View file @ acae2efc
......@@ -92,4 +92,35 @@ public class WebXrArHitTestTest {
mWebXrArTestFramework.executeStepAndWait("stepStartHitTesting()");
mWebXrArTestFramework.endTest();
}
/**
* Tests that hit test cancellation works for hit test sources when the session has ended.
*/
@Test
@MediumTest
@XrActivityRestriction({XrActivityRestriction.SupportedActivity.ALL})
@ArPlaybackFile("chrome/test/data/xr/ar_playback_datasets/floor_session_12s_30fps.mp4")
public void testHitTestCancellationWorks() {
mWebXrArTestFramework.loadFileAndAwaitInitialization(
"webxr_test_basic_hittest_cancellation", PAGE_LOAD_TIMEOUT_S);
mWebXrArTestFramework.enterSessionWithUserGestureOrFail();
mWebXrArTestFramework.executeStepAndWait("stepStartHitTesting(false)");
mWebXrArTestFramework.endTest();
}
/**
* Tests that hit test cancellation works for transient input hit tests when the session has
* ended.
*/
@Test
@MediumTest
@XrActivityRestriction({XrActivityRestriction.SupportedActivity.ALL})
@ArPlaybackFile("chrome/test/data/xr/ar_playback_datasets/floor_session_12s_30fps.mp4")
public void testHitTestForTransientInputCancellationWorks() {
mWebXrArTestFramework.loadFileAndAwaitInitialization(
"webxr_test_basic_hittest_cancellation", PAGE_LOAD_TIMEOUT_S);
mWebXrArTestFramework.enterSessionWithUserGestureOrFail();
mWebXrArTestFramework.executeStepAndWait("stepStartHitTesting(true)");
mWebXrArTestFramework.endTest();
}
}
chrome/test/data/xr/e2e_test_files/html/webxr_test_basic_hittest_cancellation.html 0 → 100644
View file @ acae2efc
<!--
Tests that AR hit test returns results when a plane is present.
-->
<html>
<head>
<link rel="stylesheet" type="text/css" href="../resources/webxr_e2e.css">
</head>
<body>
<canvas id="webgl-canvas"></canvas>
<script src="../../../../../../third_party/blink/web_tests/resources/testharness.js"></script>
<script src="../resources/webxr_e2e.js"></script>
<script>var shouldAutoCreateNonImmersiveSession = false;</script>
<script src="../resources/webxr_boilerplate.js"></script>
<script>
setup({single_test: true});
const TestState = Object.freeze({
"Initial": 0,
"HitTestSourceRequested": 1,
"HitTestSourceAvailable": 2, // hitTestSource variable is guaranteed to be non-null in this state
"SessionEndRequested": 3,
"Done": 4,
});
let testState = TestState.Initial;
let hitTestSource = null;
function stepStartHitTesting(isTransientTest) {
const sessionInfo = sessionInfos[sessionTypes.AR];
const referenceSpace = sessionInfo.currentRefSpace;
const currentSession = sessionInfo.currentSession;
if(isTransientTest) {
currentSession.requestHitTestSourceForTransientInput({
profile: "generic-touchscreen",
offsetRay: new XRRay()
}).then((hts) => {
hitTestSource = hts;
testState = TestState.HitTestSourceAvailable;
});
} else {
currentSession.requestHitTestSource({
space: referenceSpace,
offsetRay: new XRRay()
}).then((hts) => {
hitTestSource = hts;
testState = TestState.HitTestSourceAvailable;
});
}
testState = TestState.HitTestSourceRequested;
onARFrameCallback = (session, frame) => {
switch(testState) {
case TestState.HitTestSourceAvailable: {
session.end().then(() => {
hitTestSource.cancel();
testState = TestState.Done;
done();
});
testState = TestState.SessionEndRequested;
return;
}
default:
return;
}
};
}
</script>
</body>
</html>
chrome/test/data/xr/e2e_test_files/html/webxr_test_basic_hittest_results_availability.html
View file @ acae2efc
......@@ -14,14 +14,13 @@ Tests that AR hit test results are available in rAF as soon as hit test source i
<script>
setup({single_test: true});
const TestState = Object.freeze({
"Initial": 0,
"RequestHitTestSource": 1,
"HitTestSourceRequested": 2,
"HitTestSourceAvailable": 3, // hitTestSource variable is guaranteed to be non-null in this state
"Done": 4,
});
});
let testState = null;
......
third_party/blink/renderer/modules/xr/xr_session.cc
View file @ acae2efc
......@@ -70,7 +70,9 @@ const char kIncompatibleLayer[] =
const char kInlineVerticalFOVNotSupported[] =
"This session does not support inlineVerticalFieldOfView";
const char kAnchorsNotSupported[] = "Device does not support anchors!";
const char kAnchorsNotSupportedByDevice[] = "Device does not support anchors!";
const char kHitTestNotSupportedByDevice[] = "Device does not support hit test!";
const char kDeviceDisconnected[] = "The XR device has been disconnected.";
......@@ -569,7 +571,7 @@ ScriptPromise XRSession::CreateAnchorHelper(
// Reject the promise if device doesn't support the anchors API.
if (!xr_->xrEnvironmentProviderRemote()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kAnchorsNotSupported);
kAnchorsNotSupportedByDevice);
return ScriptPromise();
}
......@@ -619,7 +621,7 @@ ScriptPromise XRSession::CreatePlaneAnchorHelper(
// Reject the promise if device doesn't support the anchors API.
if (!xr_->xrEnvironmentProviderRemote()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kAnchorsNotSupported);
kAnchorsNotSupportedByDevice);
return ScriptPromise();
}
......@@ -728,6 +730,12 @@ ScriptPromise XRSession::requestHitTestSource(
return {};
}
if (!xr_->xrEnvironmentProviderRemote()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kHitTestNotSupportedByDevice);
return {};
}
// 1. Grab the native origin from the passed in XRSpace.
base::Optional<device::mojom::blink::XRNativeOriginInformation>
maybe_native_origin = options_init && options_init->hasSpace()
......@@ -817,6 +825,12 @@ ScriptPromise XRSession::requestHitTestSourceForTransientInput(
return {};
}
if (!xr_->xrEnvironmentProviderRemote()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kHitTestNotSupportedByDevice);
return {};
}
if (RuntimeEnabledFeatures::WebXRHitTestEntityTypesEnabled() &&
options_init->hasEntityTypes() && options_init->entityTypes().IsEmpty()) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
......@@ -1878,11 +1892,30 @@ bool XRSession::RemoveHitTestSource(XRHitTestSource* hit_test_source) {
DCHECK(hit_test_source);
if (!base::Contains(hit_test_source_ids_, hit_test_source->id())) {
DVLOG(2) << __func__
<< ": hit test source was already removed, hit_test_source->id()="
<< hit_test_source->id();
return false;
}
if (ended_) {
DVLOG(1) << __func__
<< ": attempted to remove a hit test source on a session that has "
"already ended.";
// Since the session has ended, we won't be able to reach out to the device
// to remove a hit test source subscription. Just notify the caller that the
// removal was successful.
return true;
}
DCHECK_HIT_TEST_SOURCES();
hit_test_source_ids_to_hit_test_sources_.erase(hit_test_source->id());
hit_test_source_ids_.erase(hit_test_source->id());
DCHECK(xr_->xrEnvironmentProviderRemote());
xr_->xrEnvironmentProviderRemote()->UnsubscribeFromHitTest(
hit_test_source->id());
......@@ -1893,14 +1926,36 @@ bool XRSession::RemoveHitTestSource(XRHitTestSource* hit_test_source) {
bool XRSession::RemoveHitTestSource(
XRTransientInputHitTestSource* hit_test_source) {
DVLOG(2) << __func__;
DCHECK(hit_test_source);
if (!base::Contains(hit_test_source_for_transient_input_ids_,
hit_test_source->id())) {
DVLOG(2) << __func__
<< ": hit test source was already removed, hit_test_source->id()="
<< hit_test_source->id();
return false;
}
if (ended_) {
DVLOG(1) << __func__
<< ": attempted to remove a hit test source on a session that has "
"already ended.";
// Since the session has ended, we won't be able to reach out to the device
// to remove a hit test source subscription. Just notify the caller that the
// removal was successful.
return true;
}
DCHECK_HIT_TEST_SOURCES();
hit_test_source_ids_to_transient_input_hit_test_sources_.erase(
hit_test_source->id());
hit_test_source_for_transient_input_ids_.erase(hit_test_source->id());
DCHECK(xr_->xrEnvironmentProviderRemote());
xr_->xrEnvironmentProviderRemote()->UnsubscribeFromHitTest(
hit_test_source->id());
......
third_party/blink/renderer/modules/xr/xr_transient_input_hit_test_source.cc
View file @ acae2efc
......@@ -31,6 +31,8 @@ uint64_t XRTransientInputHitTestSource::id() const {
}
void XRTransientInputHitTestSource::cancel(ExceptionState& exception_state) {
DVLOG(2) << __func__;
if (!xr_session_->RemoveHitTestSource(this)) {
exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError,
kCannotCancelHitTestSource);
......
third_party/blink/web_tests/external/wpt/webxr/hit-test/ar_hittest_source_cancel.https.html 0 → 100644
View file @ acae2efc
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../resources/webxr_util.js"></script>
<script src="../resources/webxr_test_asserts.js"></script>
<script src="../resources/webxr_test_constants.js"></script>
<script src="../resources/webxr_test_constants_fake_world.js"></script>
<canvas />
<script>
// at world origin.
const VIEWER_ORIGIN_TRANSFORM = {
position: [0, 0, 0],
orientation: [0, 0, 0, 1],
};
// at world origin.
const FLOOR_ORIGIN_TRANSFORM = {
position: [0, 0, 0],
orientation: [0, 0, 0, 1],
};
const fakeDeviceInitParams = {
supportedModes: ["immersive-ar"],
views: VALID_VIEWS,
floorOrigin: FLOOR_ORIGIN_TRANSFORM, // aka mojo_from_floor
viewerOrigin: VIEWER_ORIGIN_TRANSFORM, // aka mojo_from_viewer
supportedFeatures: ALL_FEATURES,
world: createFakeWorld(5.0, 2.0, 5.0), // webxr_test_constants_fake_world.js has detailed description of the fake world
};
const test_function_generator = function(isTransientTest, isSessionEndedTest) {
return function(session, fakeDeviceController, t) {
let done = false;
return session.requestReferenceSpace('local').then((localSpace) => {
const validation_function = (hitTestSource) => {
const rAFcb = function(time, frame) {
if(isSessionEndedTest) {
// Session is marked as "ended" synchronously, there is no need to
// wait for the promise it returns to settle.
session.end();
// Currently, the specification does not say what happen
// when a hit test source gets cancelled post-session-end.
hitTestSource.cancel();
done = true;
} else {
hitTestSource.cancel();
t.step(() => {
assert_throws_dom("InvalidStateError", () => hitTestSource.cancel());
});
done = true;
}
};
session.requestAnimationFrame(rAFcb);
return t.step_wait(() => done);
};
// Same validation will happen both in transient and non-transient variant
if(isTransientTest) {
return session.requestHitTestSourceForTransientInput({
profile: "generic-touchscreen",
offsetRay: new XRRay(),
}).then(validation_function);
} else {
return session.requestHitTestSource({
space: localSpace,
offsetRay: new XRRay(),
}).then(validation_function);
}
}); // return session.requestReferenceSpace('local').then((localSpace) => { ...
}; // return function(session, fakeDeviceController, t) { ...
}
xr_session_promise_test("Ensures hit test source cancellation works when the session has not ended.",
test_function_generator(/*isTransientTest=*/false, /*isSessionEndedTest=*/false),
fakeDeviceInitParams,
'immersive-ar', { 'requiredFeatures': ['hit-test'] });
xr_session_promise_test("Ensures transient input hit test source cancellation works when the session has not ended.",
test_function_generator(/*isTransientTest=*/true, /*isSessionEndedTest=*/false),
fakeDeviceInitParams,
'immersive-ar', { 'requiredFeatures': ['hit-test'] });
xr_session_promise_test("Ensures hit test source cancellation works when the session has ended",
test_function_generator(/*isTransientTest=*/false, /*isSessionEndedTest=*/true),
fakeDeviceInitParams,
'immersive-ar', { 'requiredFeatures': ['hit-test'] });
xr_session_promise_test("Ensures transient input hit test source cancellation works when the session has ended",
test_function_generator(/*isTransientTest=*/true, /*isSessionEndedTest=*/true),
fakeDeviceInitParams,
'immersive-ar', { 'requiredFeatures': ['hit-test'] });
</script>
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