Commit 0b9341b1 authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

Revert "Enforce active lifetime of XRFrame objects"

This reverts commit ff451c7b.

Reason for revert: Suspect of causing consistent failure of xr_browser_tests on Win10 Debug (NVIDIA) bot.

See 
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Debug%20%28NVIDIA%29

First failure:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Win10%20Debug%20%28NVIDIA%29/10962



Original change's description:
> Enforce active lifetime of XRFrame objects
> 
> Deactivates XRFrame objects once the relevant callback returns, which
> causes future calls to the object methods to throw an exception.
> 
> Bug: 906842
> Change-Id: I192453f9f23b1b4d9a404daca352ab80a5393753
> Reviewed-on: https://chromium-review.googlesource.com/c/1343065
> Commit-Queue: Brandon Jones <bajones@chromium.org>
> Reviewed-by: Klaus Weidner <klausw@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#612887}

TBR=bajones@chromium.org,klausw@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 906842
Change-Id: I26eb3477ea875362a5f1e6fd0b67bbe4a3c2c4a5
Reviewed-on: https://chromium-review.googlesource.com/c/1356945Reviewed-by: default avatarGuido Urdaneta <guidou@chromium.org>
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613039}
parent 32376a5f
...@@ -10,30 +10,17 @@ ...@@ -10,30 +10,17 @@
#include "third_party/blink/renderer/modules/xr/xr_input_source.h" #include "third_party/blink/renderer/modules/xr/xr_input_source.h"
#include "third_party/blink/renderer/modules/xr/xr_session.h" #include "third_party/blink/renderer/modules/xr/xr_session.h"
#include "third_party/blink/renderer/modules/xr/xr_view.h" #include "third_party/blink/renderer/modules/xr/xr_view.h"
#include "third_party/blink/renderer/platform/bindings/exception_state.h"
namespace blink { namespace blink {
namespace {
const char kInactiveFrame[] =
"XRFrame access outside the callback that produced it is invalid.";
}
XRFrame::XRFrame(XRSession* session) : session_(session) {} XRFrame::XRFrame(XRSession* session) : session_(session) {}
const HeapVector<Member<XRView>>& XRFrame::views() const { const HeapVector<Member<XRView>>& XRFrame::views() const {
return session_->views(); return session_->views();
} }
XRDevicePose* XRFrame::getDevicePose(XRCoordinateSystem* coordinate_system, XRDevicePose* XRFrame::getDevicePose(
ExceptionState& exception_state) const { XRCoordinateSystem* coordinate_system) const {
if (!active_) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
kInactiveFrame);
return nullptr;
}
session_->LogGetPose(); session_->LogGetPose();
// If we don't have a valid base pose return null. Most common when tracking // If we don't have a valid base pose return null. Most common when tracking
...@@ -57,15 +44,9 @@ XRDevicePose* XRFrame::getDevicePose(XRCoordinateSystem* coordinate_system, ...@@ -57,15 +44,9 @@ XRDevicePose* XRFrame::getDevicePose(XRCoordinateSystem* coordinate_system,
return MakeGarbageCollected<XRDevicePose>(session(), std::move(pose)); return MakeGarbageCollected<XRDevicePose>(session(), std::move(pose));
} }
XRInputPose* XRFrame::getInputPose(XRInputSource* input_source, XRInputPose* XRFrame::getInputPose(
XRCoordinateSystem* coordinate_system, XRInputSource* input_source,
ExceptionState& exception_state) const { XRCoordinateSystem* coordinate_system) const {
if (!active_) {
exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
kInactiveFrame);
return nullptr;
}
if (!input_source || !coordinate_system) { if (!input_source || !coordinate_system) {
return nullptr; return nullptr;
} }
...@@ -142,10 +123,6 @@ void XRFrame::SetBasePoseMatrix(const TransformationMatrix& base_pose_matrix) { ...@@ -142,10 +123,6 @@ void XRFrame::SetBasePoseMatrix(const TransformationMatrix& base_pose_matrix) {
base_pose_matrix_ = TransformationMatrix::Create(base_pose_matrix); base_pose_matrix_ = TransformationMatrix::Create(base_pose_matrix);
} }
void XRFrame::Deactivate() {
active_ = false;
}
void XRFrame::Trace(blink::Visitor* visitor) { void XRFrame::Trace(blink::Visitor* visitor) {
visitor->Trace(session_); visitor->Trace(session_);
ScriptWrappable::Trace(visitor); ScriptWrappable::Trace(visitor);
......
...@@ -14,7 +14,6 @@ ...@@ -14,7 +14,6 @@
namespace blink { namespace blink {
class ExceptionState;
class XRCoordinateSystem; class XRCoordinateSystem;
class XRDevicePose; class XRDevicePose;
class XRInputPose; class XRInputPose;
...@@ -31,21 +30,16 @@ class XRFrame final : public ScriptWrappable { ...@@ -31,21 +30,16 @@ class XRFrame final : public ScriptWrappable {
XRSession* session() const { return session_; } XRSession* session() const { return session_; }
const HeapVector<Member<XRView>>& views() const; const HeapVector<Member<XRView>>& views() const;
XRDevicePose* getDevicePose(XRCoordinateSystem*, ExceptionState&) const; XRDevicePose* getDevicePose(XRCoordinateSystem*) const;
XRInputPose* getInputPose(XRInputSource*, XRInputPose* getInputPose(XRInputSource*, XRCoordinateSystem*) const;
XRCoordinateSystem*,
ExceptionState&) const;
void SetBasePoseMatrix(const TransformationMatrix&); void SetBasePoseMatrix(const TransformationMatrix&);
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
void Deactivate();
private: private:
const Member<XRSession> session_; const Member<XRSession> session_;
std::unique_ptr<TransformationMatrix> base_pose_matrix_; std::unique_ptr<TransformationMatrix> base_pose_matrix_;
bool active_ = true;
}; };
} // namespace blink } // namespace blink
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
readonly attribute XRSession session; readonly attribute XRSession session;
readonly attribute FrozenArray<XRView> views; readonly attribute FrozenArray<XRView> views;
[RaisesException] XRDevicePose? getDevicePose(XRCoordinateSystem coordinateSystem); XRDevicePose? getDevicePose(XRCoordinateSystem coordinateSystem);
[RaisesException] XRInputPose? getInputPose(XRInputSource inputSource, XRInputPose? getInputPose(XRInputSource inputSource,
XRCoordinateSystem coordinateSystem); XRCoordinateSystem coordinateSystem);
}; };
...@@ -487,9 +487,9 @@ void XRSession::OnFrame( ...@@ -487,9 +487,9 @@ void XRSession::OnFrame(
if (!base_layer_) if (!base_layer_)
return; return;
if (pending_frame_) { XRFrame* presentation_frame = CreatePresentationFrame();
XRFrame* presentation_frame = CreatePresentationFrame();
if (pending_frame_) {
pending_frame_ = false; pending_frame_ = false;
// Make sure that any frame-bounded changed to the views array take effect. // Make sure that any frame-bounded changed to the views array take effect.
...@@ -521,9 +521,6 @@ void XRSession::OnFrame( ...@@ -521,9 +521,6 @@ void XRSession::OnFrame(
// OnFrameEnd if it's still valid. // OnFrameEnd if it's still valid.
if (!ended_) if (!ended_)
frame_base_layer->OnFrameEnd(); frame_base_layer->OnFrameEnd();
// Ensure the XRFrame cannot be used outside the callbacks.
presentation_frame->Deactivate();
} }
} }
...@@ -636,9 +633,6 @@ void XRSession::OnSelectStart(XRInputSource* input_source) { ...@@ -636,9 +633,6 @@ void XRSession::OnSelectStart(XRInputSource* input_source) {
if (event->defaultPrevented()) if (event->defaultPrevented())
input_source->selection_cancelled = true; input_source->selection_cancelled = true;
// Ensure the frame cannot be used outside of the event handler.
event->frame()->Deactivate();
} }
void XRSession::OnSelectEnd(XRInputSource* input_source) { void XRSession::OnSelectEnd(XRInputSource* input_source) {
...@@ -661,9 +655,6 @@ void XRSession::OnSelectEnd(XRInputSource* input_source) { ...@@ -661,9 +655,6 @@ void XRSession::OnSelectEnd(XRInputSource* input_source) {
if (event->defaultPrevented()) if (event->defaultPrevented())
input_source->selection_cancelled = true; input_source->selection_cancelled = true;
// Ensure the frame cannot be used outside of the event handler.
event->frame()->Deactivate();
} }
void XRSession::OnSelect(XRInputSource* input_source) { void XRSession::OnSelect(XRInputSource* input_source) {
...@@ -681,9 +672,6 @@ void XRSession::OnSelect(XRInputSource* input_source) { ...@@ -681,9 +672,6 @@ void XRSession::OnSelect(XRInputSource* input_source) {
XRInputSourceEvent* event = XRInputSourceEvent* event =
CreateInputSourceEvent(event_type_names::kSelect, input_source); CreateInputSourceEvent(event_type_names::kSelect, input_source);
DispatchEvent(*event); DispatchEvent(*event);
// Ensure the frame cannot be used outside of the event handler.
event->frame()->Deactivate();
} }
} }
......
<!DOCTYPE html>
<body>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src="resources/webxr_util.js"></script>
<canvas></canvas>
<script>
let immersiveTestName = "XRFrame methods throw exceptions outside of the " +
"requestAnimationFrame callback for immersive sessions";
let nonImmersiveTestName = "XRFrame methods throw exceptions outside of the " +
"requestAnimationFrame callback for non-immersive sessions";
let fakeDeviceInitParams = { supportsImmersive:true };
let immersiveSessionOptions = { immersive: true };
let nonImmersiveSessionOptions = { outputContext: getOutputContext() };
let testFunction = (testSession, testController, t) => new Promise((resolve) => {
let staleFrame = null;
let frameOfRef = null;
function onFrame(time, xrFrame) {
t.step(() => {
assert_true(xrFrame instanceof XRFrame);
});
staleFrame = xrFrame;
step_timeout(afterFrame, 0);
}
function afterFrame() {
t.step(() => {
// Attempting to call a method on the frame outside the callback that
// originally provided it should cause it to throw an exception.
assert_throws('NotAllowedError', () => staleFrame.getDevicePose(frameOfRef));
});
// Test does not complete until the this function has executed.
resolve();
}
testSession.requestFrameOfReference('eye-level').then((xrFrameOfRef) => {
frameOfRef = xrFrameOfRef;
testSession.requestAnimationFrame(onFrame);
});
});
xr_session_promise_test(immersiveTestName, testFunction,
fakeDeviceInitParams, immersiveSessionOptions);
xr_session_promise_test(nonImmersiveTestName, testFunction,
fakeDeviceInitParams, nonImmersiveSessionOptions);
</script>
</body>
\ No newline at end of file
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