Repository: servo/servo (language: python)

Issue:
XRInputSourcesChangeEvent.added/.removed should store frozen array values
Spec: https://immersive-web.github.io/webxr/#xrinputsourceschangeevent-interface
Implementation: https://github.com/servo/servo/blob/main/components/script/dom/xrinputsourceschangeevent.rs#L90-L97

Instead of converting a Rust array to an equivalent JS value, we should use the [to_frozen_array](https://doc.servo.org/script/dom/bindings/utils/fn.to_frozen_array.html) helper.

I hope we will see some new tests passing in `./mach test-wpt tests/wpt/tests/webxr/events_input_sources_change.https.html`.


Discussion / hints:
Hmm, this will good for correctness but I think the only failing subtests remaining there have to do with the actual events being fired off, which I don't think will change from this.

Sidenote, whoever picks this up, make sure to update the rustdoc comment for `to_frozen_array` as well. The "array of ports" it refers to doesn't make much sense anymore as this helper is used in multiple files now.
Can I be assigned this?
when i use `to_frozen_array`, i get this clippy warning:

```
error[E0423]: cannot initialize a tuple struct which contains private fields
   --> components\script\dom\xrinputsourceschangeevent.rs:91:58
    |
91  | ...n_array(added, JSContext(*cx)));
    |                   ^^^^^^^^^
    |
note: constructor is not visible here due to private fields
   --> components\script\script_runtime.rs:882:22
    |
882 | pub struct JSContext(*mut RawJSContext);
    |                      ^^^^^^^^^^^^^^^^^ private field
help: consider making the field publicly accessible
   --> components\script\script_runtime.rs:882:22
    |
882 | pub struct JSContext(pub *mut RawJSContext);
    |                      +++
```

Is it advisable to follow its recommendation? or change the function signature to add `cx: JSContext`
Use the public constructor instead: https://doc.servo.org/script/script_runtime/struct.JSContext.html#method.from_ptr
@jdm i am not seeing any changes in the status of the tests :(
> Sidenote, whoever picks this up, make sure to update the rustdoc comment for `to_frozen_array` as well. The "array of ports" it refers to doesn't make much sense anymore as this helper is used in multiple files now.

should i change it to -
Returns a `JSVal` representing the frozen JavaScript array

Fix patch:
diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs
index c3f6bf6697162..c9cbbae2723de 100644
--- a/components/script/dom/bindings/utils.rs
+++ b/components/script/dom/bindings/utils.rs
@@ -116,7 +116,7 @@ impl Clone for DOMJSClass {
 }
 unsafe impl Sync for DOMJSClass {}
 
-/// Returns a JSVal representing a frozen array of ports
+/// Returns a JSVal representing the frozen JavaScript array
 pub fn to_frozen_array<T: ToJSValConvertible>(convertibles: &[T], cx: SafeJSContext) -> JSVal {
     rooted!(in(*cx) let mut ports = UndefinedValue());
     unsafe { convertibles.to_jsval(*cx, ports.handle_mut()) };
diff --git a/components/script/dom/xrinputsourceschangeevent.rs b/components/script/dom/xrinputsourceschangeevent.rs
index b44bc17382a0d..dc28cefa812de 100644
--- a/components/script/dom/xrinputsourceschangeevent.rs
+++ b/components/script/dom/xrinputsourceschangeevent.rs
@@ -3,9 +3,8 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use dom_struct::dom_struct;
-use js::conversions::ToJSValConvertible;
 use js::jsapi::Heap;
-use js::jsval::{JSVal, UndefinedValue};
+use js::jsval::JSVal;
 use js::rust::HandleObject;
 use servo_atoms::Atom;
 
@@ -17,6 +16,7 @@ use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::reflector::{reflect_dom_object_with_proto, DomObject};
 use crate::dom::bindings::root::{Dom, DomRoot};
 use crate::dom::bindings::str::DOMString;
+use crate::dom::bindings::utils::to_frozen_array;
 use crate::dom::event::Event;
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::window::Window;
@@ -87,15 +87,8 @@ impl XRInputSourcesChangeEvent {
         }
         let _ac = enter_realm(global);
         let cx = GlobalScope::get_cx();
-        unsafe {
-            rooted!(in(*cx) let mut added_val = UndefinedValue());
-            added.to_jsval(*cx, added_val.handle_mut());
-            changeevent.added.set(added_val.get());
-            rooted!(in(*cx) let mut removed_val = UndefinedValue());
-            removed.to_jsval(*cx, removed_val.handle_mut());
-            changeevent.removed.set(removed_val.get());
-        }
-
+        changeevent.added.set(to_frozen_array(added, cx));
+        changeevent.removed.set(to_frozen_array(removed, cx));
         changeevent
     }
 }


Test patch:
diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json
index 47b49c3fd01f1..098e27f4e4d94 100644
--- a/tests/wpt/meta/MANIFEST.json
+++ b/tests/wpt/meta/MANIFEST.json
@@ -844130,7 +844130,7 @@
      ]
     ],
     "events_input_sources_change.https.html": [
-     "b2a17d4e17ae03bf7ed51e07095aa36dcce033a3",
+     "db8a8233fe55a920a077a644d711a455daf14b6b",
      [
       null,
       {}
diff --git a/tests/wpt/tests/webxr/events_input_sources_change.https.html b/tests/wpt/tests/webxr/events_input_sources_change.https.html
index b2a17d4e17ae0..db8a8233fe55a 100644
--- a/tests/wpt/tests/webxr/events_input_sources_change.https.html
+++ b/tests/wpt/tests/webxr/events_input_sources_change.https.html
@@ -5,6 +5,7 @@
 <script src="resources/webxr_test_constants.js"></script>
 
 <script>
+"use strict"
 let testName = "Transient input sources fire events in the right order";
 
 let watcherDone = new Event("watcherdone");
@@ -25,6 +26,17 @@
       assert_equals(event.session, session);
       validateSameObject(event);
 
+      assert_throws_js(
+          TypeError,
+          () => { event.added[0] = "test"; },
+          "added array must be frozen"
+      );
+      assert_throws_js(
+          TypeError,
+          () => { event.removed[0] = "test"; },
+          "removed array must be frozen"
+      );
+
       // The first change event should be adding our controller.
       if (inputChangeEvents === 1) {
         validateAdded(event.added, 1);