From 7909d2f989e937e9300933a718e1721bf7346d9d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Wed, 29 Jan 2025 02:46:44 +0100
Subject: [PATCH] st/theme: Reuse stylesheets if possible

The following happens when processing an `@import()` rule:

 1. `_st_theme_resolve_url()` to resolve file
 2. `insert_stylesheet()` to track file/sheet
   a. take ownership of file/sheet (ref)
   b. use file as key in `stylesheets_by_file` hash table
   c. use file as value in `files_by_stylesheet` hash table
 3. release reference to file

This leads to a refcount error when importing a file that
was already parsed before:

 1. file start with refcount 1
 2. `insert_stylesheet()`
   a. increases refcount to 2
   b. inserting into `stylesheets_by_file` *decreases* the
      passed-in key if the key already exists
   c. `files_by_stylesheet` now tracks a file with recount 1
 3. releases the last reference to file

The file object tracked in `files_by_stylesheet` is now invalid,
and accessing it results in a crash.

Avoid this issue by reusing existing stylesheets, so we don't insert
a stylesheet that's already tracked.

As a side-effect, this also saves us from re-parsing the same file
unnecessarily.

Closes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/7306
Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/3619>
---
 src/st/st-theme.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index b567f7e5e3..3ecaf5ed1b 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -249,6 +249,27 @@ insert_stylesheet (StTheme      *theme,
   g_hash_table_insert (theme->files_by_stylesheet, stylesheet, file);
 }
 
+static CRStyleSheet *
+resolve_stylesheet (StTheme  *theme,
+                    GFile    *file,
+                    GError  **error)
+{
+  CRStyleSheet *sheet;
+
+  sheet = g_hash_table_lookup (theme->stylesheets_by_file, file);
+  if (sheet)
+    {
+      cr_stylesheet_ref (sheet);
+      return sheet;
+    }
+
+  sheet = parse_stylesheet (file, error);
+  if (sheet)
+    insert_stylesheet (theme, file, sheet);
+
+  return sheet;
+}
+
 gboolean
 st_theme_load_stylesheet (StTheme    *theme,
                           GFile      *file,
@@ -256,13 +277,12 @@ st_theme_load_stylesheet (StTheme    *theme,
 {
   CRStyleSheet *stylesheet;
 
-  stylesheet = parse_stylesheet (file, error);
+  stylesheet = resolve_stylesheet (theme, file, error);
   if (!stylesheet)
     return FALSE;
 
   stylesheet->app_data = GUINT_TO_POINTER (TRUE);
 
-  insert_stylesheet (theme, file, stylesheet);
   cr_stylesheet_ref (stylesheet);
   theme->custom_stylesheets = g_slist_prepend (theme->custom_stylesheets, stylesheet);
   g_signal_emit (theme, signals[STYLESHEETS_CHANGED], 0);
@@ -873,6 +893,7 @@ add_matched_properties (StTheme      *a_this,
 
             if (import_rule->sheet == NULL)
               {
+                CRStyleSheet *sheet = NULL;
                 GFile *file = NULL;
 
                 if (import_rule->url->stryng && import_rule->url->stryng->str)
@@ -880,13 +901,12 @@ add_matched_properties (StTheme      *a_this,
                     file = _st_theme_resolve_url (a_this,
                                                   a_nodesheet,
                                                   import_rule->url->stryng->str);
-                    import_rule->sheet = parse_stylesheet (file, NULL);
+                    sheet = resolve_stylesheet (a_this, file, NULL);
                   }
 
-                if (import_rule->sheet)
+                if (sheet)
                   {
-                    insert_stylesheet (a_this, file, import_rule->sheet);
-                    /* refcount of stylesheets starts off at zero, so we don't need to unref! */
+                    import_rule->sheet = sheet;
                   }
                 else
                   {
-- 
2.49.0