[librsvg] rsvg_handle_read_stream_sync(): Port the implementation to Rust

[Federico Mena Quintero <federico src gnome org>]

commit 9cb6c084653c232c63cfc17d4b95335f29a0b1c1
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Dec 17 19:44:28 2018 -0600

    rsvg_handle_read_stream_sync(): Port the implementation to Rust
    
    This involves expanding LoadingError to deal with the XML parsing
    errors.  This is not the cleanest code right now, but will probably
    refactor into something nicer.

 librsvg/rsvg-handle.c           | 40 +++++-----------------
 rsvg_internals/src/error.rs     | 14 +++++++-
 rsvg_internals/src/handle.rs    | 74 ++++++++++++++++++++++++++++++++++++++++-
 rsvg_internals/src/lib.rs       |  2 --
 rsvg_internals/src/load.rs      |  2 +-
 rsvg_internals/src/xml.rs       | 29 ++++++++++------
 rsvg_internals/src/xml2_load.rs | 59 ++++++++------------------------
 7 files changed, 127 insertions(+), 93 deletions(-)
---
diff --git a/librsvg/rsvg-handle.c b/librsvg/rsvg-handle.c
index 8ba6dd6a..2564f407 100644
--- a/librsvg/rsvg-handle.c
+++ b/librsvg/rsvg-handle.c
@@ -148,13 +148,6 @@ typedef enum {
 extern RsvgXmlState *rsvg_xml_state_new (RsvgHandle *handle);
 extern void rsvg_xml_state_error(RsvgXmlState *xml, const char *msg);
 
-/* Implemented in rsvg_internals/src/xml2_load.rs */
-extern gboolean rsvg_xml_state_load_from_possibly_compressed_stream (RsvgXmlState *xml,
-                                                                     guint         flags,
-                                                                     GInputStream *stream,
-                                                                     GCancellable *cancellable,
-                                                                     GError      **error);
-
 G_GNUC_INTERNAL
 RsvgHandleRust *rsvg_handle_get_rust (RsvgHandle *handle);
 
@@ -176,6 +169,10 @@ extern guint rsvg_handle_rust_get_flags (RsvgHandleRust *raw_handle);
 extern void rsvg_handle_rust_set_flags (RsvgHandleRust *raw_handle, guint flags);
 extern void rsvg_handle_rust_set_load_state (RsvgHandleRust *raw_handle, RsvgHandleState state);
 extern RsvgHandleState rsvg_handle_rust_get_load_state (RsvgHandleRust *raw_handle);
+extern gboolean rsvg_handle_rust_read_stream_sync (RsvgHandle *handle,
+                                                   GInputStream *stream,
+                                                   GCancellable *cancellable,
+                                                   GError **error);
 
 /* Implemented in rsvg_internals/src/xml.rs */
 extern void rsvg_xml_state_free (RsvgXmlState *xml);
@@ -859,36 +856,15 @@ rsvg_handle_read_stream_sync (RsvgHandle   *handle,
                               GCancellable *cancellable,
                               GError      **error)
 {
-    RsvgHandlePrivate *priv;
-    gboolean read_successfully;
-    gboolean result;
-    RsvgXmlState *xml;
-
     g_return_val_if_fail (RSVG_IS_HANDLE (handle), FALSE);
     g_return_val_if_fail (G_IS_INPUT_STREAM (stream), FALSE);
     g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
     g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
 
-    priv = handle->priv;
-
-    g_return_val_if_fail (rsvg_handle_rust_get_load_state (priv->rust_handle) == RSVG_HANDLE_STATE_START,
-                          FALSE);
-
-    rsvg_handle_rust_set_load_state (priv->rust_handle, RSVG_HANDLE_STATE_LOADING);
-
-    xml = rsvg_xml_state_new (handle);
-    read_successfully = rsvg_xml_state_load_from_possibly_compressed_stream (
-        xml,
-        rsvg_handle_rust_get_flags (priv->rust_handle),
-        stream,
-        cancellable,
-        error
-    );
-
-    result = finish_load (handle, xml, read_successfully, error);
-    rsvg_xml_state_free (xml);
-
-    return result;
+    return rsvg_handle_rust_read_stream_sync (handle,
+                                              stream,
+                                              cancellable,
+                                              error);
 }
 
 /* http://www.ietf.org/rfc/rfc2396.txt */
diff --git a/rsvg_internals/src/error.rs b/rsvg_internals/src/error.rs
index e73848af..72770b6e 100644
--- a/rsvg_internals/src/error.rs
+++ b/rsvg_internals/src/error.rs
@@ -140,9 +140,13 @@ impl<O, E: Into<ValueErrorKind>> AttributeResultExt<O, E> for Result<O, E> {
 #[derive(Debug, Clone)]
 pub enum LoadingError {
     // Could not parse data: URL
+    CouldNotCreateXmlParser,
+    XmlParseError(String),
     BadDataUrl,
     Cairo(cairo::Status),
     EmptyData,
+    SvgHasNoElements,
+    RootElementIsNotSvg,
     Glib(glib::Error),
     Unknown,
 }
@@ -150,9 +154,13 @@ pub enum LoadingError {
 impl error::Error for LoadingError {
     fn description(&self) -> &str {
         match *self {
+            LoadingError::CouldNotCreateXmlParser => "could not create XML parser",
+            LoadingError::XmlParseError(_) => "XML parse error",
             LoadingError::BadDataUrl => "invalid data: URL",
             LoadingError::Cairo(_) => "cairo error",
             LoadingError::EmptyData => "empty data",
+            LoadingError::SvgHasNoElements => "SVG has no elements",
+            LoadingError::RootElementIsNotSvg => "root element is not <svg>",
             LoadingError::Glib(ref e) => e.description(),
             LoadingError::Unknown => "unknown error",
         }
@@ -163,8 +171,12 @@ impl fmt::Display for LoadingError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
             LoadingError::Cairo(status) => write!(f, "cairo error: {:?}", status),
-            LoadingError::BadDataUrl
+            LoadingError::XmlParseError(ref s) => write!(f, "XML parse error: {}", s),
+            LoadingError::CouldNotCreateXmlParser
+            | LoadingError::BadDataUrl
             | LoadingError::EmptyData
+            | LoadingError::SvgHasNoElements
+            | LoadingError::RootElementIsNotSvg
             | LoadingError::Glib(_)
             | LoadingError::Unknown => write!(f, "{}", self.description()),
         }
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index c6af775a..4d52d503 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -24,6 +24,7 @@ use surface_utils::shared_surface::SharedImageSurface;
 use svg::Svg;
 use util::rsvg_g_warning;
 use xml::XmlState;
+use xml2_load::xml_state_load_from_possibly_compressed_stream;
 
 // A *const RsvgHandle is just an opaque pointer we get from C
 #[repr(C)]
@@ -45,7 +46,7 @@ pub struct LoadOptions {
 }
 
 #[repr(C)]
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, PartialEq)]
 pub enum LoadState {
     Start,
     Loading,
@@ -71,6 +72,48 @@ impl Handle {
             load_state: Cell::new(LoadState::Start),
         }
     }
+
+    pub fn read_stream_sync(
+        &mut self,
+        handle: *mut RsvgHandle,
+        stream: gio::InputStream,
+        cancellable: Option<gio::Cancellable>,
+    ) -> Result<(), LoadingError> {
+        self.load_state.set(LoadState::Loading);
+
+        self.read_stream_internal(handle, stream, cancellable)
+            .and_then(|_| {
+                self.load_state.set(LoadState::ClosedOk);
+                Ok(())
+            })
+            .map_err(|e| {
+                self.load_state.set(LoadState::ClosedError);
+                e
+            })
+    }
+
+    fn read_stream_internal(
+        &mut self,
+        handle: *mut RsvgHandle,
+        stream: gio::InputStream,
+        cancellable: Option<gio::Cancellable>,
+    ) -> Result<(), LoadingError> {
+        let load_options = self.load_options.get();
+
+        let mut xml = XmlState::new(handle);
+
+        xml_state_load_from_possibly_compressed_stream(
+            &mut xml,
+            &load_options,
+            stream,
+            cancellable,
+        )?;
+
+        xml.validate_tree()?;
+
+        *self.svg.borrow_mut() = Some(xml.steal_result());
+        Ok(())
+    }
 }
 
 // Keep these in sync with rsvg.h:RsvgHandleFlags
@@ -598,3 +641,32 @@ pub unsafe extern "C" fn rsvg_handle_rust_set_load_state(
 
     rhandle.load_state.set(load_state)
 }
+
+#[no_mangle]
+pub unsafe extern "C" fn rsvg_handle_rust_read_stream_sync(
+    handle: *mut RsvgHandle,
+    stream: *mut gio_sys::GInputStream,
+    cancellable: *mut gio_sys::GCancellable,
+    error: *mut *mut glib_sys::GError,
+) -> glib_sys::gboolean {
+    let rhandle = get_rust_handle(handle);
+
+    if rhandle.load_state.get() != LoadState::Start {
+        rsvg_g_warning(
+            "handle must not be already loaded in order to call rsvg_handle_read_stream_sync()",
+        );
+        return false.to_glib();
+    }
+
+    let stream = from_glib_none(stream);
+    let cancellable = from_glib_none(cancellable);
+
+    match rhandle.read_stream_sync(handle, stream, cancellable) {
+        Ok(()) => true.to_glib(),
+
+        Err(e) => {
+            set_gerror(error, 0, &format!("{}", e));
+            false.to_glib()
+        }
+    }
+}
diff --git a/rsvg_internals/src/lib.rs b/rsvg_internals/src/lib.rs
index f29483b9..7e7630ed 100644
--- a/rsvg_internals/src/lib.rs
+++ b/rsvg_internals/src/lib.rs
@@ -78,8 +78,6 @@ pub use xml::{
     rsvg_xml_state_tree_is_valid,
 };
 
-pub use xml2_load::rsvg_xml_state_load_from_possibly_compressed_stream;
-
 #[macro_use]
 mod log;
 
diff --git a/rsvg_internals/src/load.rs b/rsvg_internals/src/load.rs
index a112b02b..cc62be0a 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -138,7 +138,7 @@ pub unsafe extern "C" fn rsvg_load_close(
 
         Err(e) => {
             match e {
-                ParseFromStreamError::CouldNotCreateParser => {
+                ParseFromStreamError::CouldNotCreateXmlParser => {
                     set_gerror(error, 0, "Error creating XML parser");
                 }
 
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index af4e5133..b6a41a8f 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -118,7 +118,7 @@ enum AcquireError {
 }
 
 impl XmlState {
-    fn new(handle: *mut RsvgHandle) -> XmlState {
+    pub fn new(handle: *mut RsvgHandle) -> XmlState {
         XmlState {
             tree: None,
             defs: Some(Defs::new()),
@@ -139,6 +139,18 @@ impl XmlState {
         self.tree = Some(Tree::new(root));
     }
 
+    pub fn validate_tree(&self) -> Result<(), LoadingError> {
+        if let Some(ref tree) = self.tree {
+            if tree.root_is_svg() {
+                Ok(())
+            } else {
+                Err(LoadingError::RootElementIsNotSvg)
+            }
+        } else {
+            Err(LoadingError::SvgHasNoElements)
+        }
+    }
+
     pub fn steal_result(&mut self) -> Svg {
         Svg::new(
             self.tree.take().unwrap(),
@@ -520,7 +532,7 @@ impl XmlState {
 
         // FIXME: pass a cancellable
         xml_state_parse_from_stream(self, &load_options, stream, None).map_err(|e| match e {
-            ParseFromStreamError::CouldNotCreateParser => AcquireError::FatalError,
+            ParseFromStreamError::CouldNotCreateXmlParser => AcquireError::FatalError,
             ParseFromStreamError::IoError(_) => AcquireError::ResourceError,
             ParseFromStreamError::XmlParseError(_) => AcquireError::FatalError,
         })
@@ -617,16 +629,13 @@ pub unsafe extern "C" fn rsvg_xml_state_tree_is_valid(
     assert!(!xml.is_null());
     let xml = &mut *xml;
 
-    if let Some(ref tree) = xml.tree {
-        if tree.root_is_svg() {
-            true.to_glib()
-        } else {
-            set_gerror(error, 0, "root element is not <svg>");
+    match xml.validate_tree() {
+        Ok(()) => true.to_glib(),
+
+        Err(e) => {
+            set_gerror(error, 0, &format!("{}", e));
             false.to_glib()
         }
-    } else {
-        set_gerror(error, 0, "SVG has no elements");
-        false.to_glib()
     }
 }
 
diff --git a/rsvg_internals/src/xml2_load.rs b/rsvg_internals/src/xml2_load.rs
index cfb59df1..60d44bb7 100644
--- a/rsvg_internals/src/xml2_load.rs
+++ b/rsvg_internals/src/xml2_load.rs
@@ -3,8 +3,6 @@
 
 use gio;
 use gio::prelude::*;
-use gio_sys;
-use glib_sys;
 use std::cell::RefCell;
 use std::mem;
 use std::ptr;
@@ -14,7 +12,7 @@ use std::str;
 
 use glib::translate::*;
 
-use error::set_gerror;
+use error::LoadingError;
 use handle::LoadOptions;
 use io::get_input_stream_for_loading;
 use property_bag::PropertyBag;
@@ -321,7 +319,7 @@ impl Xml2Parser {
             if parser.is_null() {
                 // on error, xmlCreateIOParserCtxt() frees our ctx via the
                 // stream_ctx_close function
-                Err(ParseFromStreamError::CouldNotCreateParser)
+                Err(ParseFromStreamError::CouldNotCreateXmlParser)
             } else {
                 set_xml_parse_options(parser, load_options);
                 Ok(Xml2Parser { parser, gio_error })
@@ -388,7 +386,7 @@ fn xml2_error_to_string(xerr: xmlErrorPtr) -> String {
 // Error returned when parsing an XML stream
 pub enum ParseFromStreamError {
     // We couldn't even create the libxml2 parser
-    CouldNotCreateParser,
+    CouldNotCreateXmlParser,
 
     // GIO error from the I/O callbacks
     IoError(glib::Error),
@@ -397,6 +395,16 @@ pub enum ParseFromStreamError {
     XmlParseError(String),
 }
 
+impl From<ParseFromStreamError> for LoadingError {
+    fn from(e: ParseFromStreamError) -> LoadingError {
+        match e {
+            ParseFromStreamError::CouldNotCreateXmlParser => LoadingError::CouldNotCreateXmlParser,
+            ParseFromStreamError::IoError(e) => LoadingError::Glib(e),
+            ParseFromStreamError::XmlParseError(s) => LoadingError::XmlParseError(s),
+        }
+    }
+}
+
 // Parses XML from a stream into an XmlState.
 //
 // This can be called "in the middle" of an XmlState's processing status,
@@ -422,44 +430,3 @@ pub fn xml_state_load_from_possibly_compressed_stream(
 
     xml_state_parse_from_stream(xml, load_options, stream, cancellable.as_ref())
 }
-
-#[no_mangle]
-pub unsafe extern "C" fn rsvg_xml_state_load_from_possibly_compressed_stream(
-    xml: *mut XmlState,
-    flags: u32,
-    stream: *mut gio_sys::GInputStream,
-    cancellable: *mut gio_sys::GCancellable,
-    error: *mut *mut glib_sys::GError,
-) -> glib_sys::gboolean {
-    assert!(!xml.is_null());
-    let xml = &mut *xml;
-
-    let load_options = LoadOptions::from_flags(flags);
-
-    let stream = from_glib_none(stream);
-    let cancellable = from_glib_none(cancellable);
-
-    match xml_state_load_from_possibly_compressed_stream(xml, &load_options, stream, cancellable) {
-        Ok(()) => true.to_glib(),
-
-        Err(e) => {
-            match e {
-                ParseFromStreamError::CouldNotCreateParser => {
-                    set_gerror(error, 0, "Error creating XML parser");
-                }
-
-                ParseFromStreamError::IoError(e) => {
-                    if !error.is_null() {
-                        *error = e.to_glib_full() as *mut _;
-                    }
-                }
-
-                ParseFromStreamError::XmlParseError(s) => {
-                    set_gerror(error, 0, &s);
-                }
-            }
-
-            false.to_glib()
-        }
-    }
-}