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}

third_party/blink/renderer/modules/xr/xr_frame.cc

@@ -10,17 +10,30 @@
 #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_view.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
 
 namespace blink {
 
+namespace {
+
+const char kInactiveFrame[] =
+    "XRFrame access outside the callback that produced it is invalid.";
+}
+
 XRFrame::XRFrame(XRSession* session) : session_(session) {}
 
 const HeapVector<Member<XRView>>& XRFrame::views() const {
   return session_->views();
 }
 
-XRDevicePose* XRFrame::getDevicePose(
-    XRCoordinateSystem* coordinate_system) const {
+XRDevicePose* XRFrame::getDevicePose(XRCoordinateSystem* coordinate_system,
+                                     ExceptionState& exception_state) const {
+  if (!active_) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
+                                      kInactiveFrame);
+    return nullptr;
+  }
+
   session_->LogGetPose();
 
   // If we don't have a valid base pose return null. Most common when tracking
@@ -44,9 +57,15 @@ XRDevicePose* XRFrame::getDevicePose(
   return MakeGarbageCollected<XRDevicePose>(session(), std::move(pose));
 }
 
-XRInputPose* XRFrame::getInputPose(
-    XRInputSource* input_source,
-    XRCoordinateSystem* coordinate_system) const {
+XRInputPose* XRFrame::getInputPose(XRInputSource* input_source,
+                                   XRCoordinateSystem* coordinate_system,
+                                   ExceptionState& exception_state) const {
+  if (!active_) {
+    exception_state.ThrowDOMException(DOMExceptionCode::kNotAllowedError,
+                                      kInactiveFrame);
+    return nullptr;
+  }
+
   if (!input_source || !coordinate_system) {
     return nullptr;
   }
@@ -123,6 +142,10 @@ void XRFrame::SetBasePoseMatrix(const TransformationMatrix& base_pose_matrix) {
   base_pose_matrix_ = TransformationMatrix::Create(base_pose_matrix);
 }
 
+void XRFrame::Deactivate() {
+  active_ = false;
+}
+
 void XRFrame::Trace(blink::Visitor* visitor) {
   visitor->Trace(session_);
   ScriptWrappable::Trace(visitor);

third_party/blink/renderer/modules/xr/xr_frame.h

@@ -14,6 +14,7 @@
 
 namespace blink {
 
+class ExceptionState;
 class XRCoordinateSystem;
 class XRDevicePose;
 class XRInputPose;
@@ -30,16 +31,21 @@ class XRFrame final : public ScriptWrappable {
   XRSession* session() const { return session_; }
 
   const HeapVector<Member<XRView>>& views() const;
-  XRDevicePose* getDevicePose(XRCoordinateSystem*) const;
-  XRInputPose* getInputPose(XRInputSource*, XRCoordinateSystem*) const;
+  XRDevicePose* getDevicePose(XRCoordinateSystem*, ExceptionState&) const;
+  XRInputPose* getInputPose(XRInputSource*,
+                            XRCoordinateSystem*,
+                            ExceptionState&) const;
 
   void SetBasePoseMatrix(const TransformationMatrix&);
 
   void Trace(blink::Visitor*) override;
 
+  void Deactivate();
+
  private:
   const Member<XRSession> session_;
   std::unique_ptr<TransformationMatrix> base_pose_matrix_;
+  bool active_ = true;
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/xr/xr_frame.idl

@@ -11,7 +11,7 @@
   readonly attribute XRSession session;
   readonly attribute FrozenArray<XRView> views;
 
-  XRDevicePose? getDevicePose(XRCoordinateSystem coordinateSystem);
-  XRInputPose? getInputPose(XRInputSource inputSource,
+  [RaisesException] XRDevicePose? getDevicePose(XRCoordinateSystem coordinateSystem);
+  [RaisesException] XRInputPose? getInputPose(XRInputSource inputSource,
                             XRCoordinateSystem coordinateSystem);
 };

third_party/blink/renderer/modules/xr/xr_session.cc

@@ -487,9 +487,9 @@ void XRSession::OnFrame(
   if (!base_layer_)
     return;
 
-  XRFrame* presentation_frame = CreatePresentationFrame();
-
   if (pending_frame_) {
+    XRFrame* presentation_frame = CreatePresentationFrame();
+
     pending_frame_ = false;
 
     // Make sure that any frame-bounded changed to the views array take effect.
@@ -521,6 +521,9 @@ void XRSession::OnFrame(
     // OnFrameEnd if it's still valid.
     if (!ended_)
       frame_base_layer->OnFrameEnd();
+
+    // Ensure the XRFrame cannot be used outside the callbacks.
+    presentation_frame->Deactivate();
   }
 }
 
@@ -633,6 +636,9 @@ void XRSession::OnSelectStart(XRInputSource* input_source) {
 
   if (event->defaultPrevented())
     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) {
@@ -655,6 +661,9 @@ void XRSession::OnSelectEnd(XRInputSource* input_source) {
 
   if (event->defaultPrevented())
     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) {
@@ -672,6 +681,9 @@ void XRSession::OnSelect(XRInputSource* input_source) {
     XRInputSourceEvent* event =
         CreateInputSourceEvent(event_type_names::kSelect, input_source);
     DispatchEvent(*event);
+
+    // Ensure the frame cannot be used outside of the event handler.
+    event->frame()->Deactivate();
   }
 }
 

third_party/blink/web_tests/external/wpt/webxr/xrFrame_lifetime.https.html

+<!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