Ensure that XRSessions only request a runtime if needed

If the only features requested for an XRSession can be fully supported
by a sensorless session, then just create the sensorless session rather
than trying to get a fully device backed session.  There were cases
where the only requested features were "viewer", but the session was
still being backed by the orientation_sensors device.

This change introduces an enum that indicates whether sensor data, and
therefore a runtime, is needed, optional, or not required. If sensor
data is not required, then a sensorless session is created rather than
querying for device support.

This also refactors "CanRequestSensorlessInlineSession" to an enum, to
better consolidate and support this expanded logic.

Fixed: 1018794
Change-Id: I0b930a733ccab85275fcc965969a17313cb3cf9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884827
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710608}

chrome/browser/vr/webxr_vr_isolated_device_service_test.cc

@@ -26,12 +26,13 @@ WEBXR_VR_ALL_RUNTIMES_BROWSER_TEST_F(TestDeviceServiceDisconnect) {
   t->LoadUrlAndAwaitInitialization(
       t->GetFileUrlForHtmlTestFile("test_isolated_device_service_disconnect"));
 
-  // We expect one change from the initial device being available.
-  t->PollJavaScriptBooleanOrFail("deviceChanges === 1",
-                                 WebXrVrBrowserTestBase::kPollTimeoutMedium);
-
   t->EnterSessionWithUserGestureOrFail();
 
+  // We don't care how many device changes we've received prior to this point.
+  // We should now be at a steady state, so what we really care about is the
+  // number of device changes after this point.
+  t->RunJavaScriptOrFail("resetDeviceChanges()");
+
   device_hook->TerminateDeviceServiceProcessForTesting();
 
   // Ensure that we've actually exited the session.
@@ -41,7 +42,7 @@ WEBXR_VR_ALL_RUNTIMES_BROWSER_TEST_F(TestDeviceServiceDisconnect) {
 
   // We expect one change indicating the device was disconnected, and then
   // one more indicating that the device was re-connected.
-  t->PollJavaScriptBooleanOrFail("deviceChanges === 3",
+  t->PollJavaScriptBooleanOrFail("deviceChanges === 2",
                                  WebXrVrBrowserTestBase::kPollTimeoutMedium);
 
   // One last check now that we have the device change that we can actually

chrome/test/data/xr/e2e_test_files/html/test_isolated_device_service_disconnect.html

@@ -14,6 +14,10 @@ WebXR page without any code specific to one test
     <script>
       var deviceChanges = 0;
       navigator.xr.ondevicechange = () => { deviceChanges++; };
+
+      function resetDeviceChanges() {
+        deviceChanges = 0;
+      }
     </script>
   </body>
 </html>

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

@@ -313,7 +313,9 @@ XR::PendingRequestSessionQuery::PendingRequestSessionQuery(
       mode_(session_mode),
       required_features_(std::move(required_features)),
       optional_features_(std::move(optional_features)),
-      ukm_source_id_(ukm_source_id) {}
+      ukm_source_id_(ukm_source_id) {
+  ParseSensorRequirement();
+}
 
 void XR::PendingRequestSessionQuery::Resolve(XRSession* session) {
   resolver_->Resolve(session);
@@ -404,6 +406,33 @@ ScriptState* XR::PendingRequestSessionQuery::GetScriptState() const {
   return resolver_->GetScriptState();
 }
 
+void XR::PendingRequestSessionQuery::ParseSensorRequirement() {
+  // All modes other than inline require sensors.
+  if (mode_ != XRSession::kModeInline) {
+    sensor_requirement_ = SensorRequirement::kRequired;
+    return;
+  }
+
+  // If any required features require sensors, then sensors are required.
+  for (const auto& feature : RequiredFeatures()) {
+    if (feature != device::mojom::XRSessionFeature::REF_SPACE_VIEWER) {
+      sensor_requirement_ = SensorRequirement::kRequired;
+      return;
+    }
+  }
+
+  // If any optional features require sensors, then sensors are optional.
+  for (const auto& feature : OptionalFeatures()) {
+    if (feature != device::mojom::XRSessionFeature::REF_SPACE_VIEWER) {
+      sensor_requirement_ = SensorRequirement::kOptional;
+      return;
+    }
+  }
+
+  // By this point any situation that requires sensors should have returned.
+  sensor_requirement_ = kNone;
+}
+
 void XR::PendingRequestSessionQuery::Trace(blink::Visitor* visitor) {
   visitor->Trace(resolver_);
 }
@@ -672,21 +701,27 @@ void XR::RequestInlineSession(LocalFrame* frame,
     return;
   }
 
-  if (!service_) {
-    // If we don't have a service by the time we reach this call, there is no XR
-    // hardware. Create a sensorless session if possible.
-    if (CanCreateSensorlessInlineSession(query)) {
-      XRSession* session = CreateSensorlessInlineSession();
-      query->Resolve(session);
-    } else {
-      query->RejectWithDOMException(DOMExceptionCode::kNotSupportedError,
-                                    kSessionNotSupported, exception_state);
-    }
+  // Reject session if any of the required features were invalid.
+  if (query->InvalidRequiredFeatures()) {
+    query->RejectWithDOMException(DOMExceptionCode::kNotSupportedError,
+                                  kSessionNotSupported, exception_state);
     return;
   }
 
-  // Reject session if any of the required features were invalid.
-  if (query->InvalidRequiredFeatures()) {
+  auto sensor_requirement = query->GetSensorRequirement();
+
+  // If no sensors are requested, or if we don't have a service and sensors are
+  // not required, then just create a sensorless session.
+  if (sensor_requirement == SensorRequirement::kNone ||
+      (!service_ && sensor_requirement != SensorRequirement::kRequired)) {
+    query->Resolve(CreateSensorlessInlineSession());
+    return;
+  }
+
+  // If we don't have a service, then we don't have any WebXR hardware.
+  // If we didn't already create a sensorless session, we can't create a session
+  // without hardware, so just reject now.
+  if (!service_) {
     query->RejectWithDOMException(DOMExceptionCode::kNotSupportedError,
                                   kSessionNotSupported, exception_state);
     return;
@@ -883,7 +918,7 @@ void XR::OnRequestSessionReturned(
   if (!session_ptr) {
     // |service_| does not support the requested mode. Attempt to create a
     // sensorless session.
-    if (CanCreateSensorlessInlineSession(query)) {
+    if (query->GetSensorRequirement() != SensorRequirement::kRequired) {
       XRSession* session = CreateSensorlessInlineSession();
       query->Resolve(session);
       return;
@@ -1051,23 +1086,6 @@ XRSession* XR::CreateSession(
   return session;
 }
 
-bool XR::CanCreateSensorlessInlineSession(
-    const PendingRequestSessionQuery* query) const {
-  // Sensorless can only support an inline mode
-  if (query->mode() != XRSession::kModeInline)
-    return false;
-
-  // Sensorless can only be supported if the only required feature is the
-  // viewer reference space.
-  for (const auto& feature : query->RequiredFeatures()) {
-    if (feature != device::mojom::XRSessionFeature::REF_SPACE_VIEWER) {
-      return false;
-    }
-  }
-
-  return true;
-}
-
 XRSession* XR::CreateSensorlessInlineSession() {
   // TODO(https://crbug.com/944936): The blend mode could be "additive".
   XRSession::EnvironmentBlendMode blend_mode = XRSession::kBlendModeOpaque;

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

@@ -94,6 +94,12 @@ class XR final : public EventTargetWithInlineData,
   base::TimeTicks NavigationStart() const { return navigation_start_; }
 
  private:
+  enum SensorRequirement {
+    kNone,
+    kOptional,
+    kRequired,
+  };
+
   // These values are persisted to logs. Entries should not be renumbered and
   // numeric values should never be reused.
   enum class SessionRequestStatus : int {
@@ -155,6 +161,10 @@ class XR final : public EventTargetWithInlineData,
     bool InvalidRequiredFeatures() const;
     bool InvalidOptionalFeatures() const;
 
+    SensorRequirement GetSensorRequirement() const {
+      return sensor_requirement_;
+    }
+
     // Returns underlying resolver's script state.
     ScriptState* GetScriptState() const;
 
@@ -162,11 +172,13 @@ class XR final : public EventTargetWithInlineData,
 
    private:
     void ReportRequestSessionResult(SessionRequestStatus status);
+    void ParseSensorRequirement();
 
     Member<ScriptPromiseResolver> resolver_;
     const XRSession::SessionMode mode_;
     RequestedXRSessionFeatureSet required_features_;
     RequestedXRSessionFeatureSet optional_features_;
+    SensorRequirement sensor_requirement_ = SensorRequirement::kNone;
 
     const int64_t ukm_source_id_;
 
@@ -273,8 +285,6 @@ class XR final : public EventTargetWithInlineData,
       XRSessionFeatureSet enabled_features,
       bool sensorless_session = false);
 
-  bool CanCreateSensorlessInlineSession(
-      const PendingRequestSessionQuery* query) const;
   XRSession* CreateSensorlessInlineSession();
 
   void Dispose();

third_party/blink/web_tests/wpt_internal/webxr/xrSession_dataProviderDisconnect_inline.https.html

@@ -26,7 +26,10 @@ let testFunction = function(session, fakeDeviceController, t) {
   });
 }
 
+// Inline that solely relies on the implicit viewer reference space ignores the
+// data provider, so we need to make sure that we get a session that supports
+// the local reference space.
 xr_session_promise_test(
-  testName, testFunction, fakeDeviceInitParams, 'inline');
+  testName, testFunction, fakeDeviceInitParams, 'inline', { requiredFeatures: ["local"] });
 
 </script>