[librsvg: 5/23] Store LanguageTags inside SystemLanguage and really evaluate them at rendering time

[Federico Mena Quintero <federico src gnome org>]

commit 56f3218a738f568f59705bcb41098fa78ca2d294
Author: Federico Mena Quintero <federico gnome org>
Date:   Tue May 18 18:44:40 2021 -0500

    Store LanguageTags inside SystemLanguage and really evaluate them at rendering time
    
    Instead of storing the result of evaluating the systemLanguage
    attribute at loading time, SystemLanguage is now a newtype around a
    set of LanguageTags.  It is matched against the environment's language
    tags at rendering time.
    
    And at rendering time, it is now DrawingCtx which generates the list
    of the user's language tags from the environment.
    
    I remove the Lazy<Locale> - the tests started failing with the
    shuffling above.  The result of g_get_language_names() will only be
    converted once per render, anyway, because now it is done in DrawingCtx.

 src/cond.rs        | 87 +++++++++++++++++++++++++++++++++---------------------
 src/drawing_ctx.rs | 37 +++++++++++++++++++++++
 src/element.rs     | 36 ++++------------------
 src/structure.rs   |  2 +-
 4 files changed, 98 insertions(+), 64 deletions(-)
---
diff --git a/src/cond.rs b/src/cond.rs
index d7c9b88d..f159ca23 100644
--- a/src/cond.rs
+++ b/src/cond.rs
@@ -80,9 +80,14 @@ impl RequiredFeatures {
 /// A list of BCP47 language tags.
 ///
 /// https://www.rfc-editor.org/info/rfc5664
+#[derive(Debug, Clone, PartialEq)]
 pub struct LanguageTags(Vec<LanguageTag>);
 
 impl LanguageTags {
+    pub fn empty() -> Self {
+        LanguageTags(Vec::new())
+    }
+
     /// Converts a `Locale` to a set of language tags.
     pub fn from_locale(locale: &Locale) -> Result<LanguageTags, String> {
         let mut tags = Vec::new();
@@ -120,7 +125,7 @@ impl LanguageTags {
 }
 
 #[derive(Debug, PartialEq)]
-pub struct SystemLanguage(pub bool);
+pub struct SystemLanguage(LanguageTags);
 
 impl SystemLanguage {
     /// Parse a `systemLanguage` attribute and match it against a given `Locale`
@@ -138,25 +143,23 @@ impl SystemLanguage {
     ///
     /// [`systemLanguage`]: 
https://www.w3.org/TR/SVG/struct.html#ConditionalProcessingSystemLanguageAttribute
     /// [BCP47]: http://www.ietf.org/rfc/bcp/bcp47.txt
-    pub fn from_attribute(s: &str, locale: &Locale) -> Result<SystemLanguage, ValueErrorKind> {
-        let locale_tags =
-            LanguageTags::from_locale(locale).map_err(|e| ValueErrorKind::value_error(&e))?;
-
-        let attribute_tags = s.split(',')
+    pub fn from_attribute(s: &str) -> Result<SystemLanguage, ValueErrorKind> {
+        let attribute_tags = s
+            .split(',')
             .map(str::trim)
-            .map(|s| LanguageTag::from_str(s).map_err(|e| ValueErrorKind::parse_error(&format!(
-                "invalid language tag: \"{}\"",
-                e
-            ))))
+            .map(|s| {
+                LanguageTag::from_str(s).map_err(|e| {
+                    ValueErrorKind::parse_error(&format!("invalid language tag: \"{}\"", e))
+                })
+            })
             .collect::<Result<Vec<LanguageTag>, _>>()?;
 
-        let matches = attribute_tags.iter().any(|tag| locale_tags.any_matches(tag));
-        Ok(SystemLanguage(matches))
+        Ok(SystemLanguage(LanguageTags(attribute_tags)))
     }
 
     /// Evaluate a systemLanguage value for conditional processing.
-    pub fn eval(&self) -> bool {
-        self.0
+    pub fn eval(&self, locale_tags: &LanguageTags) -> bool {
+        (self.0).0.iter().any(|tag| locale_tags.any_matches(tag))
     }
 }
 
@@ -207,50 +210,68 @@ mod tests {
 
     #[test]
     fn system_language() {
-        let user_prefers = Locale::new("de,en-US").unwrap();
+        let locale = Locale::new("de,en-US").unwrap();
+
+        let locale_tags = LanguageTags::from_locale(&locale).unwrap();
 
-        assert!(SystemLanguage::from_attribute("", &user_prefers).is_err());
+        assert!(SystemLanguage::from_attribute("").is_err());
 
-        assert!(SystemLanguage::from_attribute("12345", &user_prefers).is_err());
+        assert!(SystemLanguage::from_attribute("12345").is_err());
 
         assert_eq!(
-            SystemLanguage::from_attribute("fr", &user_prefers).unwrap(),
-            SystemLanguage(false)
+            SystemLanguage::from_attribute("fr")
+                .unwrap()
+                .eval(&locale_tags),
+            false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en", &user_prefers).unwrap(),
-            SystemLanguage(false)
+            SystemLanguage::from_attribute("en")
+                .unwrap()
+                .eval(&locale_tags),
+            false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("de", &user_prefers).unwrap(),
-            SystemLanguage(true)
+            SystemLanguage::from_attribute("de")
+                .unwrap()
+                .eval(&locale_tags),
+            true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en-US", &user_prefers).unwrap(),
-            SystemLanguage(true)
+            SystemLanguage::from_attribute("en-US")
+                .unwrap()
+                .eval(&locale_tags),
+            true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("en-GB", &user_prefers).unwrap(),
-            SystemLanguage(false)
+            SystemLanguage::from_attribute("en-GB")
+                .unwrap()
+                .eval(&locale_tags),
+            false
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("DE", &user_prefers).unwrap(),
-            SystemLanguage(true)
+            SystemLanguage::from_attribute("DE")
+                .unwrap()
+                .eval(&locale_tags),
+            true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("de-LU", &user_prefers).unwrap(),
-            SystemLanguage(true)
+            SystemLanguage::from_attribute("de-LU")
+                .unwrap()
+                .eval(&locale_tags),
+            true
         );
 
         assert_eq!(
-            SystemLanguage::from_attribute("fr, de", &user_prefers).unwrap(),
-            SystemLanguage(true)
+            SystemLanguage::from_attribute("fr, de")
+                .unwrap()
+                .eval(&locale_tags),
+            true
         );
     }
 }
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index 1e039d25..c041a7d5 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -1,6 +1,7 @@
 //! The main context structure which drives the drawing process.
 
 use float_cmp::approx_eq;
+use locale_config::{LanguageRange, Locale};
 use once_cell::sync::Lazy;
 use pango::FontMapExt;
 use regex::{Captures, Regex};
@@ -11,6 +12,7 @@ use std::rc::{Rc, Weak};
 
 use crate::aspect_ratio::AspectRatio;
 use crate::bbox::BoundingBox;
+use crate::cond::LanguageTags;
 use crate::coord_units::CoordUnits;
 use crate::dasharray::Dasharray;
 use crate::document::{AcquiredNodes, NodeId};
@@ -149,6 +151,8 @@ pub struct DrawingCtx {
     cr_stack: Rc<RefCell<Vec<cairo::Context>>>,
     cr: cairo::Context,
 
+    locale_tags: LanguageTags,
+
     viewport_stack: Rc<RefCell<Vec<Viewport>>>,
 
     drawsub_stack: Vec<Node>,
@@ -249,6 +253,27 @@ impl Drop for DrawingCtx {
     }
 }
 
+/// Gets the user's preferred locale from the environment and
+/// translates it to a `Locale` with `LanguageRange` fallbacks.
+///
+/// The `Locale::current()` call only contemplates a single language,
+/// but glib is smarter, and `g_get_langauge_names()` can provide
+/// fallbacks, for example, when LC_MESSAGES="en_US.UTF-8:de" (USA
+/// English and German).  This function converts the output of
+/// `g_get_language_names()` into a `Locale` with appropriate
+/// fallbacks.
+fn locale_from_environment() -> Locale {
+    let mut locale = Locale::invariant();
+
+    for name in glib::get_language_names() {
+        if let Ok(range) = LanguageRange::from_unix(&name) {
+            locale.add(&range);
+        }
+    }
+
+    locale
+}
+
 impl DrawingCtx {
     fn new(
         cr: &cairo::Context,
@@ -264,11 +289,18 @@ impl DrawingCtx {
 
         let viewport_stack = vec![initial_viewport];
 
+        let locale_tags = LanguageTags::from_locale(&locale_from_environment())
+            .map_err(|s| {
+                rsvg_log!("could not convert locale to language tags: {}", s);
+            })
+            .unwrap_or_else(|_| LanguageTags::empty());
+
         DrawingCtx {
             initial_viewport,
             dpi,
             cr_stack: Rc::new(RefCell::new(Vec::new())),
             cr: cr.clone(),
+            locale_tags,
             viewport_stack: Rc::new(RefCell::new(viewport_stack)),
             drawsub_stack,
             measuring,
@@ -292,6 +324,7 @@ impl DrawingCtx {
             dpi: self.dpi,
             cr_stack,
             cr,
+            locale_tags: self.locale_tags.clone(),
             viewport_stack: self.viewport_stack.clone(),
             drawsub_stack: Vec::new(),
             measuring: self.measuring,
@@ -299,6 +332,10 @@ impl DrawingCtx {
         }
     }
 
+    pub fn locale_tags(&self) -> &LanguageTags {
+        &self.locale_tags
+    }
+
     pub fn toplevel_viewport(&self) -> Rect {
         *self.initial_viewport.vbox
     }
diff --git a/src/element.rs b/src/element.rs
index 3154c321..801b0c9e 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -1,6 +1,5 @@
 //! SVG Elements.
 
-use locale_config::{LanguageRange, Locale};
 use markup5ever::{expanded_name, local_name, namespace_url, ns, QualName};
 use once_cell::sync::Lazy;
 use std::collections::{HashMap, HashSet};
@@ -8,7 +7,7 @@ use std::fmt;
 use std::ops::Deref;
 
 use crate::bbox::BoundingBox;
-use crate::cond::{RequiredExtensions, RequiredFeatures, SystemLanguage};
+use crate::cond::{LanguageTags, RequiredExtensions, RequiredFeatures, SystemLanguage};
 use crate::css::{Declaration, Origin};
 use crate::document::AcquiredNodes;
 use crate::drawing_ctx::DrawingCtx;
@@ -177,7 +176,7 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
         self.values = values.clone();
     }
 
-    fn get_cond(&self) -> bool {
+    fn get_cond(&self, locale_tags: &LanguageTags) -> bool {
         self.required_extensions
             .as_ref()
             .map(|v| v.eval())
@@ -190,7 +189,7 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
             && self
                 .system_language
                 .as_ref()
-                .map(|v| v.eval())
+                .map(|v| v.eval(locale_tags))
                 .unwrap_or(true)
     }
 
@@ -224,7 +223,7 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
 
                 expanded_name!("", "systemLanguage") => {
                     self.system_language =
-                        Some(SystemLanguage::from_attribute(value, &LOCALE).attribute(attr)?);
+                        Some(SystemLanguage::from_attribute(value).attribute(attr)?);
                 }
 
                 _ => {}
@@ -532,8 +531,8 @@ impl Element {
         call_inner!(self, set_computed_values, values);
     }
 
-    pub fn get_cond(&self) -> bool {
-        call_inner!(self, get_cond)
+    pub fn get_cond(&self, locale_tags: &LanguageTags) -> bool {
+        call_inner!(self, get_cond, locale_tags)
     }
 
     pub fn get_transform(&self) -> Transform {
@@ -825,29 +824,6 @@ static ELEMENT_CREATORS: Lazy<HashMap<&'static str, (ElementCreateFn, ElementCre
     creators_table.into_iter().map(|(n, c, f)| (n, (c, f))).collect()
 });
 
-/// Gets the user's preferred locale from the environment and
-/// translates it to a `Locale` with `LanguageRange` fallbacks.
-///
-/// The `Locale::current()` call only contemplates a single language,
-/// but glib is smarter, and `g_get_langauge_names()` can provide
-/// fallbacks, for example, when LC_MESSAGES="en_US.UTF-8:de" (USA
-/// English and German).  This function converts the output of
-/// `g_get_language_names()` into a `Locale` with appropriate
-/// fallbacks.
-static LOCALE: Lazy<Locale> = Lazy::new(|| {
-    let mut locale = Locale::invariant();
-
-    // This call has a lot of memory churn internally with many
-    // short-lived allocations, so we do this only once.
-    for name in glib::get_language_names() {
-        if let Ok(range) = LanguageRange::from_unix(&name) {
-            locale.add(&range);
-        }
-    }
-
-    locale
-});
-
 #[cfg(ignore)]
 mod sizes {
     //! This module is in this file just because here we have all the imports.
diff --git a/src/structure.rs b/src/structure.rs
index 47fc9de5..8a386f1b 100644
--- a/src/structure.rs
+++ b/src/structure.rs
@@ -87,7 +87,7 @@ impl Draw for Switch {
             &mut |an, dc| {
                 if let Some(child) = node.children().filter(|c| c.is_element()).find(|c| {
                     let elt = c.borrow_element();
-                    elt.get_cond() && !elt.is_in_error()
+                    elt.get_cond(&dc.locale_tags()) && !elt.is_in_error()
                 }) {
                     child.draw(an, &CascadedValues::new(cascaded, &child), dc, clipping)
                 } else {