Avoid dropping template contents nodes recursively.

jdm · jdm · commit f54a307885a3 · 2020-06-06T11:07:36.000-04:00
diff --git a/rcdom/lib.rs b/rcdom/lib.rs
@@ -89,7 +89,7 @@ pub enum NodeData {
         /// For HTML \<template\> elements, the [template contents].
         ///
         /// [template contents]: https://html.spec.whatwg.org/multipage/#template-contents
-        template_contents: Option<Handle>,
+        template_contents: RefCell<Option<Handle>>,
 
         /// Whether the node is a [HTML integration point].
         ///
@@ -131,6 +131,11 @@ impl Drop for Node {
         while let Some(node) = nodes.pop() {
             let children = mem::replace(&mut *node.children.borrow_mut(), vec![]);
             nodes.extend(children.into_iter());
+            if let NodeData::Element { ref template_contents, .. } = node.data {
+                if let Some(template_contents) = template_contents.borrow_mut().take() {
+                    nodes.push(template_contents);
+                }
+            }
         }
     }
 }
@@ -226,11 +231,11 @@ impl TreeSink for RcDom {
 
     fn get_template_contents(&mut self, target: &Handle) -> Handle {
         if let NodeData::Element {
-            template_contents: Some(ref contents),
+            ref template_contents,
             ..
         } = target.data
         {
-            contents.clone()
+            template_contents.borrow().as_ref().expect("not a template element!").clone()
         } else {
             panic!("not a template element!")
         }
@@ -260,11 +265,11 @@ impl TreeSink for RcDom {
         Node::new(NodeData::Element {
             name: name,
             attrs: RefCell::new(attrs),
-            template_contents: if flags.template {
+            template_contents: RefCell::new(if flags.template {
                 Some(Node::new(NodeData::Document))
             } else {
                 None
-            },
+            }),
             mathml_annotation_xml_integration_point: flags.mathml_annotation_xml_integration_point,
         })
     }
diff --git a/rcdom/tests/html-driver.rs b/rcdom/tests/html-driver.rs
@@ -16,3 +16,14 @@ fn from_utf8() {
         "<html><head><title>Test</title></head><body></body></html>"
     );
 }
+
+#[test]
+fn many_templates() {
+    let mut body = String::new();
+    for _ in 1..10000 {
+        body.push_str("<template>");
+    }
+    let _ = driver::parse_document(RcDom::default(), Default::default())
+        .from_utf8()
+        .one(body.as_bytes());
+}
diff --git a/rcdom/tests/html-tree-builder.rs b/rcdom/tests/html-tree-builder.rs
@@ -146,15 +146,17 @@ fn serialize(buf: &mut String, indent: usize, handle: Handle) {
     }
 
     if let NodeData::Element {
-        template_contents: Some(ref content),
+        ref template_contents,
         ..
     } = node.data
     {
-        buf.push_str("|");
-        buf.push_str(&repeat(" ").take(indent + 2).collect::<String>());
-        buf.push_str("content\n");
-        for child in content.children.borrow().iter() {
-            serialize(buf, indent + 4, child.clone());
+        if let Some(ref content) = &*template_contents.borrow() {
+            buf.push_str("|");
+            buf.push_str(&repeat(" ").take(indent + 2).collect::<String>());
+            buf.push_str("content\n");
+            for child in content.children.borrow().iter() {
+                serialize(buf, indent + 4, child.clone());
+            }
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -146,15 +146,17 @@ fn serialize(buf: &mut String, indent: usize, handle: Handle) {`
`146`	`146`	`}`
`147`	`147`
`148`	`148`	`if let NodeData::Element {`
`149`		`- template_contents: Some(ref content),`
	`149`	`+ ref template_contents,`
`150`	`150`	`..`
`151`	`151`	`} = node.data`
`152`	`152`	`{`
`153`		`- buf.push_str("\|");`
`154`		`- buf.push_str(&repeat(" ").take(indent + 2).collect::<String>());`
`155`		`- buf.push_str("content\n");`
`156`		`- for child in content.children.borrow().iter() {`
`157`		`- serialize(buf, indent + 4, child.clone());`
	`153`	`+ if let Some(ref content) = &*template_contents.borrow() {`
	`154`	`+ buf.push_str("\|");`
	`155`	`+ buf.push_str(&repeat(" ").take(indent + 2).collect::<String>());`
	`156`	`+ buf.push_str("content\n");`
	`157`	`+ for child in content.children.borrow().iter() {`
	`158`	`+ serialize(buf, indent + 4, child.clone());`
	`159`	`+ }`
`158`	`160`	`}`
`159`	`161`	`}`
`160`	`162`	`}`