[librsvg] Resources: Don't store raw RsvgHandles, store just Svg

[Federico Mena Quintero <federico src gnome org>]

commit 755c2a6fadca4ecc7788ba6440450838dfb03196
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Jan 14 16:08:38 2019 -0600

    Resources: Don't store raw RsvgHandles, store just Svg
    
    This removes the last use of RsvgHandle from the internals.

 rsvg_internals/src/handle.rs | 56 +++++++++++---------------------------------
 rsvg_internals/src/load.rs   |  2 +-
 rsvg_internals/src/svg.rs    | 46 +++++++++++++++++-------------------
 rsvg_internals/src/xml.rs    |  4 ++--
 4 files changed, 38 insertions(+), 70 deletions(-)
---
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index 4eec6f9a..14d12b6b 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -25,7 +25,7 @@ use drawing_ctx::{DrawingCtx, RsvgRectangle};
 use error::{set_gerror, DefsLookupErrorKind, LoadingError, RenderingError};
 use io;
 use load::LoadContext;
-use node::{Node, RsvgNode};
+use node::RsvgNode;
 use pixbuf_utils::pixbuf_from_surface;
 use structure::NodeSvg;
 use surface_utils::{shared_surface::SharedImageSurface, shared_surface::SurfaceType};
@@ -77,6 +77,13 @@ impl LoadOptions {
     fn new(flags: LoadFlags, base_url: Option<Url>) -> LoadOptions {
         LoadOptions { flags, base_url }
     }
+
+    pub fn copy_with_base_url(&self, base_url: &AllowedUrl) -> LoadOptions {
+        LoadOptions {
+            flags: self.flags,
+            base_url: Some(base_url.url().clone()),
+        }
+    }
 }
 
 #[derive(Copy, Clone, PartialEq)]
@@ -206,10 +213,11 @@ impl Handle {
     ) -> Result<(), LoadingError> {
         self.load_state.set(LoadState::Loading);
 
-        let svg = Svg::load_from_stream(self.load_options(), stream, cancellable).map_err(|e| {
-            self.load_state.set(LoadState::ClosedError);
-            e
-        })?;
+        let svg =
+            Svg::load_from_stream(&self.load_options(), stream, cancellable).map_err(|e| {
+                self.load_state.set(LoadState::ClosedError);
+                e
+            })?;
 
         *self.svg.borrow_mut() = Some(Rc::new(svg));
         self.load_state.set(LoadState::ClosedOk);
@@ -229,7 +237,7 @@ impl Handle {
         if self.load_state.get() == LoadState::Start {
             self.load_state.set(LoadState::Loading);
 
-            self.load = RefCell::new(Some(LoadContext::new(self.load_options())));
+            self.load = RefCell::new(Some(LoadContext::new(&self.load_options())));
         }
 
         assert!(self.load_state.get() == LoadState::Loading);
@@ -566,45 +574,9 @@ impl LoadFlags {
 extern "C" {
     fn rsvg_handle_new_with_flags(flags: u32) -> *mut RsvgHandle;
 
-    fn rsvg_handle_new_from_gfile_sync(
-        file: *const gio_sys::GFile,
-        flags: u32,
-        cancellable: *const gio_sys::GCancellable,
-        error: *mut *mut glib_sys::GError,
-    ) -> *mut RsvgHandle;
-
     fn rsvg_handle_get_rust(handle: *const RsvgHandle) -> *mut Handle;
 }
 
-// Looks up a node by its id.
-pub fn lookup_fragment_id(handle: *const RsvgHandle, id: &str) -> Option<Rc<Node>> {
-    let rhandle = get_rust_handle(handle);
-
-    let svg_ref = rhandle.svg.borrow();
-    let svg = svg_ref.as_ref().unwrap();
-
-    svg.lookup_node_by_id(id)
-}
-
-pub fn load_extern(load_options: &LoadOptions, aurl: &AllowedUrl) -> Result<*mut RsvgHandle, ()> {
-    unsafe {
-        let file = gio::File::new_for_uri(aurl.url().as_str());
-
-        let res = rsvg_handle_new_from_gfile_sync(
-            file.to_glib_none().0,
-            load_options.flags.to_flags(),
-            ptr::null(),
-            ptr::null_mut(),
-        );
-
-        if res.is_null() {
-            Err(())
-        } else {
-            Ok(res)
-        }
-    }
-}
-
 pub fn load_image_to_surface(
     load_options: &LoadOptions,
     url: &str,
diff --git a/rsvg_internals/src/load.rs b/rsvg_internals/src/load.rs
index 94f30f35..ffef85ae 100644
--- a/rsvg_internals/src/load.rs
+++ b/rsvg_internals/src/load.rs
@@ -32,7 +32,7 @@ enum LoadState {
 }
 
 impl LoadContext {
-    pub fn new(load_options: LoadOptions) -> LoadContext {
+    pub fn new(load_options: &LoadOptions) -> LoadContext {
         LoadContext {
             load_flags: load_options.flags,
             state: LoadState::Start,
diff --git a/rsvg_internals/src/svg.rs b/rsvg_internals/src/svg.rs
index d55adefa..2cc4c6d4 100644
--- a/rsvg_internals/src/svg.rs
+++ b/rsvg_internals/src/svg.rs
@@ -1,13 +1,13 @@
+use gio;
 use std::cell::RefCell;
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
-
-use gio;
-use gobject_sys;
+use std::rc::Rc;
 
 use allowed_url::{AllowedUrl, Fragment};
 use error::LoadingError;
-use handle::{self, LoadOptions, RsvgHandle};
+use handle::LoadOptions;
+use io;
 use node::RsvgNode;
 use state::ComputedValues;
 use xml::XmlState;
@@ -45,7 +45,7 @@ impl Svg {
     }
 
     pub fn load_from_stream(
-        load_options: LoadOptions,
+        load_options: &LoadOptions,
         stream: &gio::InputStream,
         cancellable: Option<&gio::Cancellable>,
     ) -> Result<Svg, LoadingError> {
@@ -77,7 +77,7 @@ impl Svg {
 }
 
 struct Resources {
-    resources: HashMap<AllowedUrl, *mut RsvgHandle>,
+    resources: HashMap<AllowedUrl, Rc<Svg>>,
 }
 
 impl Resources {
@@ -91,8 +91,10 @@ impl Resources {
     /// externally-loaded SVG file.
     pub fn lookup(&mut self, load_options: &LoadOptions, fragment: &Fragment) -> Option<RsvgNode> {
         if let Some(ref href) = fragment.uri() {
-            match self.get_extern_handle(load_options, href) {
-                Ok(extern_handle) => handle::lookup_fragment_id(extern_handle, fragment.fragment()),
+            // FIXME: propagate errors from the loader
+            match self.get_extern_svg(load_options, href) {
+                Ok(svg) => svg.lookup_node_by_id(fragment.fragment()),
+
                 Err(()) => None,
             }
         } else {
@@ -100,30 +102,24 @@ impl Resources {
         }
     }
 
-    fn get_extern_handle(
-        &mut self,
-        load_options: &LoadOptions,
-        href: &str,
-    ) -> Result<*const RsvgHandle, ()> {
+    fn get_extern_svg(&mut self, load_options: &LoadOptions, href: &str) -> Result<Rc<Svg>, ()> {
         let aurl = AllowedUrl::from_href(href, load_options.base_url.as_ref()).map_err(|_| ())?;
 
         match self.resources.entry(aurl) {
-            Entry::Occupied(e) => Ok(*(e.get())),
+            Entry::Occupied(e) => Ok(e.get().clone()),
             Entry::Vacant(e) => {
-                let extern_handle = handle::load_extern(load_options, e.key())?;
-                e.insert(extern_handle);
-                Ok(extern_handle)
+                // FIXME: propagate errors
+                let svg = load_svg(load_options, e.key()).map_err(|_| ())?;
+                let rc_svg = e.insert(Rc::new(svg));
+                Ok(rc_svg.clone())
             }
         }
     }
 }
 
-impl Drop for Resources {
-    fn drop(&mut self) {
-        for (_, handle) in self.resources.iter() {
-            unsafe {
-                gobject_sys::g_object_unref(*handle as *mut _);
-            }
-        }
-    }
+fn load_svg(load_options: &LoadOptions, aurl: &AllowedUrl) -> Result<Svg, LoadingError> {
+    // FIXME: pass a cancellable to these
+    io::acquire_stream(aurl, None).and_then(|stream| {
+        Svg::load_from_stream(&load_options.copy_with_base_url(aurl), &stream, None)
+    })
 }
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 3d6dd20f..60408564 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -92,7 +92,7 @@ enum AcquireError {
 }
 
 impl XmlState {
-    pub fn new(load_options: LoadOptions) -> XmlState {
+    pub fn new(load_options: &LoadOptions) -> XmlState {
         XmlState {
             tree_root: None,
             ids: Some(HashMap::new()),
@@ -100,7 +100,7 @@ impl XmlState {
             context_stack: vec![Context::Start],
             current_node: None,
             entities: HashMap::new(),
-            load_options,
+            load_options: load_options.clone(),
         }
     }