[librsvg: 2/3] document: factor out a DocumentBuilder object

[Federico Mena Quintero <federico src gnome org>]

commit 02e7fc7823edad0d0d98f4823410c283e952b6d0
Author: Paolo Borelli <pborelli gnome org>
Date:   Sun Oct 27 13:18:12 2019 +0100

    document: factor out a DocumentBuilder object

 rsvg_internals/src/create_node.rs |  15 +----
 rsvg_internals/src/document.rs    | 115 ++++++++++++++++++++++++++++++++-
 rsvg_internals/src/xml.rs         | 130 +++++++++++---------------------------
 3 files changed, 154 insertions(+), 106 deletions(-)
---
diff --git a/rsvg_internals/src/create_node.rs b/rsvg_internals/src/create_node.rs
index 8f4cc33a..bbeab051 100644
--- a/rsvg_internals/src/create_node.rs
+++ b/rsvg_internals/src/create_node.rs
@@ -228,11 +228,7 @@ lazy_static! {
     };
 }
 
-pub fn create_node_and_register_id(
-    name: &QualName,
-    pbag: &PropertyBag,
-    ids: &mut HashMap<String, RsvgNode>,
-) -> RsvgNode {
+pub fn create_node(name: &QualName, pbag: &PropertyBag) -> RsvgNode {
     let mut id = None;
     let mut class = None;
 
@@ -262,12 +258,5 @@ pub fn create_node_and_register_id(
         class = None;
     };
 
-    let node = create_fn(name, id, class);
-
-    if let Some(id) = id {
-        // This is so we don't overwrite an existing id
-        ids.entry(id.to_string()).or_insert_with(|| node.clone());
-    }
-
-    node
+    create_fn(name, id, class)
 }
diff --git a/rsvg_internals/src/document.rs b/rsvg_internals/src/document.rs
index 32cc6c20..afd18137 100644
--- a/rsvg_internals/src/document.rs
+++ b/rsvg_internals/src/document.rs
@@ -1,18 +1,23 @@
 use gdk_pixbuf::{PixbufLoader, PixbufLoaderExt};
 use gio;
+use markup5ever::{LocalName, Namespace, QualName};
 use std::cell::RefCell;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::rc::Rc;
 
 use crate::allowed_url::{AllowedUrl, Fragment};
+use crate::create_node::create_node;
+use crate::css::CssRules;
 use crate::error::LoadingError;
 use crate::handle::LoadOptions;
 use crate::io::{self, BinaryData};
-use crate::node::{NodeCascade, NodeType, RsvgNode};
+use crate::node::{NodeCascade, NodeData, NodeType, RsvgNode};
 use crate::properties::ComputedValues;
+use crate::property_bag::PropertyBag;
 use crate::structure::{IntrinsicDimensions, NodeSvg};
 use crate::surface_utils::shared_surface::SharedImageSurface;
+use crate::text::NodeChars;
 use crate::xml::xml_load_from_possibly_compressed_stream;
 
 /// A loaded SVG file and its derived data
@@ -213,3 +218,111 @@ fn load_image(
 
     Ok(surface)
 }
+
+pub struct DocumentBuilder {
+    load_options: LoadOptions,
+    tree: Option<RsvgNode>,
+    ids: HashMap<String, RsvgNode>,
+    css_rules: CssRules,
+}
+
+impl DocumentBuilder {
+    pub fn new(load_options: &LoadOptions) -> DocumentBuilder {
+        DocumentBuilder {
+            load_options: load_options.clone(),
+            tree: None,
+            ids: HashMap::new(),
+            css_rules: CssRules::default(),
+        }
+    }
+
+    pub fn append_element(
+        &mut self,
+        name: &QualName,
+        pbag: &PropertyBag,
+        parent: Option<RsvgNode>,
+    ) -> RsvgNode {
+        let mut node = create_node(name, pbag);
+
+        if let Some(id) = node.borrow().get_id() {
+            // This is so we don't overwrite an existing id
+            self.ids
+                .entry(id.to_string())
+                .or_insert_with(|| node.clone());
+        }
+
+        node.borrow_mut()
+            .set_atts(parent.as_ref().clone(), pbag, self.load_options.locale());
+
+        if let Some(mut parent) = parent {
+            parent.append(node.clone());
+        } else if self.tree.is_none() {
+            self.tree = Some(node.clone());
+        } else {
+            panic!("The tree root has already been set");
+        }
+
+        node
+    }
+
+    pub fn append_characters(&mut self, text: &str, parent: &mut RsvgNode) {
+        if text.is_empty() {
+            return;
+        }
+
+        // When the last child is a Chars node we can coalesce
+        // the text and avoid screwing up the Pango layouts
+        let chars_node = if let Some(child) = parent
+            .last_child()
+            .filter(|c| c.borrow().get_type() == NodeType::Chars)
+        {
+            child
+        } else {
+            let child = RsvgNode::new(NodeData::new(
+                NodeType::Chars,
+                &QualName::new(
+                    None,
+                    Namespace::from("https://wiki.gnome.org/Projects/LibRsvg";),
+                    LocalName::from("rsvg-chars"),
+                ),
+                None,
+                None,
+                Box::new(NodeChars::new()),
+            ));
+
+            parent.append(child.clone());
+
+            child
+        };
+
+        chars_node.borrow().get_impl::<NodeChars>().append(text);
+    }
+
+    pub fn load_css(&mut self, url: &AllowedUrl) {
+        // FIXME: handle CSS errors
+        let _ = self.css_rules.load_css(&url);
+    }
+
+    pub fn parse_css(&mut self, css_data: &str) {
+        self.css_rules
+            .parse(self.load_options.base_url.as_ref(), css_data);
+    }
+
+    pub fn build(mut self) -> Result<Document, LoadingError> {
+        match self.tree {
+            None => Err(LoadingError::SvgHasNoElements),
+            Some(ref root) if root.borrow().get_type() == NodeType::Svg => {
+                for mut node in root.descendants() {
+                    node.borrow_mut().set_style(&self.css_rules);
+                }
+
+                Ok(Document::new(
+                    self.tree.take().unwrap(),
+                    self.ids,
+                    self.load_options.clone(),
+                ))
+            }
+            _ => Err(LoadingError::RootElementIsNotSvg),
+        }
+    }
+}
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index d74d8561..c236a717 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -9,17 +9,14 @@ use std::rc::{Rc, Weak};
 use std::str;
 
 use crate::allowed_url::AllowedUrl;
-use crate::create_node::create_node_and_register_id;
-use crate::css::CssRules;
-use crate::document::Document;
+use crate::document::{Document, DocumentBuilder};
 use crate::error::LoadingError;
 use crate::handle::LoadOptions;
 use crate::io::{self, get_input_stream_for_loading};
 use crate::limits::MAX_LOADED_ELEMENTS;
-use crate::node::{NodeData, NodeType, RsvgNode};
+use crate::node::{NodeType, RsvgNode};
 use crate::property_bag::PropertyBag;
 use crate::style::NodeStyle;
-use crate::text::NodeChars;
 use crate::xml2_load::{ParseFromStreamError, Xml2Parser};
 
 #[derive(Clone)]
@@ -82,10 +79,8 @@ macro_rules! xinclude_name {
 /// what creates normal graphical elements.
 struct XmlStateInner {
     weak: Option<Weak<XmlState>>,
+    document_builder: Option<DocumentBuilder>,
     num_loaded_elements: usize,
-    tree_root: Option<RsvgNode>,
-    ids: Option<HashMap<String, RsvgNode>>,
-    css_rules: Option<CssRules>,
     context_stack: Vec<Context>,
     current_node: Option<RsvgNode>,
 
@@ -122,10 +117,8 @@ impl XmlState {
         XmlState {
             inner: RefCell::new(XmlStateInner {
                 weak: None,
+                document_builder: Some(DocumentBuilder::new(load_options)),
                 num_loaded_elements: 0,
-                tree_root: None,
-                ids: Some(HashMap::new()),
-                css_rules: Some(CssRules::default()),
                 context_stack: vec![Context::Start],
                 current_node: None,
                 entities: HashMap::new(),
@@ -144,31 +137,6 @@ impl XmlState {
         }
     }
 
-    fn steal_result(&self) -> Result<Document, LoadingError> {
-        self.check_last_error()?;
-
-        let mut inner = self.inner.borrow_mut();
-
-        match inner.tree_root {
-            None => Err(LoadingError::SvgHasNoElements),
-            Some(ref root) if root.borrow().get_type() == NodeType::Svg => {
-                let root = inner.tree_root.take().unwrap();
-                let css_rules = inner.css_rules.as_ref().unwrap();
-
-                for mut node in root.descendants() {
-                    node.borrow_mut().set_style(css_rules);
-                }
-
-                Ok(Document::new(
-                    root,
-                    inner.ids.take().unwrap(),
-                    self.load_options.clone(),
-                ))
-            }
-            _ => Err(LoadingError::RootElementIsNotSvg),
-        }
-    }
-
     fn check_limits(&self) -> Result<(), ()> {
         if self.inner.borrow().num_loaded_elements > MAX_LOADED_ELEMENTS {
             self.error(ParseFromStreamError::XmlParseError(format!(
@@ -272,9 +240,7 @@ impl XmlState {
                 if let Ok(aurl) =
                     AllowedUrl::from_href(&href.unwrap(), self.load_options.base_url.as_ref())
                 {
-                    // FIXME: handle CSS errors
-                    let css_rules = inner.css_rules.as_mut().unwrap();
-                    let _ = css_rules.load_css(&aurl);
+                    inner.document_builder.as_mut().unwrap().load_css(&aurl);
                 } else {
                     self.error(ParseFromStreamError::XmlParseError(String::from(
                         "disallowed URL in xml-stylesheet",
@@ -317,23 +283,12 @@ impl XmlState {
         } else {
             let mut inner = self.inner.borrow_mut();
 
-            let ids = inner.ids.as_mut().unwrap();
-            let mut node = create_node_and_register_id(name, pbag, ids);
-
             let parent = inner.current_node.clone();
-            node.borrow_mut()
-                .set_atts(parent.as_ref(), pbag, self.load_options.locale());
-
-            if let Some(mut parent) = parent {
-                parent.append(node.clone());
-            } else {
-                if inner.tree_root.is_some() {
-                    panic!("The tree root has already been set");
-                }
-
-                inner.tree_root = Some(node.clone());
-            }
-
+            let node = inner
+                .document_builder
+                .as_mut()
+                .unwrap()
+                .append_element(name, pbag, parent);
             inner.current_node = Some(node);
 
             Context::ElementCreation
@@ -346,48 +301,26 @@ impl XmlState {
         let node = inner.current_node.take().unwrap();
 
         if node.borrow().get_type() == NodeType::Style {
-            let css_rules = inner.css_rules.as_mut().unwrap();
             let css_data = node.borrow().get_impl::<NodeStyle>().get_css(&node);
-
-            css_rules.parse(self.load_options.base_url.as_ref(), &css_data);
+            inner
+                .document_builder
+                .as_mut()
+                .unwrap()
+                .parse_css(&css_data);
         }
 
         inner.current_node = node.parent();
     }
 
     fn element_creation_characters(&self, text: &str) {
-        if text.is_empty() {
-            return;
-        }
-
-        // When the last child is a Chars node we can coalesce
-        // the text and avoid screwing up the Pango layouts
-        let mut current_node = self.inner.borrow().current_node.as_ref().unwrap().clone();
-        let chars_node = if let Some(child) = current_node
-            .last_child()
-            .filter(|c| c.borrow().get_type() == NodeType::Chars)
-        {
-            child
-        } else {
-            let child = RsvgNode::new(NodeData::new(
-                NodeType::Chars,
-                &QualName::new(
-                    None,
-                    Namespace::from("https://wiki.gnome.org/Projects/LibRsvg";),
-                    LocalName::from("rsvg-chars"),
-                ),
-                None,
-                None,
-                Box::new(NodeChars::new()),
-            ));
-
-            self.inner.borrow_mut().num_loaded_elements += 1;
-            current_node.append(child.clone());
-
-            child
-        };
+        let mut inner = self.inner.borrow_mut();
 
-        chars_node.borrow().get_impl::<NodeChars>().append(text);
+        let mut parent = inner.current_node.clone().unwrap();
+        inner
+            .document_builder
+            .as_mut()
+            .unwrap()
+            .append_characters(text, &mut parent);
     }
 
     fn xinclude_start_element(&self, _name: &QualName, pbag: &PropertyBag) -> Context {
@@ -578,6 +511,21 @@ impl XmlState {
     fn unsupported_xinclude_start_element(&self, _name: &QualName) -> Context {
         Context::UnsupportedXIncludeChild
     }
+
+    fn build_document(
+        &self,
+        stream: &gio::InputStream,
+        cancellable: Option<&gio::Cancellable>,
+    ) -> Result<Document, LoadingError> {
+        self.parse_from_stream(stream, cancellable)?;
+
+        self.inner
+            .borrow_mut()
+            .document_builder
+            .take()
+            .unwrap()
+            .build()
+    }
 }
 
 impl Drop for XmlState {
@@ -637,7 +585,5 @@ pub fn xml_load_from_possibly_compressed_stream(
     let stream =
         get_input_stream_for_loading(stream, cancellable).map_err(ParseFromStreamError::IoError)?;
 
-    state.parse_from_stream(&stream, cancellable)?;
-
-    state.steal_result()
+    state.build_document(&stream, cancellable)
 }