Recreate XRInputSource when profiles change.

Based on discussion at immersive-web/webxr#783 ,
we are confident this will become a requirement in the WebXR spec.

Also update the OpenVR render loop to re-send the input source description
when the profiles array is updated.

Bug: 989244
Change-Id: I77bb2b15d894fe41c532235b062163bdc87a4ea2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1744663
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Commit-Queue: Jacob DeWitt <jacde@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686103}

chrome/browser/vr/webxr_vr_input_browser_test.cc

@@ -352,6 +352,47 @@ IN_PROC_MULTI_CLASS_BROWSER_TEST_F2(WebXrVrOpenVrBrowserTest,
   t->EndTest();
 }
 
+// Ensure that when an input source's profiles array changes, an input source
+// change event is fired and a new input source is created.
+// OpenVR-only since WMR only supports one kind of gamepad, so it's not possible
+// to update the connected gamepad functionality to force the profiles array to
+// change.
+IN_PROC_BROWSER_TEST_F(WebXrVrOpenVrBrowserTest, TestInputProfilesChange) {
+  WebXrControllerInputMock my_mock;
+  unsigned int controller_index = my_mock.CreateAndConnectMinimalGamepad();
+
+  LoadUrlAndAwaitInitialization(
+      GetFileUrlForHtmlTestFile("test_webxr_input_same_object"));
+  EnterSessionWithUserGestureOrFail();
+
+  // Wait for the first changed event
+  PollJavaScriptBooleanOrFail("inputChangeEvents === 1",
+                              WebXrVrBrowserTestBase::kPollTimeoutShort);
+
+  // We only expect one input source, cache it.
+  RunJavaScriptOrFail("validateInputSourceLength(1)");
+  RunJavaScriptOrFail("updateCachedInputSource(0)");
+
+  // Add a touchpad so that the profiles array changes and verify that we get a
+  // change event.
+  uint64_t supported_buttons =
+      device::XrButtonMaskFromId(device::XrButtonId::kAxisTrigger) |
+      device::XrButtonMaskFromId(device::XrButtonId::kAxisTrackpad);
+  std::map<device::XrButtonId, unsigned int> axis_types = {
+      {device::XrButtonId::kAxisTrackpad, device::XrAxisType::kTrackpad},
+      {device::XrButtonId::kAxisTrigger, device::XrAxisType::kTrigger},
+  };
+  my_mock.UpdateControllerSupport(controller_index, axis_types,
+                                  supported_buttons);
+
+  PollJavaScriptBooleanOrFail("inputChangeEvents === 2",
+                              WebXrVrBrowserTestBase::kPollTimeoutShort);
+  RunJavaScriptOrFail("validateCachedSourcePresence(false)");
+  RunJavaScriptOrFail("validateInputSourceLength(1)");
+  RunJavaScriptOrFail("done()");
+  EndTest();
+}
+
 // Ensure that changes to a gamepad object respect that it is the same object
 // and that if whether or not an input source has a gamepad changes that the
 // input source change event is fired and a new input source is created.

device/vr/openvr/openvr_render_loop.cc

@@ -290,9 +290,11 @@ std::vector<mojom::XRInputSourceStatePtr> OpenVRRenderLoop::GetInputState(
                                                   controller_state, handedness);
     state->gamepad = input_source_data.gamepad;
 
-    // If this is a newly active controller or if the handedness has changed
-    // since the last update, re-send the controller's description.
-    if (newly_active || controller_role != input_active_state.controller_role) {
+    // Re-send the controller's description if it's newly active or if the
+    // handedness or profile strings have changed.
+    if (newly_active ||
+        (controller_role != input_active_state.controller_role) ||
+        (input_source_data.profiles != input_active_state.profiles)) {
       device::mojom::XRInputSourceDescriptionPtr desc =
           device::mojom::XRInputSourceDescription::New();
 
@@ -313,6 +315,9 @@ std::vector<mojom::XRInputSourceStatePtr> OpenVRRenderLoop::GetInputState(
       desc->profiles = input_source_data.profiles;
 
       state->description = std::move(desc);
+
+      // Keep track of the current profiles so we know if it changes next frame.
+      input_active_state.profiles = input_source_data.profiles;
     }
 
     input_states.push_back(std::move(state));
@@ -321,4 +326,7 @@ std::vector<mojom::XRInputSourceStatePtr> OpenVRRenderLoop::GetInputState(
   return input_states;
 }
 
+OpenVRRenderLoop::InputActiveState::InputActiveState() = default;
+OpenVRRenderLoop::InputActiveState::~InputActiveState() = default;
+
 }  // namespace device

device/vr/openvr/openvr_render_loop.h

@@ -5,6 +5,9 @@
 #ifndef DEVICE_VR_OPENVR_RENDER_LOOP_H
 #define DEVICE_VR_OPENVR_RENDER_LOOP_H
 
+#include <string>
+#include <vector>
+
 #include "base/memory/scoped_refptr.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
@@ -52,7 +55,13 @@ class OpenVRRenderLoop : public XRCompositorCommon {
     vr::ETrackedDeviceClass device_class;
     vr::ETrackedControllerRole controller_role;
 
+    std::vector<std::string> profiles;
+
+    InputActiveState();
+    ~InputActiveState();
     void MarkAsInactive();
+
+    DISALLOW_COPY_AND_ASSIGN(InputActiveState);
   };
 
   InputActiveState input_active_states_[vr::k_unMaxTrackedDeviceCount];

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

@@ -50,6 +50,16 @@ XRInputSource::InternalState::InternalState(const InternalState& other) =
 
 XRInputSource::InternalState::~InternalState() = default;
 
+void XRInputSource::SetProfiles(
+    const device::mojom::blink::XRInputSourceStatePtr& state) {
+  if (state->description) {
+    profiles_.clear();
+    for (const auto& name : state->description->profiles) {
+      profiles_.push_back(name);
+    }
+  }
+}
+
 XRInputSource* XRInputSource::CreateOrUpdateFrom(
     XRInputSource* other,
     XRSession* session,
@@ -63,11 +73,14 @@ XRInputSource* XRInputSource::CreateOrUpdateFrom(
   if (!other) {
     auto source_id = state->source_id;
     updated_source = MakeGarbageCollected<XRInputSource>(session, source_id);
+    updated_source->SetProfiles(state);
   } else if (other->InvalidatesSameObject(state)) {
     // Something in the state has changed which requires us to re-create the
     // object.  Create a copy now, and we will blindly update any state later,
-    // knowing that we now have a new object if needed.
+    // knowing that we now have a new object if needed.  The one exception is
+    // the profiles array, which we make sure to only copy when necessary.
     updated_source = MakeGarbageCollected<XRInputSource>(*other);
+    updated_source->SetProfiles(state);
   }
 
   updated_source->UpdateGamepad(state->gamepad);
@@ -83,13 +96,6 @@ XRInputSource* XRInputSource::CreateOrUpdateFrom(
 
     updated_source->pointer_transform_matrix_ =
         TryGetTransformationMatrix(desc->pointer_offset);
-
-    // Update the profiles list in-place.
-    // TODO(crbug.com/989244): Re-create the XRInputSource object if necessary.
-    updated_source->profiles_.clear();
-    for (const auto& name : desc->profiles) {
-      updated_source->profiles_.push_back(name);
-    }
   }
 
   updated_source->base_pose_matrix_ = TryGetTransformationMatrix(state->grip);
@@ -173,6 +179,16 @@ bool XRInputSource::InvalidatesSameObject(
     if (state->description->target_ray_mode != state_.target_ray_mode) {
       return true;
     }
+
+    if (state->description->profiles.size() != profiles_.size()) {
+      return true;
+    }
+
+    for (wtf_size_t i = 0; i < profiles_.size(); ++i) {
+      if (state->description->profiles[i] != profiles_[i]) {
+        return true;
+      }
+    }
   }
 
   return false;

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

@@ -120,6 +120,8 @@ class XRInputSource : public ScriptWrappable, public Gamepad::Client {
   // from InvalidatesSameObject
   void UpdateGamepad(const base::Optional<device::Gamepad>& gamepad);
 
+  void SetProfiles(const device::mojom::blink::XRInputSourceStatePtr& state);
+
   XRInputSourceEvent* CreateInputSourceEvent(const AtomicString& type);
 
   // These member variables all require special behavior when being copied or

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

@@ -31,28 +31,48 @@
         assert_not_equals(cached_input_source, null);
         assert_equals(cached_input_source.handedness, "none");
         assert_equals(cached_input_source.targetRayMode, "gaze");
+        assertProfilesEqual(cached_input_source.profiles, ["a", "b"]);
       } else if (inputChangeEvents == 2) {
         // The second event should be replacing the controller with one that has
         // the updated target ray mode.
-        validateInputSourceChange(event, "none", "tracked-pointer");
+        validateInputSourceChange(event, "none", "tracked-pointer", ["a", "b"]);
         cached_input_source = getInputSources()[0];
       } else if (inputChangeEvents == 3) {
         // The third event should be replacing the controller with one that has
         // the updated handedness.
-        validateInputSourceChange(event, "left", "tracked-pointer");
+        validateInputSourceChange(event, "left", "tracked-pointer", ["a", "b"]);
+        cached_input_source = getInputSources()[0];
+      } else if (inputChangeEvents == 4) {
+        // The fourth event should be updating the second value in the profiles
+        // array.
+        validateInputSourceChange(event, "left", "tracked-pointer", ["a", "c"]);
+        cached_input_source = getInputSources()[0];
+      } else if (inputChangeEvents == 5) {
+        // The fifth event should be removing the second value from the profiles
+        // array.
+        validateInputSourceChange(event, "left", "tracked-pointer", ["a"]);
         session.dispatchEvent(watcherDone);
       }
     });
   }
 
-  function validateInputSourceChange(event, expected_hand, expected_mode) {
+  function assertProfilesEqual(profiles, expected_profiles) {
+    assert_equals(profiles.length, expected_profiles.length);
+    for (let i = 0; i < profiles.length; ++i) {
+      assert_equals(profiles[i], expected_profiles[i]);
+    }
+  }
+
+  function validateInputSourceChange(
+      event, expected_hand, expected_mode, expected_profiles) {
     validateAdded(event.added, 1);
     validateRemoved(event.removed, 1);
     assert_true(event.removed.includes(cached_input_source));
     assert_false(event.added.includes(cached_input_source));
     let source = event.added[0];
     assert_equals(source.handedness, expected_hand);
     assert_equals(source.targetRayMode, expected_mode);
+    assertProfilesEqual(source.profiles, expected_profiles);
   }
 
   function validateAdded(added, length) {
@@ -95,17 +115,24 @@
     handedness: "none",
     targetRayMode: "gaze",
     pointerOrigin: VALID_POINTER_TRANSFORM,
-    profiles: []
+    profiles: ["a", "b"]
   });
 
-  // Make our input source change after one frame, and wait an additional
-  // frame for that change to propogate.
-  // Two changes made in total.
+  // Make our first input source change after one frame, and wait an additional
+  // frame for that change to propogate. Then make additional changes, waiting
+  // one frame after each one to verify that they fired an inputsourceschanged
+  // event.
   session.requestAnimationFrame((time, xrFrame) => {
     input_source.setTargetRayMode("tracked-pointer");
     session.requestAnimationFrame((time, xrFrame) => {
       input_source.setHandedness("left");
-      session.requestAnimationFrame((time, xrFrame) => {});
+      session.requestAnimationFrame((time, xrFrame) => {
+        input_source.setProfiles(["a", "c"]);
+        session.requestAnimationFrame((time, xrFrame) => {
+          input_source.setProfiles(["a"]);
+          session.requestAnimationFrame((time, xrFrame) => {});
+        });
+      });
     });
   });