script: Expose a constructor on HTMLCollection that takes a static function (#34667)

Expose a new constructor on `HTMLCollection`, `new_with_filter_fn`, that
filters elements using a simple static function -- a common pattern.
This allows more easily creating this `HTMLCollection` without having to
create a new data structure. Since the constructor takes a static
function, no data should be captured preventing garbage collection
issues.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/script/dom/document.rs

@@ -497,55 +497,6 @@ pub struct Document {
     status_code: Option<u16>,
 }
 
-#[derive(JSTraceable, MallocSizeOf)]
-struct ImagesFilter;
-impl CollectionFilter for ImagesFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        elem.is::<HTMLImageElement>()
-    }
-}
-
-#[derive(JSTraceable, MallocSizeOf)]
-struct EmbedsFilter;
-impl CollectionFilter for EmbedsFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        elem.is::<HTMLEmbedElement>()
-    }
-}
-
-#[derive(JSTraceable, MallocSizeOf)]
-struct LinksFilter;
-impl CollectionFilter for LinksFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        (elem.is::<HTMLAnchorElement>() || elem.is::<HTMLAreaElement>()) &&
-            elem.has_attribute(&local_name!("href"))
-    }
-}
-
-#[derive(JSTraceable, MallocSizeOf)]
-struct FormsFilter;
-impl CollectionFilter for FormsFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        elem.is::<HTMLFormElement>()
-    }
-}
-
-#[derive(JSTraceable, MallocSizeOf)]
-struct ScriptsFilter;
-impl CollectionFilter for ScriptsFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        elem.is::<HTMLScriptElement>()
-    }
-}
-
-#[derive(JSTraceable, MallocSizeOf)]
-struct AnchorsFilter;
-impl CollectionFilter for AnchorsFilter {
-    fn filter(&self, elem: &Element, _root: &Node) -> bool {
-        elem.is::<HTMLAnchorElement>() && elem.has_attribute(&local_name!("href"))
-    }
-}
-
 #[allow(non_snake_case)]
 impl Document {
     pub fn note_node_with_dirty_descendants(&self, node: &Node) {
@@ -4907,16 +4858,18 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
     // https://html.spec.whatwg.org/multipage/#dom-document-images
     fn Images(&self) -> DomRoot<HTMLCollection> {
         self.images.or_init(|| {
-            let filter = Box::new(ImagesFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                element.is::<HTMLImageElement>()
+            })
         })
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-embeds
     fn Embeds(&self) -> DomRoot<HTMLCollection> {
         self.embeds.or_init(|| {
-            let filter = Box::new(EmbedsFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                element.is::<HTMLEmbedElement>()
+            })
         })
     }
 
@@ -4928,32 +4881,37 @@ impl DocumentMethods<crate::DomTypeHolder> for Document {
     // https://html.spec.whatwg.org/multipage/#dom-document-links
     fn Links(&self) -> DomRoot<HTMLCollection> {
         self.links.or_init(|| {
-            let filter = Box::new(LinksFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                (element.is::<HTMLAnchorElement>() || element.is::<HTMLAreaElement>()) &&
+                    element.has_attribute(&local_name!("href"))
+            })
         })
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-forms
     fn Forms(&self) -> DomRoot<HTMLCollection> {
         self.forms.or_init(|| {
-            let filter = Box::new(FormsFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                element.is::<HTMLFormElement>()
+            })
         })
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-scripts
     fn Scripts(&self) -> DomRoot<HTMLCollection> {
         self.scripts.or_init(|| {
-            let filter = Box::new(ScriptsFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                element.is::<HTMLScriptElement>()
+            })
         })
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-document-anchors
     fn Anchors(&self) -> DomRoot<HTMLCollection> {
         self.anchors.or_init(|| {
-            let filter = Box::new(AnchorsFilter);
+            HTMLCollection::new_with_filter_fn(&self.window, self.upcast(), |element, _| {
-            HTMLCollection::create(&self.window, self.upcast(), filter)
+                element.is::<HTMLAnchorElement>() && element.has_attribute(&local_name!("href"))
+            })
         })
     }
 

components/script/dom/htmlcollection.rs

@@ -104,20 +104,46 @@ impl HTMLCollection {
         window: &Window,
         root: &Node,
         filter: Box<dyn CollectionFilter + 'static>,
-    ) -> DomRoot<HTMLCollection> {
+    ) -> DomRoot<Self> {
         reflect_dom_object(
-            Box::new(HTMLCollection::new_inherited(root, filter)),
+            Box::new(Self::new_inherited(root, filter)),
             window,
             CanGc::note(),
         )
     }
 
-    pub fn create(
+    /// Create a new  [`HTMLCollection`] that just filters element using a static function.
+    pub(crate) fn new_with_filter_fn(
+        window: &Window,
+        root: &Node,
+        filter_function: fn(&Element, &Node) -> bool,
+    ) -> DomRoot<Self> {
+        #[derive(JSTraceable, MallocSizeOf)]
+        pub(crate) struct StaticFunctionFilter(
+            // The function *must* be static so that it never holds references to DOM objects, which
+            // would cause issues with garbage collection -- since it isn't traced.
+            #[no_trace]
+            #[ignore_malloc_size_of = "Static function pointer"]
+            fn(&Element, &Node) -> bool,
+        );
+        impl CollectionFilter for StaticFunctionFilter {
+            fn filter(&self, element: &Element, root: &Node) -> bool {
+                (self.0)(element, root)
+            }
+        }
+        Self::new(
+            window,
+            root,
+            Box::new(StaticFunctionFilter(filter_function)),
+        )
+    }
+
+    pub(crate) fn create(
         window: &Window,
         root: &Node,
         filter: Box<dyn CollectionFilter + 'static>,
-    ) -> DomRoot<HTMLCollection> {
+    ) -> DomRoot<Self> {
-        HTMLCollection::new(window, root, filter)
+        Self::new(window, root, filter)
     }
 
     fn validate_cache(&self) {
@@ -275,14 +301,9 @@ impl HTMLCollection {
     }
 
     pub fn children(window: &Window, root: &Node) -> DomRoot<HTMLCollection> {
-        #[derive(JSTraceable, MallocSizeOf)]
+        HTMLCollection::new_with_filter_fn(window, root, |element, root| {
-        struct ElementChildFilter;
+            root.is_parent_of(element.upcast())
-        impl CollectionFilter for ElementChildFilter {
+        })
-            fn filter(&self, elem: &Element, root: &Node) -> bool {
-                root.is_parent_of(elem.upcast())
-            }
-        }
-        HTMLCollection::create(window, root, Box::new(ElementChildFilter))
     }
 
     pub fn elements_iter_after<'a>(

components/script/dom/htmldatalistelement.rs

@@ -10,8 +10,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLDataListElementBinding::HTMLDat
 use crate::dom::bindings::inheritance::Castable;
 use crate::dom::bindings::root::DomRoot;
 use crate::dom::document::Document;
-use crate::dom::element::Element;
+use crate::dom::htmlcollection::HTMLCollection;
-use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::htmloptionelement::HTMLOptionElement;
 use crate::dom::node::{window_from_node, Node};
@@ -55,15 +54,8 @@ impl HTMLDataListElement {
 impl HTMLDataListElementMethods<crate::DomTypeHolder> for HTMLDataListElement {
     // https://html.spec.whatwg.org/multipage/#dom-datalist-options
     fn Options(&self) -> DomRoot<HTMLCollection> {
-        #[derive(JSTraceable, MallocSizeOf)]
+        HTMLCollection::new_with_filter_fn(&window_from_node(self), self.upcast(), |element, _| {
-        struct HTMLDataListOptionsFilter;
+            element.is::<HTMLOptionElement>()
-        impl CollectionFilter for HTMLDataListOptionsFilter {
+        })
-            fn filter(&self, elem: &Element, _root: &Node) -> bool {
-                elem.is::<HTMLOptionElement>()
-            }
-        }
-        let filter = Box::new(HTMLDataListOptionsFilter);
-        let window = window_from_node(self);
-        HTMLCollection::create(&window, self.upcast(), filter)
     }
 }

components/script/dom/htmlfieldsetelement.rs

@@ -17,7 +17,7 @@ use crate::dom::bindings::str::DOMString;
 use crate::dom::customelementregistry::CallbackReaction;
 use crate::dom::document::Document;
 use crate::dom::element::{AttributeMutation, Element};
-use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
+use crate::dom::htmlcollection::HTMLCollection;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::htmlformelement::{FormControl, HTMLFormElement};
 use crate::dom::htmllegendelement::HTMLLegendElement;
@@ -88,17 +88,11 @@ impl HTMLFieldSetElement {
 impl HTMLFieldSetElementMethods<crate::DomTypeHolder> for HTMLFieldSetElement {
     // https://html.spec.whatwg.org/multipage/#dom-fieldset-elements
     fn Elements(&self) -> DomRoot<HTMLCollection> {
-        #[derive(JSTraceable, MallocSizeOf)]
+        HTMLCollection::new_with_filter_fn(&window_from_node(self), self.upcast(), |element, _| {
-        struct ElementsFilter;
+            element
-        impl CollectionFilter for ElementsFilter {
+                .downcast::<HTMLElement>()
-            fn filter<'a>(&self, elem: &'a Element, _root: &'a Node) -> bool {
+                .is_some_and(HTMLElement::is_listed_element)
-                elem.downcast::<HTMLElement>()
+        })
-                    .is_some_and(HTMLElement::is_listed_element)
-            }
-        }
-        let filter = Box::new(ElementsFilter);
-        let window = window_from_node(self);
-        HTMLCollection::create(&window, self.upcast(), filter)
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-fieldset-disabled

components/script/dom/htmltableelement.rs

@@ -300,20 +300,16 @@ impl HTMLTableElementMethods<crate::DomTypeHolder> for HTMLTableElement {
 
     // https://html.spec.whatwg.org/multipage/#dom-table-tbodies
     fn TBodies(&self) -> DomRoot<HTMLCollection> {
-        #[derive(JSTraceable)]
-        struct TBodiesFilter;
-        impl CollectionFilter for TBodiesFilter {
-            fn filter(&self, elem: &Element, root: &Node) -> bool {
-                elem.is::<HTMLTableSectionElement>() &&
-                    elem.local_name() == &local_name!("tbody") &&
-                    elem.upcast::<Node>().GetParentNode().as_deref() == Some(root)
-            }
-        }
-
         self.tbodies.or_init(|| {
-            let window = window_from_node(self);
+            HTMLCollection::new_with_filter_fn(
-            let filter = Box::new(TBodiesFilter);
+                &window_from_node(self),
-            HTMLCollection::create(&window, self.upcast(), filter)
+                self.upcast(),
+                |element, root| {
+                    element.is::<HTMLTableSectionElement>() &&
+                        element.local_name() == &local_name!("tbody") &&
+                        element.upcast::<Node>().GetParentNode().as_deref() == Some(root)
+                },
+            )
         })
     }
 

components/script/dom/htmltablerowelement.rs

@@ -18,7 +18,7 @@ use crate::dom::bindings::root::{DomRoot, LayoutDom, MutNullableDom};
 use crate::dom::bindings::str::DOMString;
 use crate::dom::document::Document;
 use crate::dom::element::{Element, LayoutElementHelpers};
-use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
+use crate::dom::htmlcollection::HTMLCollection;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::htmltablecellelement::HTMLTableCellElement;
 use crate::dom::htmltableelement::HTMLTableElement;
@@ -27,15 +27,6 @@ use crate::dom::node::{window_from_node, Node};
 use crate::dom::virtualmethods::VirtualMethods;
 use crate::script_runtime::CanGc;
 
-#[derive(JSTraceable)]
-struct CellsFilter;
-impl CollectionFilter for CellsFilter {
-    fn filter(&self, elem: &Element, root: &Node) -> bool {
-        (elem.is::<HTMLTableCellElement>()) &&
-            elem.upcast::<Node>().GetParentNode().as_deref() == Some(root)
-    }
-}
-
 #[dom_struct]
 pub struct HTMLTableRowElement {
     htmlelement: HTMLElement,
@@ -95,9 +86,14 @@ impl HTMLTableRowElementMethods<crate::DomTypeHolder> for HTMLTableRowElement {
     // https://html.spec.whatwg.org/multipage/#dom-tr-cells
     fn Cells(&self) -> DomRoot<HTMLCollection> {
         self.cells.or_init(|| {
-            let window = window_from_node(self);
+            HTMLCollection::new_with_filter_fn(
-            let filter = Box::new(CellsFilter);
+                &window_from_node(self),
-            HTMLCollection::create(&window, self.upcast(), filter)
+                self.upcast(),
+                |element, root| {
+                    (element.is::<HTMLTableCellElement>()) &&
+                        element.upcast::<Node>().GetParentNode().as_deref() == Some(root)
+                },
+            )
         })
     }
 

components/script/dom/htmltablesectionelement.rs

@@ -16,7 +16,7 @@ use crate::dom::bindings::root::{DomRoot, LayoutDom};
 use crate::dom::bindings::str::DOMString;
 use crate::dom::document::Document;
 use crate::dom::element::{Element, LayoutElementHelpers};
-use crate::dom::htmlcollection::{CollectionFilter, HTMLCollection};
+use crate::dom::htmlcollection::HTMLCollection;
 use crate::dom::htmlelement::HTMLElement;
 use crate::dom::htmltablerowelement::HTMLTableRowElement;
 use crate::dom::node::{window_from_node, Node};
@@ -61,19 +61,17 @@ impl HTMLTableSectionElement {
     }
 }
 
-#[derive(JSTraceable)]
-struct RowsFilter;
-impl CollectionFilter for RowsFilter {
-    fn filter(&self, elem: &Element, root: &Node) -> bool {
-        elem.is::<HTMLTableRowElement>() &&
-            elem.upcast::<Node>().GetParentNode().as_deref() == Some(root)
-    }
-}
-
 impl HTMLTableSectionElementMethods<crate::DomTypeHolder> for HTMLTableSectionElement {
     // https://html.spec.whatwg.org/multipage/#dom-tbody-rows
     fn Rows(&self) -> DomRoot<HTMLCollection> {
-        HTMLCollection::create(&window_from_node(self), self.upcast(), Box::new(RowsFilter))
+        HTMLCollection::new_with_filter_fn(
+            &window_from_node(self),
+            self.upcast(),
+            |element, root| {
+                element.is::<HTMLTableRowElement>() &&
+                    element.upcast::<Node>().GetParentNode().as_deref() == Some(root)
+            },
+        )
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-tbody-insertrow