[librsvg: 5/16] Propagate AcquireError::MaxReferencesExceeded quickly upstream

[Federico Mena Quintero <federico src gnome org>]

commit 7fb5175d7dd15776dbef96214b41a478776cfae5
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Oct 10 19:02:25 2019 -0500

    Propagate AcquireError::MaxReferencesExceeded quickly upstream
    
    Don't let the paint server machinery in DrawingCtx just ignore it
    to follow the "no paint server" code path.
    
    This also adds an rsvg_log!() to the legacy C API, since it is not
    able to return rendering errors.

 librsvg/c_api.rs                   | 16 ++++++++------
 rsvg_internals/src/drawing_ctx.rs  | 42 +++++++++++++++++++++---------------
 rsvg_internals/src/paint_server.rs |  5 +++++
 rsvg_internals/src/pattern.rs      | 44 ++++++++++++++++++++++++++++----------
 rsvg_internals/src/structure.rs    | 34 +++++++++++++++++++++--------
 5 files changed, 98 insertions(+), 43 deletions(-)
---
diff --git a/librsvg/c_api.rs b/librsvg/c_api.rs
index 51831e52..ba1543b8 100644
--- a/librsvg/c_api.rs
+++ b/librsvg/c_api.rs
@@ -986,8 +986,8 @@ pub unsafe extern "C" fn rsvg_rust_handle_render_cairo_sub(
     match rhandle.render_cairo_sub(&cr, id.as_ref().map(String::as_str)) {
         Ok(()) => true.to_glib(),
 
-        Err(_) => {
-            // FIXME: return a proper error code to the public API
+        Err(e) => {
+            rsvg_log!("could not render: {}", e);
             false.to_glib()
         }
     }
@@ -1003,7 +1003,10 @@ pub unsafe extern "C" fn rsvg_rust_handle_get_pixbuf_sub(
 
     match rhandle.get_pixbuf_sub(id.as_ref().map(String::as_str)) {
         Ok(pixbuf) => pixbuf.to_glib_full(),
-        Err(_) => ptr::null_mut(),
+        Err(e) => {
+            rsvg_log!("could not render: {}", e);
+            ptr::null_mut()
+        }
     }
 }
 
@@ -1034,7 +1037,8 @@ pub unsafe extern "C" fn rsvg_rust_handle_get_dimensions_sub(
             true.to_glib()
         }
 
-        Err(_) => {
+        Err(e) => {
+            rsvg_log!("could not get dimensions: {}", e);
             *dimension_data = RsvgDimensionData::empty();
             false.to_glib()
         }
@@ -1057,13 +1061,13 @@ pub unsafe extern "C" fn rsvg_rust_handle_get_position_sub(
             true.to_glib()
         }
 
-        Err(_) => {
+        Err(e) => {
             let p = &mut *position_data;
 
             p.x = 0;
             p.y = 0;
 
-            // FIXME: return a proper error code to the public API
+            rsvg_log!("could not get position: {}", e);
             false.to_glib()
         }
     }
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 51b44a77..732427fd 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -643,24 +643,32 @@ impl DrawingCtx {
             } => {
                 let mut had_paint_server = false;
 
-                if let Ok(acquired) = self.acquire_paint_server(iri) {
-                    let node = acquired.get();
-
-                    had_paint_server = match node.borrow().get_type() {
-                        NodeType::LinearGradient => node
-                            .borrow()
-                            .get_impl::<NodeLinearGradient>()
-                            .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
-                        NodeType::RadialGradient => node
-                            .borrow()
-                            .get_impl::<NodeRadialGradient>()
-                            .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
-                        NodeType::Pattern => node
-                            .borrow()
-                            .get_impl::<NodePattern>()
-                            .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
-                        _ => unreachable!(),
+                match self.acquire_paint_server(iri) {
+                    Ok(acquired) => {
+                        let node = acquired.get();
+
+                        had_paint_server = match node.borrow().get_type() {
+                            NodeType::LinearGradient => node
+                                .borrow()
+                                .get_impl::<NodeLinearGradient>()
+                                .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
+                            NodeType::RadialGradient => node
+                                .borrow()
+                                .get_impl::<NodeRadialGradient>()
+                                .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
+                            NodeType::Pattern => node
+                                .borrow()
+                                .get_impl::<NodePattern>()
+                                .resolve_fallbacks_and_set_pattern(&node, self, opacity, bbox)?,
+                            _ => unreachable!(),
+                        }
+                    }
+
+                    Err(AcquireError::MaxReferencesExceeded) => {
+                        return Err(RenderingError::InstancingLimit);
                     }
+
+                    Err(_) => (),
                 }
 
                 if !had_paint_server && alternate.is_some() {
diff --git a/rsvg_internals/src/paint_server.rs b/rsvg_internals/src/paint_server.rs
index 3e7b54d8..d7c104fa 100644
--- a/rsvg_internals/src/paint_server.rs
+++ b/rsvg_internals/src/paint_server.rs
@@ -83,6 +83,11 @@ pub trait PaintSource {
                 Err(RenderingError::CircularReference)
             }
 
+            Err(AcquireError::MaxReferencesExceeded) => {
+                rsvg_log!("maximum number of references exceeded");
+                Err(RenderingError::InstancingLimit)
+            }
+
             Err(e) => {
                 rsvg_log!("not using paint server {}: {}", node, e);
 
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index f823399b..182fed2f 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -8,7 +8,7 @@ use crate::aspect_ratio::*;
 use crate::bbox::*;
 use crate::coord_units::CoordUnits;
 use crate::drawing_ctx::{DrawingCtx, NodeStack};
-use crate::error::{AttributeResultExt, AcquireError, RenderingError};
+use crate::error::{AcquireError, AttributeResultExt, RenderingError};
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::length::*;
 use crate::node::*;
@@ -120,7 +120,9 @@ impl NodeTrait for NodePattern {
         for (attr, value) in pbag.iter() {
             match attr {
                 local_name!("patternUnits") => self.common.units = Some(attr.parse(value)?),
-                local_name!("patternContentUnits") => self.common.content_units = Some(attr.parse(value)?),
+                local_name!("patternContentUnits") => {
+                    self.common.content_units = Some(attr.parse(value)?)
+                }
                 local_name!("viewBox") => self.common.vbox = Some(Some(attr.parse(value)?)),
                 local_name!("preserveAspectRatio") => {
                     self.common.preserve_aspect_ratio = Some(attr.parse(value)?)
@@ -164,7 +166,10 @@ impl PaintSource for NodePattern {
             return Ok(pattern.clone());
         }
 
-        let Unresolved { mut pattern, mut fallback } = self.get_unresolved(node);
+        let Unresolved {
+            mut pattern,
+            mut fallback,
+        } = self.get_unresolved(node);
 
         let mut stack = NodeStack::new();
 
@@ -188,6 +193,10 @@ impl PaintSource for NodePattern {
                         stack.push(acquired_node);
                     }
 
+                    Err(AcquireError::MaxReferencesExceeded) => {
+                        return Err(AcquireError::MaxReferencesExceeded)
+                    }
+
                     Err(e) => {
                         rsvg_log!("Stopping pattern resolution: {}", e);
                         pattern = pattern.resolve_from_defaults();
@@ -372,9 +381,10 @@ impl AsPaintSource for Pattern {
 
         cr_pattern.set_matrix(caffine);
 
-        let res = draw_ctx.with_discrete_layer(&node_with_children, pattern_values, false, &mut |dc| {
-            node_with_children.draw_children(&pattern_cascaded, dc, false)
-        });
+        let res =
+            draw_ctx.with_discrete_layer(&node_with_children, pattern_values, false, &mut |dc| {
+                node_with_children.draw_children(&pattern_cascaded, dc, false)
+            });
 
         // Return to the original coordinate system and rendering context
 
@@ -433,7 +443,10 @@ impl UnresolvedPattern {
         let units = self.common.units.or(fallback.common.units);
         let content_units = self.common.content_units.or(fallback.common.content_units);
         let vbox = self.common.vbox.or(fallback.common.vbox);
-        let preserve_aspect_ratio = 
self.common.preserve_aspect_ratio.or(fallback.common.preserve_aspect_ratio);
+        let preserve_aspect_ratio = self
+            .common
+            .preserve_aspect_ratio
+            .or(fallback.common.preserve_aspect_ratio);
         let affine = self.common.affine.or(fallback.common.affine);
         let x = self.common.x.or(fallback.common.x);
         let y = self.common.y.or(fallback.common.y);
@@ -459,9 +472,15 @@ impl UnresolvedPattern {
 
     fn resolve_from_defaults(&self) -> UnresolvedPattern {
         let units = self.common.units.or(Some(PatternUnits::default()));
-        let content_units = self.common.content_units.or(Some(PatternContentUnits::default()));
+        let content_units = self
+            .common
+            .content_units
+            .or(Some(PatternContentUnits::default()));
         let vbox = self.common.vbox.or(Some(None));
-        let preserve_aspect_ratio = self.common.preserve_aspect_ratio.or(Some(AspectRatio::default()));
+        let preserve_aspect_ratio = self
+            .common
+            .preserve_aspect_ratio
+            .or(Some(AspectRatio::default()));
         let affine = self.common.affine.or(Some(cairo::Matrix::identity()));
         let x = self.common.x.or(Some(Default::default()));
         let y = self.common.y.or(Some(Default::default()));
@@ -490,7 +509,10 @@ impl UnresolvedChildren {
     fn from_node(node: &RsvgNode) -> UnresolvedChildren {
         let weak = node.downgrade();
 
-        if node.children().any(|child| child.borrow().get_type() != NodeType::Chars) {
+        if node
+            .children()
+            .any(|child| child.borrow().get_type() != NodeType::Chars)
+        {
             UnresolvedChildren::WithChildren(weak)
         } else {
             UnresolvedChildren::Unresolved
@@ -572,7 +594,7 @@ mod tests {
             local_name!("pattern"),
             None,
             None,
-            Box::new(NodePattern::default())
+            Box::new(NodePattern::default()),
         ));
 
         let borrow = node.borrow();
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index de27c194..2aea9d89 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -6,7 +6,7 @@ use crate::aspect_ratio::*;
 use crate::bbox::BoundingBox;
 use crate::dpi::Dpi;
 use crate::drawing_ctx::{ClipMode, DrawingCtx, ViewParams};
-use crate::error::{AttributeResultExt, RenderingError};
+use crate::error::{AcquireError, AttributeResultExt, RenderingError};
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::length::*;
 use crate::node::*;
@@ -315,14 +315,30 @@ impl NodeTrait for NodeUse {
 
         let link = self.link.as_ref().unwrap();
 
-        let child = if let Ok(acquired) = draw_ctx.acquire_node(link, &[]) {
-            // Here we clone the acquired child, so that we can drop the AcquiredNode as
-            // early as possible.  This is so that the child's drawing method will be able
-            // to re-acquire the child for other purposes.
-            acquired.get().clone()
-        } else {
-            rsvg_log!("element {} references nonexistent \"{}\"", node, link);
-            return Ok(draw_ctx.empty_bbox());
+        let child = match draw_ctx.acquire_node(link, &[]) {
+            Ok(acquired) => {
+                // Here we clone the acquired child, so that we can drop the AcquiredNode as
+                // early as possible.  This is so that the child's drawing method will be able
+                // to re-acquire the child for other purposes.
+                acquired.get().clone()
+            }
+
+            Err(AcquireError::CircularReference(_)) => {
+                // FIXME: add a fragment or node id to this:
+                rsvg_log!("circular reference in <use> element {}", node);
+                return Err(RenderingError::CircularReference);
+            }
+
+            Err(AcquireError::MaxReferencesExceeded) => {
+                return Err(RenderingError::InstancingLimit);
+            }
+
+            Err(AcquireError::InvalidLinkType(_)) => unreachable!(),
+
+            Err(AcquireError::LinkNotFound(fragment)) => {
+                rsvg_log!("element {} references nonexistent \"{}\"", node, fragment);
+                return Ok(draw_ctx.empty_bbox());
+            }
         };
 
         if node.ancestors().any(|ancestor| ancestor == child) {