Make XRSession.inputSources comply with SameObject requirement

Per the WebXR Spec, the inputSources attribute is supposed to be a live
array whose input sources are updated in place.  Currently the code
creates a new array on every query.  This changes the internal hash map
of input sources to be an XRInputSourceArray object.

The XRInputSourceArray object is implemented with a backing HeapHashMap
so that the existing logic (i.e. looking up/setting/erasing by id) is
more performant than getting by the index.  This does have a side effect
of meaning that an input source's position in the inputSources array can
change, as it's ordering is based on the ordering of the HeapHashMap's
backing .Values store.

Bug: 966552
Change-Id: I21396ebbbbef6799a6bdd98791706c9159c9e061
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1632872
Commit-Queue: Alexander Cooper <alcooper@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664463}

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

@@ -12,11 +12,35 @@ XRInputSource* XRInputSourceArray::AnonymousIndexedGetter(
   if (index >= input_sources_.size())
     return nullptr;
 
-  return input_sources_[index];
+  auto it = input_sources_.Values().begin();
+
+  // HeapHashMap's iterators don't expose a generic + operator.  We're ensuring
+  // that this won't be past the end with the size check above.
+  for (unsigned i = 0; i < index; i++) {
+    ++it;
+  }
+
+  return *(it.Get());
+}
+
+XRInputSource* XRInputSourceArray::operator[](unsigned index) const {
+  DCHECK(index < length());
+  return AnonymousIndexedGetter(index);
+}
+
+XRInputSource* XRInputSourceArray::GetWithSourceId(uint32_t source_id) {
+  return input_sources_.at(source_id);
+}
+
+void XRInputSourceArray::RemoveWithSourceId(uint32_t source_id) {
+  auto it = input_sources_.find(source_id);
+  if (it != input_sources_.end())
+    input_sources_.erase(it);
 }
 
-void XRInputSourceArray::Add(XRInputSource* input_source) {
-  input_sources_.push_back(input_source);
+void XRInputSourceArray::SetWithSourceId(uint32_t source_id,
+                                         XRInputSource* input_source) {
+  input_sources_.Set(source_id, input_source);
 }
 
 void XRInputSourceArray::Trace(blink::Visitor* visitor) {

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

@@ -17,12 +17,16 @@ class XRInputSourceArray : public ScriptWrappable {
   unsigned length() const { return input_sources_.size(); }
   XRInputSource* AnonymousIndexedGetter(unsigned index) const;
 
-  void Add(XRInputSource* input_source);
+  XRInputSource* operator[](unsigned index) const;
+
+  XRInputSource* GetWithSourceId(uint32_t source_id);
+  void RemoveWithSourceId(uint32_t source_id);
+  void SetWithSourceId(uint32_t source_id, XRInputSource* input_source);
 
   void Trace(blink::Visitor*) override;
 
  private:
-  HeapVector<Member<XRInputSource>> input_sources_;
+  HeapHashMap<uint32_t, Member<XRInputSource>> input_sources_;
 };
 
 }  // namespace blink

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

@@ -123,6 +123,7 @@ XRSession::XRSession(
       environment_integration_(mode == kModeImmersiveAR),
       world_tracking_state_(MakeGarbageCollected<XRWorldTrackingState>()),
       world_information_(MakeGarbageCollected<XRWorldInformation>(this)),
+      input_sources_(MakeGarbageCollected<XRInputSourceArray>()),
       client_binding_(this, std::move(client_request)),
       callback_collection_(
           MakeGarbageCollected<XRFrameRequestCallbackCollection>(
@@ -343,19 +344,7 @@ XRInputSourceArray* XRSession::inputSources() const {
     did_log_getInputSources_ = true;
   }
 
-  XRInputSourceArray* source_array = MakeGarbageCollected<XRInputSourceArray>();
-  for (const auto& input_source : input_sources_.Values()) {
-    source_array->Add(input_source);
-  }
-
-  if (canvas_input_provider_) {
-    XRInputSource* input_source = canvas_input_provider_->GetInputSource();
-    if (input_source) {
-      source_array->Add(input_source);
-    }
-  }
-
-  return source_array;
+  return input_sources_;
 }
 
 ScriptPromise XRSession::requestHitTest(ScriptState* script_state,
@@ -478,19 +467,27 @@ ScriptPromise XRSession::end(ScriptState* script_state) {
 }
 
 void XRSession::ForceEnd() {
+  // If we've already ended, then just abort.  Since this is called only by C++
+  // code, and predominantly just to ensure that the session is shut down, this
+  // is fine.
+  if (ended_)
+    return;
+
   // Detach this session from the XR system.
   ended_ = true;
   pending_frame_ = false;
 
+  for (unsigned i = 0; i < input_sources_->length(); i++) {
+    (*input_sources_)[i]->SetGamepadConnected(false);
+  }
+
+  input_sources_ = nullptr;
+
   if (canvas_input_provider_) {
     canvas_input_provider_->Stop();
     canvas_input_provider_ = nullptr;
   }
 
-  for (auto& input_source : input_sources_.Values()) {
-    input_source->SetGamepadConnected(false);
-  }
-
   // If this session is the active immersive session, notify the frameProvider
   // that it's ended.
   if (xr_->frameProvider()->immersive_session() == this) {
@@ -766,13 +763,13 @@ void XRSession::OnInputStateChange(
   // sources so we can flag the ones that are no longer active.
   for (const auto& input_state : input_states) {
     XRInputSource* stored_input_source =
-        input_sources_.at(input_state->source_id);
+        input_sources_->GetWithSourceId(input_state->source_id);
     XRInputSource* input_source = XRInputSource::CreateOrUpdateFrom(
         stored_input_source, this, input_state);
 
     // Using pointer equality to determine if the pointer needs to be set.
     if (stored_input_source != input_source) {
-      input_sources_.Set(input_state->source_id, input_source);
+      input_sources_->SetWithSourceId(input_state->source_id, input_source);
       added.push_back(input_source);
 
       // If we previously had a stored_input_source, disconnect it's gamepad
@@ -793,7 +790,8 @@ void XRSession::OnInputStateChange(
   // processing removed, because if we replaced any input sources, they would
   // also be in removed, and we'd remove our newly added source.
   std::vector<uint32_t> inactive_sources;
-  for (const auto& input_source : input_sources_.Values()) {
+  for (unsigned i = 0; i < input_sources_->length(); i++) {
+    auto* input_source = (*input_sources_)[i];
     if (input_source->activeFrameId() != frame_id) {
       inactive_sources.push_back(input_source->source_id());
       input_source->SetGamepadConnected(false);
@@ -802,7 +800,7 @@ void XRSession::OnInputStateChange(
   }
 
   for (uint32_t source_id : inactive_sources) {
-    input_sources_.erase(source_id);
+    input_sources_->RemoveWithSourceId(source_id);
   }
 
   // If there have been any changes, fire the input sources change event.
@@ -819,7 +817,8 @@ void XRSession::OnInputStateChange(
     if (ended_)
       break;
 
-    XRInputSource* input_source = input_sources_.at(input_state->source_id);
+    XRInputSource* input_source =
+        input_sources_->GetWithSourceId(input_state->source_id);
     DCHECK(input_source);
     UpdateSelectState(input_source, input_state);
   }

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

@@ -89,8 +89,6 @@ class XRSession final : public EventTargetWithInlineData,
   int requestAnimationFrame(V8XRFrameRequestCallback*);
   void cancelAnimationFrame(int id);
 
-  using InputSourceMap = HeapHashMap<uint32_t, Member<XRInputSource>>;
-
   XRInputSourceArray* inputSources() const;
 
   ScriptPromise requestHitTest(ScriptState* script_state,
@@ -222,7 +220,7 @@ class XRSession final : public EventTargetWithInlineData,
 
   WTF::Vector<XRViewData> views_;
 
-  InputSourceMap input_sources_;
+  Member<XRInputSourceArray> input_sources_;
   Member<ResizeObserver> resize_observer_;
   Member<XRCanvasInputProvider> canvas_input_provider_;
   bool environment_error_handler_subscribed_ = false;

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

@@ -24,7 +24,7 @@ enum XREnvironmentBlendMode {
 ] interface XRSession : EventTarget {
   readonly attribute XREnvironmentBlendMode environmentBlendMode;
   [SameObject] readonly attribute XRRenderState renderState;
-  [MeasureAs=XRSessionGetInputSources] readonly attribute XRInputSourceArray inputSources;
+  [MeasureAs=XRSessionGetInputSources, SameObject] readonly attribute XRInputSourceArray inputSources;
 
   attribute EventHandler onblur;
   attribute EventHandler onfocus;