script: Allow reusing results from xpath queries (#39392)

This behaviour is optional, but observable. Other browsers implement it,
so we should do it too.

Testing: There are no WPT tests for this, which is fair since the spec
explicitly states implementors may choose to not reuse the result.
Fixes: Part of https://github.com/servo/servo/issues/34527

---------

Signed-off-by: Simon Wülker <simon.wuelker@arcor.de>

components/script/dom/xpathexpression.rs

@@ -53,7 +53,7 @@ impl XPathExpression {
         &self,
         context_node: &Node,
         result_type_num: u16,
-        _result: Option<&XPathResult>,
+        result: Option<&XPathResult>,
         resolver: Option<Rc<XPathNSResolver>>,
         can_gc: CanGc,
     ) -> Fallible<DomRoot<XPathResult>> {
@@ -63,16 +63,23 @@ impl XPathExpression {
         let global = self.global();
         let window = global.as_window();
 
-        let result_value = evaluate_parsed_xpath(&self.parsed_expression, context_node, resolver)?;
+        let result_value =
+            evaluate_parsed_xpath(&self.parsed_expression, context_node, resolver)?.into();
 
-        // TODO(vlindhol): support putting results into mutable `_result` as per the spec
+        if let Some(result) = result {
-        Ok(XPathResult::new(
+            // According to https://www.w3.org/TR/DOM-Level-3-XPath/xpath.html#XPathEvaluator-evaluate, reusing
-            window,
+            // the provided result object is optional. We choose to do it here because thats what other browsers do.
-            None,
+            result.reinitialize_with(result_type, result_value);
-            can_gc,
+            Ok(DomRoot::from_ref(result))
-            result_type,
+        } else {
-            result_value.into(),
+            Ok(XPathResult::new(
-        ))
+                window,
+                None,
+                can_gc,
+                result_type,
+                result_value,
+            ))
+        }
     }
 }
 

components/script/dom/xpathresult.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
-use std::cell::Cell;
+use std::cell::{Cell, RefCell};
 
 use dom_struct::dom_struct;
 use js::rust::HandleObject;
@@ -84,8 +84,8 @@ impl From<Value> for XPathResultValue {
 pub(crate) struct XPathResult {
     reflector_: Reflector,
     window: Dom<Window>,
-    result_type: XPathResultType,
+    result_type: Cell<XPathResultType>,
-    value: XPathResultValue,
+    value: RefCell<XPathResultValue>,
     iterator_invalid: Cell<bool>,
     iterator_pos: Cell<usize>,
 }
@@ -112,10 +112,10 @@ impl XPathResult {
         XPathResult {
             reflector_: Reflector::new(),
             window: Dom::from_ref(window),
-            result_type: inferred_result_type,
+            result_type: Cell::new(inferred_result_type),
             iterator_invalid: Cell::new(false),
             iterator_pos: Cell::new(0),
-            value,
+            value: RefCell::new(value),
         }
     }
 
@@ -136,17 +136,24 @@ impl XPathResult {
             can_gc,
         )
     }
+
+    pub(crate) fn reinitialize_with(&self, result_type: XPathResultType, value: XPathResultValue) {
+        self.result_type.set(result_type);
+        *self.value.borrow_mut() = value;
+        self.iterator_invalid.set(false);
+        self.iterator_pos.set(0);
+    }
 }
 
 impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-resulttype>
     fn ResultType(&self) -> u16 {
-        self.result_type as u16
+        self.result_type.get() as u16
     }
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-numbervalue>
     fn GetNumberValue(&self) -> Fallible<f64> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (XPathResultValue::Number(n), XPathResultType::Number) => Ok(*n),
             _ => Err(Error::Type(
                 "Can't get number value for non-number XPathResult".to_string(),
@@ -156,7 +163,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-stringvalue>
     fn GetStringValue(&self) -> Fallible<DOMString> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (XPathResultValue::String(s), XPathResultType::String) => Ok(s.clone()),
             _ => Err(Error::Type(
                 "Can't get string value for non-string XPathResult".to_string(),
@@ -166,7 +173,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-booleanvalue>
     fn GetBooleanValue(&self) -> Fallible<bool> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (XPathResultValue::Boolean(b), XPathResultType::Boolean) => Ok(*b),
             _ => Err(Error::Type(
                 "Can't get boolean value for non-boolean XPathResult".to_string(),
@@ -183,7 +190,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
             ));
         }
 
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (
                 XPathResultValue::Nodeset(nodes),
                 XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator,
@@ -208,7 +215,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
         let is_iterator_invalid = self.iterator_invalid.get();
         if is_iterator_invalid ||
             matches!(
-                self.result_type,
+                self.result_type.get(),
                 XPathResultType::OrderedNodeIterator | XPathResultType::UnorderedNodeIterator
             )
         {
@@ -222,7 +229,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-snapshotlength>
     fn GetSnapshotLength(&self) -> Fallible<u32> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (
                 XPathResultValue::Nodeset(nodes),
                 XPathResultType::OrderedNodeSnapshot | XPathResultType::UnorderedNodeSnapshot,
@@ -235,7 +242,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-snapshotitem>
     fn SnapshotItem(&self, index: u32) -> Fallible<Option<DomRoot<Node>>> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (
                 XPathResultValue::Nodeset(nodes),
                 XPathResultType::OrderedNodeSnapshot | XPathResultType::UnorderedNodeSnapshot,
@@ -248,7 +255,7 @@ impl XPathResultMethods<crate::DomTypeHolder> for XPathResult {
 
     /// <https://dom.spec.whatwg.org/#dom-xpathresult-singlenodevalue>
     fn GetSingleNodeValue(&self) -> Fallible<Option<DomRoot<Node>>> {
-        match (&self.value, self.result_type) {
+        match (&*self.value.borrow(), self.result_type.get()) {
             (
                 XPathResultValue::Nodeset(nodes),
                 XPathResultType::AnyUnorderedNode | XPathResultType::FirstOrderedNode,

tests/wpt/mozilla/meta/MANIFEST.json

@@ -14613,6 +14613,13 @@
       {}
      ]
     ],
+    "xpath-result-out-parameter.html": [
+     "e4206aba44d2c4f6d299d9860d80b5f736094789",
+     [
+      null,
+      {}
+     ]
+    ],
     "zero_size_canvas_crash.html": [
      "45eb9b559e8d6105baca5ab4d336de520d33b36b",
      [

tests/wpt/mozilla/tests/mozilla/xpath-result-out-parameter.html

@@ -0,0 +1,40 @@
+<!doctype html>
+<head>
+    <link rel="help" href="https://www.w3.org/TR/DOM-Level-3-XPath/xpath.html#XPathEvaluator-evaluate">
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+</head>
+<script>
+const xmlString = `
+<root>
+  <section>
+    <item id="a"><name>Item A</name></item>
+    <item id="b"><name>Item B</name></item>
+  </section>
+</root>
+`;
+
+const parser = new DOMParser();
+const test_document = parser.parseFromString(xmlString, "application/xml");
+
+test(function () {
+    const first_result = test_document.evaluate(
+      "//section/item",
+      test_document,
+      null,
+      XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,
+      null
+    );
+
+    const second_result = test_document.evaluate(
+      "//section/item[@id='b']",
+      test_document,
+      null,
+      XPathResult.ORDERED_NODE_SNAPSHOT_TYPE,
+      first_result
+    );
+
+    assert_equals(first_result, second_result);
+}, "Passing an existing xpath result as an out-parameter should overwrite the result");
+
+</script>