Auto merge of #25572 - pshaughn:atomnames, r=jdm

Make name content attributes consistently atoms and put them in rare_data for fast access

<!-- Please describe your changes on the following line: -->
All codepaths setting the name content attribute now use an atom, which is also stored in rare_data for direct lookup by a get_name method.

Paralleling the get_name method, I added a get_id method, which makes some internal id-lookup cases nicer.

A new test tests for a name setter on every HTML element type. In addition to its overt and upstreamable purpose of checking IDL property reflection semantics, for us this test also hits some Servo assertions that make sure the name is an atom in every case. If the test doesn't crash, even a failed test case still has the attribute as an atom rather than some other type. The failed cases are for elements that we have unimplemented or completely stubbed; I added a few missing name IDL properties to otherwise implemented elements.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #25570 and make progress on #25057

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/atoms/static_atoms.txt

@@ -84,6 +84,7 @@ radio
 range
 ratechange
 readystatechange
+referrer
 reftest-wait
 rejectionhandled
 removetrack

components/script/dom/document.rs

@@ -823,6 +823,7 @@ impl Document {
     }
 
     fn get_anchor_by_name(&self, name: &str) -> Option<DomRoot<Element>> {
+        // TODO faster name lookups (see #25548)
         let check_anchor = |node: &HTMLAnchorElement| {
             let elem = node.upcast::<Element>();
             elem.get_attribute(&ns!(), &local_name!("name"))
@@ -4091,9 +4092,7 @@ impl DocumentMethods for Document {
             if element.namespace() != &ns!(html) {
                 return false;
             }
-            element
-                .get_attribute(&ns!(), &local_name!("name"))
-                .map_or(false, |attr| &**attr.value() == &*name)
+            element.get_name().map_or(false, |atom| *atom == *name)
         })
     }
 
@@ -4303,6 +4302,7 @@ impl DocumentMethods for Document {
         }
         // https://html.spec.whatwg.org/multipage/#dom-document-nameditem-filter
         fn filter_by_name(name: &Atom, node: &Node) -> bool {
+            // TODO faster name lookups (see #25548)
             let html_elem_type = match node.type_id() {
                 NodeTypeId::Element(ElementTypeId::HTMLElement(type_)) => type_,
                 _ => return false,

components/script/dom/element.rs

@@ -1420,11 +1420,28 @@ impl Element {
     // https://dom.spec.whatwg.org/#concept-element-attributes-get-by-name
     pub fn get_attribute_by_name(&self, name: DOMString) -> Option<DomRoot<Attr>> {
         let name = &self.parsed_name(name);
-        self.attrs
+        let maybe_attribute = self
+            .attrs
             .borrow()
             .iter()
             .find(|a| a.name() == name)
-            .map(|js| DomRoot::from_ref(&**js))
+            .map(|js| DomRoot::from_ref(&**js));
+
+        fn id_and_name_must_be_atoms(name: &LocalName, maybe_attr: &Option<DomRoot<Attr>>) -> bool {
+            if *name == local_name!("id") || *name == local_name!("name") {
+                match maybe_attr {
+                    None => true,
+                    Some(ref attr) => match *attr.value() {
+                        AttrValue::Atom(_) => true,
+                        _ => false,
+                    },
+                }
+            } else {
+                true
+            }
+        }
+        debug_assert!(id_and_name_must_be_atoms(name, &maybe_attribute));
+        maybe_attribute
     }
 
     pub fn set_attribute_from_parser(
@@ -1819,6 +1836,14 @@ impl Element {
         let other = other.upcast::<Element>();
         self.root_element() == other.root_element()
     }
+
+    pub fn get_id(&self) -> Option<Atom> {
+        self.id_attribute.borrow().clone()
+    }
+
+    pub fn get_name(&self) -> Option<Atom> {
+        self.rare_data().as_ref()?.name_attribute.clone()
+    }
 }
 
 impl ElementMethods for Element {
@@ -1855,6 +1880,8 @@ impl ElementMethods for Element {
     }
 
     // https://dom.spec.whatwg.org/#dom-element-id
+    // This always returns a string; if you'd rather see None
+    // on a null id, call get_id
     fn Id(&self) -> DOMString {
         self.get_string_attribute(&local_name!("id"))
     }
@@ -2802,6 +2829,20 @@ impl VirtualMethods for Element {
                     }
                 }
             },
+            &local_name!("name") => {
+                // Keep the name in rare data for fast access
+                self.ensure_rare_data().name_attribute =
+                    mutation.new_value(attr).and_then(|value| {
+                        let value = value.as_atom();
+                        if value != &atom!("") {
+                            Some(value.clone())
+                        } else {
+                            None
+                        }
+                    });
+                // TODO: notify the document about the name change
+                // once it has a name_map (#25548)
+            },
             _ => {
                 // FIXME(emilio): This is pretty dubious, and should be done in
                 // the relevant super-classes.
@@ -2820,6 +2861,7 @@ impl VirtualMethods for Element {
     fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
         match name {
             &local_name!("id") => AttrValue::from_atomic(value.into()),
+            &local_name!("name") => AttrValue::from_atomic(value.into()),
             &local_name!("class") => AttrValue::from_serialized_tokenlist(value.into()),
             _ => self
                 .super_type()

components/script/dom/htmlanchorelement.rs

@@ -152,7 +152,7 @@ impl HTMLAnchorElementMethods for HTMLAnchorElement {
     make_getter!(Name, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-a-name
-    make_setter!(SetName, "name");
+    make_atomic_setter!(SetName, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-a-rev
     make_getter!(Rev, "rev");

components/script/dom/htmlbuttonelement.rs

@@ -142,7 +142,7 @@ impl HTMLButtonElementMethods for HTMLButtonElement {
     make_getter!(Name, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-fe-name
-    make_setter!(SetName, "name");
+    make_atomic_setter!(SetName, "name");
 
     // https://html.spec.whatwg.org/multipage/#dom-button-value
     make_getter!(Value, "value");

components/script/dom/htmlcollection.rs

@@ -366,11 +366,12 @@ impl HTMLCollectionMethods for HTMLCollection {
             return None;
         }
 
+        let key = Atom::from(key);
+
         // Step 2.
         self.elements_iter().find(|elem| {
-            elem.get_string_attribute(&local_name!("id")) == key ||
-                (elem.namespace() == &ns!(html) &&
-                    elem.get_string_attribute(&local_name!("name")) == key)
+            elem.get_id().map_or(false, |id| id == key) ||
+                (elem.namespace() == &ns!(html) && elem.get_name().map_or(false, |id| id == key))
         })
     }
 
@@ -392,17 +393,20 @@ impl HTMLCollectionMethods for HTMLCollection {
         // Step 2
         for elem in self.elements_iter() {
             // Step 2.1
-            let id_attr = elem.get_string_attribute(&local_name!("id"));
-            if !id_attr.is_empty() && !result.contains(&id_attr) {
-                result.push(id_attr)
+            if let Some(id_atom) = elem.get_id() {
+                let id_str = DOMString::from(&*id_atom);
+                if !result.contains(&id_str) {
+                    result.push(id_str);
+                }
             }
             // Step 2.2
-            let name_attr = elem.get_string_attribute(&local_name!("name"));
-            if !name_attr.is_empty() &&
-                !result.contains(&name_attr) &&
-                *elem.namespace() == ns!(html)
-            {
-                result.push(name_attr)
+            if *elem.namespace() == ns!(html) {
+                if let Some(name_atom) = elem.get_name() {
+                    let name_str = DOMString::from(&*name_atom);
+                    if !result.contains(&name_str) {
+                        result.push(name_str)
+                    }
+                }
             }
         }
 

components/script/dom/htmlfieldsetelement.rs

@@ -7,6 +7,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding;
 use crate::dom::bindings::codegen::Bindings::HTMLFieldSetElementBinding::HTMLFieldSetElementMethods;
 use crate::dom::bindings::inheritance::{Castable, ElementTypeId, HTMLElementTypeId, NodeTypeId};
 use crate::dom::bindings::root::{DomRoot, MutNullableDom};
+use crate::dom::bindings::str::DOMString;
 use crate::dom::document::Document;
 use crate::dom::element::{AttributeMutation, Element};
 use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
@@ -88,6 +89,12 @@ impl HTMLFieldSetElementMethods for HTMLFieldSetElement {
     // https://html.spec.whatwg.org/multipage/#dom-fieldset-disabled
     make_bool_setter!(SetDisabled, "disabled");
 
+    // https://html.spec.whatwg.org/multipage/#dom-fe-name
+    make_atomic_setter!(SetName, "name");
+
+    // https://html.spec.whatwg.org/multipage/#dom-fe-name
+    make_getter!(Name, "name");
+
     // https://html.spec.whatwg.org/multipage/#dom-fae-form
     fn GetForm(&self) -> Option<DomRoot<HTMLFormElement>> {
         self.form_owner()

components/script/dom/htmlformcontrolscollection.rs

@@ -18,6 +18,7 @@ use crate::dom::node::Node;
 use crate::dom::radionodelist::RadioNodeList;
 use crate::dom::window::Window;
 use dom_struct::dom_struct;
+use servo_atoms::Atom;
 
 #[dom_struct]
 pub struct HTMLFormControlsCollection {
@@ -67,9 +68,11 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
             return None;
         }
 
+        let name = Atom::from(name);
+
         let mut filter_map = self.collection.elements_iter().filter_map(|elem| {
-            if elem.get_string_attribute(&local_name!("name")) == name ||
-                elem.get_string_attribute(&local_name!("id")) == name
+            if elem.get_name().map_or(false, |n| n == name) ||
+                elem.get_id().map_or(false, |i| i == name)
             {
                 Some(elem)
             } else {
@@ -90,7 +93,7 @@ impl HTMLFormControlsCollectionMethods for HTMLFormControlsCollection {
                 // specifically HTMLFormElement::Elements(),
                 // and the collection filter excludes image inputs.
                 Some(RadioNodeListOrElement::RadioNodeList(
-                    RadioNodeList::new_controls_except_image_inputs(window, &*self.form, name),
+                    RadioNodeList::new_controls_except_image_inputs(window, &*self.form, &name),
                 ))
             }
         // Step 3