[librsvg: 42/48] Use a real NodeStyle for the <style> element instead of special loading logic

[Federico Mena Quintero <federico src gnome org>]

commit e14253a9fb2e33e9fcf40badd31b99e79a820b1a
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri Nov 16 10:56:16 2018 -0600

    Use a real NodeStyle for the <style> element instead of special loading logic
    
    Instead of having special cases in the state machine for loading, we
    now have a NodeStyle that just stores NodeChars with the stylesheet
    data.  The code parses that data if it is text/css when the <style>
    element is closed.

 Makefile.am                 |  1 +
 rsvg_internals/src/css.rs   |  4 +++
 rsvg_internals/src/lib.rs   |  1 +
 rsvg_internals/src/load.rs  |  4 ++-
 rsvg_internals/src/node.rs  |  1 +
 rsvg_internals/src/style.rs | 72 ++++++++++++++++++++++++++++++++++++++++
 rsvg_internals/src/text.rs  |  4 +++
 rsvg_internals/src/xml.rs   | 81 +++++----------------------------------------
 8 files changed, 94 insertions(+), 74 deletions(-)
---
diff --git a/Makefile.am b/Makefile.am
index edfe723d..2b5a2827 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -110,6 +110,7 @@ RUST_SRC =                                                  \
        rsvg_internals/src/state.rs                             \
        rsvg_internals/src/stop.rs                              \
        rsvg_internals/src/structure.rs                         \
+       rsvg_internals/src/style.rs                             \
        rsvg_internals/src/text.rs                              \
        rsvg_internals/src/transform.rs                         \
        rsvg_internals/src/tree.rs                              \
diff --git a/rsvg_internals/src/css.rs b/rsvg_internals/src/css.rs
index ba4c4657..03ee577e 100644
--- a/rsvg_internals/src/css.rs
+++ b/rsvg_internals/src/css.rs
@@ -87,6 +87,10 @@ struct DocHandlerData {
 }
 
 pub fn parse_into_handle(handle: *mut RsvgHandle, buf: &str) {
+    if buf.len() == 0 {
+        return; // libcroco doesn't like empty strings :(
+    }
+
     unsafe {
         let handler_data = DocHandlerData {
             handle,
diff --git a/rsvg_internals/src/lib.rs b/rsvg_internals/src/lib.rs
index 2783da78..ec6d4132 100644
--- a/rsvg_internals/src/lib.rs
+++ b/rsvg_internals/src/lib.rs
@@ -121,6 +121,7 @@ pub mod srgb;
 mod state;
 mod stop;
 mod structure;
+mod style;
 pub mod surface_utils;
 mod text;
 mod transform;
diff --git a/rsvg_internals/src/load.rs b/rsvg_internals/src/load.rs
index 7682e954..63c4a6b1 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -32,6 +32,7 @@ use property_bag::PropertyBag;
 use shapes::{NodeCircle, NodeEllipse, NodeLine, NodePath, NodePoly, NodeRect};
 use stop::NodeStop;
 use structure::{NodeDefs, NodeGroup, NodeSvg, NodeSwitch, NodeSymbol, NodeUse};
+use style::NodeStyle;
 use text::{NodeTRef, NodeTSpan, NodeText};
 
 macro_rules! node_create_fn {
@@ -159,6 +160,7 @@ node_create_fn!(
 );
 node_create_fn!(create_spot_light, LightSource, LightSource::new_spot_light);
 node_create_fn!(create_stop, Stop, NodeStop::new);
+node_create_fn!(create_style, Style, NodeStyle::new);
 node_create_fn!(create_svg, Svg, NodeSvg::new);
 node_create_fn!(create_switch, Switch, NodeSwitch::new);
 node_create_fn!(create_symbol, Symbol, NodeSymbol::new);
@@ -251,7 +253,7 @@ lazy_static! {
         /* h.insert("script",           (false, as NodeCreateFn)); */
         /* h.insert("set",              (false, as NodeCreateFn)); */
         h.insert("stop",                (true,  create_stop as NodeCreateFn));
-        /* h.insert("style",            (false, as NodeCreateFn)); */
+        h.insert("style",               (false, create_style as NodeCreateFn));
         h.insert("subImage",            (false, create_group as NodeCreateFn));
         h.insert("subImageRef",         (false, create_image as NodeCreateFn));
         h.insert("svg",                 (true,  create_svg as NodeCreateFn));
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index 4fc0fe4b..4337ef92 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -199,6 +199,7 @@ pub enum NodeType {
     RadialGradient,
     Rect,
     Stop,
+    Style,
     Svg,
     Switch,
     Symbol,
diff --git a/rsvg_internals/src/style.rs b/rsvg_internals/src/style.rs
new file mode 100644
index 00000000..55b93fc8
--- /dev/null
+++ b/rsvg_internals/src/style.rs
@@ -0,0 +1,72 @@
+use attributes::Attribute;
+use handle::RsvgHandle;
+use node::{NodeResult, NodeTrait, NodeType, RsvgNode};
+use property_bag::PropertyBag;
+use text::NodeChars;
+
+use std::cell::RefCell;
+
+/// Represents a <style> node.
+///
+/// It does not render itself, and just holds CSS stylesheet information for the rest of
+/// the code to use.
+pub struct NodeStyle {
+    type_: RefCell<Option<String>>,
+}
+
+impl NodeStyle {
+    pub fn new() -> NodeStyle {
+        NodeStyle {
+            type_: RefCell::new(None),
+        }
+    }
+
+    pub fn get_css(&self, node: &RsvgNode) -> String {
+        // FIXME: See these:
+        //
+        // https://www.w3.org/TR/SVG11/styling.html#StyleElementTypeAttribute
+        // https://www.w3.org/TR/SVG11/styling.html#ContentStyleTypeAttribute
+        //
+        // If the "type" attribute is not present, we should fallback to the
+        // "contentStyleType" attribute of the svg element, which in turn
+        // defaults to "text/css".
+
+        let have_css = self
+            .type_
+            .borrow()
+            .as_ref()
+            .map(|t| t == "text/css")
+            .unwrap_or(true);
+
+        if have_css {
+            node.children()
+                .into_iter()
+                .filter_map(|child| {
+                    if child.get_type() == NodeType::Chars {
+                        Some(child.with_impl(|chars: &NodeChars| chars.get_string()))
+                    } else {
+                        None
+                    }
+                })
+                .collect::<String>()
+        } else {
+            "".to_string()
+        }
+    }
+}
+
+impl NodeTrait for NodeStyle {
+    fn set_atts(&self, _: &RsvgNode, _: *const RsvgHandle, pbag: &PropertyBag<'_>) -> NodeResult {
+        for (_key, attr, value) in pbag.iter() {
+            if attr == Attribute::Type {
+                *self.type_.borrow_mut() = Some(value.to_string());
+            }
+        }
+
+        Ok(())
+    }
+
+    fn accept_chars(&self) -> bool {
+        true
+    }
+}
diff --git a/rsvg_internals/src/text.rs b/rsvg_internals/src/text.rs
index efeffb2a..c664781a 100644
--- a/rsvg_internals/src/text.rs
+++ b/rsvg_internals/src/text.rs
@@ -56,6 +56,10 @@ impl NodeChars {
         }
     }
 
+    pub fn get_string(&self) -> String {
+        self.string.borrow().clone()
+    }
+
     pub fn append(&self, s: &str) {
         self.string.borrow_mut().push_str(s);
     }
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 1010690f..253c2be6 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -14,6 +14,7 @@ use load::rsvg_load_new_node;
 use node::{node_new, Node, NodeType};
 use property_bag::PropertyBag;
 use structure::NodeSvg;
+use style::NodeStyle;
 use text::NodeChars;
 use tree::{RsvgTree, Tree};
 use util::utf8_cstr;
@@ -25,12 +26,6 @@ enum ContextKind {
     // Creating nodes for elements under the current node
     ElementCreation,
 
-    // Inside a <style> element
-    Style(StyleContext),
-
-    // An element inside a <style> context, to be ignored
-    UnsupportedStyleChild,
-
     // Inside <xi:include>
     XInclude(XIncludeContext),
 
@@ -41,12 +36,6 @@ enum ContextKind {
     XIncludeFallback(XIncludeContext),
 }
 
-/// Handles the `<style>` element by parsing its character contents as CSS
-struct StyleContext {
-    is_text_css: bool,
-    text: String,
-}
-
 #[derive(Clone)]
 struct XIncludeContext {
     need_fallback: bool,
@@ -79,9 +68,6 @@ pub enum RsvgXmlState {}
 /// that context, all XML events will be forwarded to it, and processed in one of the `XmlHandler`
 /// trait objects. Normally the context refers to a `NodeCreationContext` implementation which is
 /// what creates normal graphical elements.
-///
-/// When we get to a `<style>` element, we push a `StyleContext`, which processes its contents
-/// specially.
 struct XmlState {
     tree: Option<Box<Tree>>,
     context: Context,
@@ -89,10 +75,6 @@ struct XmlState {
     current_node: Option<Rc<Node>>,
 }
 
-fn style_characters(style_ctx: &mut StyleContext, text: &str) {
-    style_ctx.text.push_str(text);
-}
-
 impl XmlState {
     fn new() -> XmlState {
         XmlState {
@@ -126,8 +108,6 @@ impl XmlState {
         let new_ctx = match ctx.kind {
             ContextKind::Start => self.element_creation_start_element(handle, name, pbag),
             ContextKind::ElementCreation => self.element_creation_start_element(handle, name, pbag),
-            ContextKind::Style(_) => self.inside_style_start_element(name),
-            ContextKind::UnsupportedStyleChild => self.inside_style_start_element(name),
             ContextKind::XInclude(ref ctx) => self.inside_xinclude_start_element(ctx, name),
             ContextKind::UnsupportedXIncludeChild => self.unsupported_xinclude_start_element(name),
             ContextKind::XIncludeFallback(ref ctx) => {
@@ -150,8 +130,6 @@ impl XmlState {
         match context.kind {
             ContextKind::Start => panic!("end_element: XML handler stack is empty!?"),
             ContextKind::ElementCreation => self.element_creation_end_element(handle),
-            ContextKind::Style(style_ctx) => self.style_end_element(style_ctx, handle),
-            ContextKind::UnsupportedStyleChild => (),
             ContextKind::XInclude(_) => (),
             ContextKind::UnsupportedXIncludeChild => (),
             ContextKind::XIncludeFallback(_) => (),
@@ -159,13 +137,11 @@ impl XmlState {
     }
 
     pub fn characters(&mut self, text: &str) {
-        let mut ctx = mem::replace(&mut self.context, Context::empty());
+        let ctx = mem::replace(&mut self.context, Context::empty());
 
         match ctx.kind {
             ContextKind::Start => panic!("characters: XML handler stack is empty!?"),
             ContextKind::ElementCreation => self.element_creation_characters(text),
-            ContextKind::Style(ref mut style_ctx) => style_characters(style_ctx, text),
-            ContextKind::UnsupportedStyleChild => (),
             ContextKind::XInclude(_) => (),
             ContextKind::UnsupportedXIncludeChild => (),
             ContextKind::XIncludeFallback(ref ctx) => {
@@ -184,7 +160,6 @@ impl XmlState {
     ) -> Context {
         match name {
             "include" => self.xinclude_start_element(handle, name, pbag),
-            "style" => self.style_start_element(name, pbag),
             _ => {
                 let node = self.create_node(self.current_node.as_ref(), handle, name, pbag);
                 if self.current_node.is_none() {
@@ -211,6 +186,12 @@ impl XmlState {
             });
         }
 
+        if node.get_type() == NodeType::Style {
+            let css_data = node.with_impl(|style: &NodeStyle| style.get_css(&node));
+
+            css::parse_into_handle(handle, &css_data);
+        }
+
         self.current_node = node.get_parent();
     }
 
@@ -267,52 +248,6 @@ impl XmlState {
         new_node
     }
 
-    fn style_start_element(&self, name: &str, pbag: &PropertyBag) -> Context {
-        // FIXME: See these:
-        //
-        // https://www.w3.org/TR/SVG11/styling.html#StyleElementTypeAttribute
-        // https://www.w3.org/TR/SVG11/styling.html#ContentStyleTypeAttribute
-        //
-        // If the "type" attribute is not present, we should fallback to the
-        // "contentStyleType" attribute of the svg element, which in turn
-        // defaults to "text/css".
-        //
-        // See where is_text_css is used to see where we parse the contents
-        // of the style element.
-
-        let mut is_text_css = true;
-
-        for (_key, attr, value) in pbag.iter() {
-            if attr == Attribute::Type {
-                is_text_css = value == "text/css";
-            }
-        }
-
-        Context {
-            element_name: name.to_string(),
-            kind: ContextKind::Style(StyleContext {
-                is_text_css,
-                text: String::new(),
-            }),
-        }
-    }
-
-    fn style_end_element(&mut self, style_ctx: StyleContext, handle: *mut RsvgHandle) {
-        if style_ctx.is_text_css {
-            css::parse_into_handle(handle, &style_ctx.text);
-        }
-    }
-
-    fn inside_style_start_element(&self, name: &str) -> Context {
-        // We are already inside a <style> element, and we don't support
-        // elements in there.  Just push a state that we will ignore.
-
-        Context {
-            element_name: name.to_string(),
-            kind: ContextKind::UnsupportedStyleChild,
-        }
-    }
-
     fn xinclude_start_element(
         &mut self,
         handle: *mut RsvgHandle,