path: root/gnome-base/gnome-shell/files/3.34.4-custom_stylesheet_crash.patch

From 87afb02e50c7a636a508cf619beb68a46168e54b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 6 May 2019 18:28:40 -0500
Subject: [PATCH 1/5] st-theme: Use CRStyleSheet app_data instead of hash map

Use the CRStyleSheet field to save stylesheet details instead of using an extra
hash table. In this way we can access to the stylesheet file faster without
having to lookup it. Define a destroy function so that we can automatically
remove the data when the container hash table is destroyed.

Fixes https://gitlab.gnome.org/GNOME/gnome-shell/issues/1265

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/536
---
 src/st/st-theme.c | 91 +++++++++++++++++++++++++++++++----------------
 1 file changed, 60 insertions(+), 31 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index b567f7e5e3..208f536c5b 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -66,11 +66,16 @@ struct _StTheme
   GSList *custom_stylesheets;
 
   GHashTable *stylesheets_by_file;
-  GHashTable *files_by_stylesheet;
 
   CRCascade *cascade;
 };
 
+typedef struct _StyleSheetData
+{
+  GFile *file;
+  gboolean extension_stylesheet;
+} StyleSheetData;
+
 enum
 {
   PROP_0,
@@ -106,12 +111,25 @@ file_equal0 (GFile *file1,
   return g_file_equal (file1, file2);
 }
 
+static void
+stylesheet_destroy (CRStyleSheet *stylesheet)
+{
+  if (stylesheet->app_data)
+    {
+      g_slice_free (StyleSheetData, stylesheet->app_data);
+      stylesheet->app_data = NULL;
+    }
+
+  cr_stylesheet_unref (stylesheet);
+}
+
 static void
 st_theme_init (StTheme *theme)
 {
-  theme->stylesheets_by_file = g_hash_table_new_full (g_file_hash, (GEqualFunc) g_file_equal,
-                                                      (GDestroyNotify)g_object_unref, (GDestroyNotify)cr_stylesheet_unref);
-  theme->files_by_stylesheet = g_hash_table_new (g_direct_hash, g_direct_equal);
+  theme->stylesheets_by_file =
+    g_hash_table_new_full (g_file_hash, (GEqualFunc) g_file_equal,
+                           (GDestroyNotify) g_object_unref,
+                           (GDestroyNotify) stylesheet_destroy);
 }
 
 static void
@@ -205,9 +223,6 @@ parse_stylesheet (GFile   *file,
       return NULL;
     }
 
-  /* Extension stylesheet */
-  stylesheet->app_data = GUINT_TO_POINTER (FALSE);
-
   return stylesheet;
 }
 
@@ -234,19 +249,30 @@ parse_stylesheet_nofail (GFile *file)
   return result;
 }
 
-static void
+static gboolean
 insert_stylesheet (StTheme      *theme,
                    GFile        *file,
                    CRStyleSheet *stylesheet)
 {
+  StyleSheetData *stylesheet_data;
+
   if (stylesheet == NULL)
-    return;
+    return FALSE;
 
-  g_object_ref (file);
-  cr_stylesheet_ref (stylesheet);
+  if (g_hash_table_contains (theme->stylesheets_by_file, file))
+    {
+      cr_stylesheet_unref (stylesheet);
+      return FALSE;
+    }
 
-  g_hash_table_insert (theme->stylesheets_by_file, file, stylesheet);
-  g_hash_table_insert (theme->files_by_stylesheet, stylesheet, file);
+  stylesheet_data = g_slice_new0 (StyleSheetData);
+  stylesheet_data->file = file;
+  stylesheet->app_data = stylesheet_data;
+
+  cr_stylesheet_ref (stylesheet);
+  g_hash_table_insert (theme->stylesheets_by_file,
+                       g_object_ref (file), stylesheet);
+  return TRUE;
 }
 
 gboolean
@@ -255,14 +281,15 @@ st_theme_load_stylesheet (StTheme    *theme,
                           GError    **error)
 {
   CRStyleSheet *stylesheet;
+  StyleSheetData *stylesheet_data;
 
   stylesheet = parse_stylesheet (file, error);
-  if (!stylesheet)
+  if (!insert_stylesheet (theme, file, stylesheet))
     return FALSE;
 
-  stylesheet->app_data = GUINT_TO_POINTER (TRUE);
+  stylesheet_data = stylesheet->app_data;
+  stylesheet_data->extension_stylesheet = 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);
@@ -283,9 +310,8 @@ st_theme_unload_stylesheet (StTheme    *theme,
   if (!g_slist_find (theme->custom_stylesheets, stylesheet))
     return;
 
-  theme->custom_stylesheets = g_slist_remove (theme->custom_stylesheets, stylesheet);
   g_hash_table_remove (theme->stylesheets_by_file, file);
-  g_hash_table_remove (theme->files_by_stylesheet, stylesheet);
+  theme->custom_stylesheets = g_slist_remove (theme->custom_stylesheets, stylesheet);
   cr_stylesheet_unref (stylesheet);
   g_signal_emit (theme, signals[STYLESHEETS_CHANGED], 0);
 }
@@ -306,9 +332,10 @@ st_theme_get_custom_stylesheets (StTheme *theme)
   for (iter = theme->custom_stylesheets; iter; iter = iter->next)
     {
       CRStyleSheet *stylesheet = iter->data;
-      GFile *file = g_hash_table_lookup (theme->files_by_stylesheet, stylesheet);
+      StyleSheetData *stylesheet_data = stylesheet->app_data;
 
-      result = g_slist_prepend (result, g_object_ref (file));
+      if (stylesheet_data && stylesheet_data->file)
+        result = g_slist_prepend (result, g_object_ref (stylesheet_data->file));
     }
 
   return result;
@@ -350,7 +377,6 @@ st_theme_finalize (GObject * object)
   theme->custom_stylesheets = NULL;
 
   g_hash_table_destroy (theme->stylesheets_by_file);
-  g_hash_table_destroy (theme->files_by_stylesheet);
 
   g_clear_object (&theme->application_stylesheet);
   g_clear_object (&theme->theme_stylesheet);
@@ -877,18 +903,20 @@ add_matched_properties (StTheme      *a_this,
 
                 if (import_rule->url->stryng && import_rule->url->stryng->str)
                   {
+                    CRStyleSheet *sheet;
                     file = _st_theme_resolve_url (a_this,
                                                   a_nodesheet,
                                                   import_rule->url->stryng->str);
-                    import_rule->sheet = parse_stylesheet (file, NULL);
-                  }
+                    sheet = parse_stylesheet (file, NULL);
 
-                if (import_rule->sheet)
-                  {
-                    insert_stylesheet (a_this, file, import_rule->sheet);
-                    /* refcount of stylesheets starts off at zero, so we don't need to unref! */
+                    if (insert_stylesheet (a_this, file, sheet))
+                      import_rule->sheet = sheet;
+
+                    /* refcount of stylesheets starts off at zero, so we don't
+                     * need to unref! */
                   }
-                else
+
+                if (!import_rule->sheet)
                   {
                     /* Set a marker to avoid repeatedly trying to parse a non-existent or
                      * broken stylesheet
@@ -962,12 +990,12 @@ static inline int
 get_origin (const CRDeclaration * decl)
 {
   enum CRStyleOrigin origin = decl->parent_statement->parent_sheet->origin;
-  gboolean is_extension_sheet = GPOINTER_TO_UINT (decl->parent_statement->parent_sheet->app_data);
+  StyleSheetData *sheet_data = decl->parent_statement->parent_sheet->app_data;
 
   if (decl->important)
     origin += ORIGIN_OFFSET_IMPORTANT;
 
-  if (is_extension_sheet)
+  if (sheet_data && sheet_data->extension_stylesheet)
     origin += ORIGIN_OFFSET_EXTENSION;
 
   return origin;
@@ -1046,8 +1074,9 @@ _st_theme_resolve_url (StTheme      *theme,
   else if (base_stylesheet != NULL)
     {
       GFile *base_file = NULL, *parent;
+      StyleSheetData *stylesheet_data = base_stylesheet->app_data;
 
-      base_file = g_hash_table_lookup (theme->files_by_stylesheet, base_stylesheet);
+      base_file = stylesheet_data->file;
 
       /* This is an internal function, if we get here with
          a bad @base_stylesheet we have a problem. */
-- 
2.24.1


From 0974a3290aa2951ada3c9f2adceb2c65c2321849 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 6 May 2019 18:33:36 -0500
Subject: [PATCH 2/5] st-theme: Use newer functions to finalize objects

Use g_slist_free_full on custom stylesheets list and clear the hashtable and
its pointer using g_clear_pointer and g_hash_table_destroy.

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/536
---
 src/st/st-theme.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index 208f536c5b..2d51420a20 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -372,11 +372,11 @@ st_theme_finalize (GObject * object)
 {
   StTheme *theme = ST_THEME (object);
 
-  g_slist_foreach (theme->custom_stylesheets, (GFunc) cr_stylesheet_unref, NULL);
-  g_slist_free (theme->custom_stylesheets);
-  theme->custom_stylesheets = NULL;
+  g_clear_pointer (&theme->stylesheets_by_file, g_hash_table_destroy);
 
-  g_hash_table_destroy (theme->stylesheets_by_file);
+  g_slist_free_full (theme->custom_stylesheets,
+                     (GDestroyNotify) cr_stylesheet_unref);
+  theme->custom_stylesheets = NULL;
 
   g_clear_object (&theme->application_stylesheet);
   g_clear_object (&theme->theme_stylesheet);
-- 
2.24.1


From 801fd1884af2011ec6981e4d7844aac6f8dbecf0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 7 May 2019 01:59:11 -0500
Subject: [PATCH 3/5] st-theme: Remove custom stylesheets list

Since we already mark the stylesheet laded by extensions in the data, we don't
need to use another list to go trough these as we can just iterate over the
hash table with a minimum overhead, as this will normally contain just one
default stylesheet and all the extension stylesheets anyways.

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/536
---
 src/st/st-theme.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index 2d51420a20..9f6b2f6e35 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -63,7 +63,6 @@ struct _StTheme
   GFile *application_stylesheet;
   GFile *default_stylesheet;
   GFile *theme_stylesheet;
-  GSList *custom_stylesheets;
 
   GHashTable *stylesheets_by_file;
 
@@ -290,8 +289,6 @@ st_theme_load_stylesheet (StTheme    *theme,
   stylesheet_data = stylesheet->app_data;
   stylesheet_data->extension_stylesheet = TRUE;
 
-  cr_stylesheet_ref (stylesheet);
-  theme->custom_stylesheets = g_slist_prepend (theme->custom_stylesheets, stylesheet);
   g_signal_emit (theme, signals[STYLESHEETS_CHANGED], 0);
 
   return TRUE;
@@ -302,17 +299,17 @@ st_theme_unload_stylesheet (StTheme    *theme,
                             GFile      *file)
 {
   CRStyleSheet *stylesheet;
+  StyleSheetData *stylesheet_data;
 
   stylesheet = g_hash_table_lookup (theme->stylesheets_by_file, file);
-  if (!stylesheet)
+  if (!stylesheet || !stylesheet->app_data)
     return;
 
-  if (!g_slist_find (theme->custom_stylesheets, stylesheet))
+  stylesheet_data = stylesheet->app_data;
+  if (!stylesheet_data->extension_stylesheet)
     return;
 
   g_hash_table_remove (theme->stylesheets_by_file, file);
-  theme->custom_stylesheets = g_slist_remove (theme->custom_stylesheets, stylesheet);
-  cr_stylesheet_unref (stylesheet);
   g_signal_emit (theme, signals[STYLESHEETS_CHANGED], 0);
 }
 
@@ -327,14 +324,17 @@ GSList*
 st_theme_get_custom_stylesheets (StTheme *theme)
 {
   GSList *result = NULL;
-  GSList *iter;
+  GHashTableIter iter;
+  gpointer value;
 
-  for (iter = theme->custom_stylesheets; iter; iter = iter->next)
+  g_hash_table_iter_init (&iter, theme->stylesheets_by_file);
+
+  while (g_hash_table_iter_next (&iter, NULL, &value))
     {
-      CRStyleSheet *stylesheet = iter->data;
+      CRStyleSheet *stylesheet = value;
       StyleSheetData *stylesheet_data = stylesheet->app_data;
 
-      if (stylesheet_data && stylesheet_data->file)
+      if (stylesheet_data && stylesheet_data->extension_stylesheet)
         result = g_slist_prepend (result, g_object_ref (stylesheet_data->file));
     }
 
@@ -374,10 +374,6 @@ st_theme_finalize (GObject * object)
 
   g_clear_pointer (&theme->stylesheets_by_file, g_hash_table_destroy);
 
-  g_slist_free_full (theme->custom_stylesheets,
-                     (GDestroyNotify) cr_stylesheet_unref);
-  theme->custom_stylesheets = NULL;
-
   g_clear_object (&theme->application_stylesheet);
   g_clear_object (&theme->theme_stylesheet);
   g_clear_object (&theme->default_stylesheet);
@@ -1030,7 +1026,7 @@ _st_theme_get_matched_properties (StTheme        *theme,
   enum CRStyleOrigin origin = 0;
   CRStyleSheet *sheet = NULL;
   GPtrArray *props = g_ptr_array_new ();
-  GSList *iter;
+  GHashTableIter iter;
 
   g_return_val_if_fail (ST_IS_THEME (theme), NULL);
   g_return_val_if_fail (ST_IS_THEME_NODE (node), NULL);
@@ -1044,8 +1040,13 @@ _st_theme_get_matched_properties (StTheme        *theme,
       add_matched_properties (theme, sheet, node, props);
     }
 
-  for (iter = theme->custom_stylesheets; iter; iter = iter->next)
-    add_matched_properties (theme, iter->data, node, props);
+  g_hash_table_iter_init (&iter, theme->stylesheets_by_file);
+  while (g_hash_table_iter_next (&iter, NULL, (gpointer) &sheet))
+    {
+      if (sheet->app_data &&
+          ((StyleSheetData *) sheet->app_data)->extension_stylesheet)
+        add_matched_properties (theme, sheet, node, props);
+    }
 
   /* We count on a stable sort here so that later declarations come
    * after earlier declarations */
-- 
2.24.1


From 5b66ecb1019f3ba32aa05562a8b58ebd95702669 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 6 May 2019 18:56:49 -0500
Subject: [PATCH 4/5] st-theme: Ref stylesheet using facility function

Define stylesheet_ref as a wrapper of cr_stylesheet_ref that returns the
ref'ed instance so that we can use this on the caller function to make clear
what we're passing.

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/536
---
 src/st/st-theme.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index 9f6b2f6e35..30733b6fd5 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -110,6 +110,13 @@ file_equal0 (GFile *file1,
   return g_file_equal (file1, file2);
 }
 
+static inline CRStyleSheet *
+stylesheet_ref (CRStyleSheet *stylesheet)
+{
+  cr_stylesheet_ref (stylesheet);
+  return stylesheet;
+}
+
 static void
 stylesheet_destroy (CRStyleSheet *stylesheet)
 {
@@ -268,9 +275,8 @@ insert_stylesheet (StTheme      *theme,
   stylesheet_data->file = file;
   stylesheet->app_data = stylesheet_data;
 
-  cr_stylesheet_ref (stylesheet);
   g_hash_table_insert (theme->stylesheets_by_file,
-                       g_object_ref (file), stylesheet);
+                       g_object_ref (file), stylesheet_ref (stylesheet));
   return TRUE;
 }
 
-- 
2.24.1


From bf5ea4dc4fbcf09c77a830ae9a227660e1c72d7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 6 May 2019 18:40:13 -0500
Subject: [PATCH 5/5] st-theme: Use glib auto free/ptr features

Use g_autofree and g_autoptr for managing memory in a smarter way.

https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/536
---
 src/st/st-theme.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/src/st/st-theme.c b/src/st/st-theme.c
index 30733b6fd5..4c8730a4ce 100644
--- a/src/st/st-theme.c
+++ b/src/st/st-theme.c
@@ -205,7 +205,7 @@ parse_stylesheet (GFile   *file,
 {
   enum CRStatus status;
   CRStyleSheet *stylesheet;
-  char *contents;
+  g_autofree char *contents = NULL;
   gsize length;
 
   if (file == NULL)
@@ -218,14 +218,11 @@ parse_stylesheet (GFile   *file,
                                           length,
                                           CR_UTF_8,
                                           &stylesheet);
-  g_free (contents);
-
   if (status != CR_OK)
     {
-      char *uri = g_file_get_uri (file);
+      g_autofree char *uri = g_file_get_uri (file);
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED,
                    "Error parsing stylesheet '%s'; errcode:%d", uri, status);
-      g_free (uri);
       return NULL;
     }
 
@@ -901,7 +898,7 @@ add_matched_properties (StTheme      *a_this,
 
             if (import_rule->sheet == NULL)
               {
-                GFile *file = NULL;
+                g_autoptr (GFile) file = NULL;
 
                 if (import_rule->url->stryng && import_rule->url->stryng->str)
                   {
@@ -925,9 +922,6 @@ add_matched_properties (StTheme      *a_this,
                      */
                     import_rule->sheet = (CRStyleSheet *) - 1;
                   }
-
-                if (file)
-                  g_object_unref (file);
               }
 
             if (import_rule->sheet != (CRStyleSheet *) - 1)
@@ -1070,20 +1064,19 @@ _st_theme_resolve_url (StTheme      *theme,
                        CRStyleSheet *base_stylesheet,
                        const char   *url)
 {
-  char *scheme;
+  g_autofree char *scheme = NULL;
   GFile *resource;
 
   if ((scheme = g_uri_parse_scheme (url)))
     {
-      g_free (scheme);
       resource = g_file_new_for_uri (url);
     }
   else if (base_stylesheet != NULL)
     {
-      GFile *base_file = NULL, *parent;
       StyleSheetData *stylesheet_data = base_stylesheet->app_data;
+      GFile *base_file = stylesheet_data->file;
+      g_autoptr (GFile) parent = NULL;
 
-      base_file = stylesheet_data->file;
 
       /* This is an internal function, if we get here with
          a bad @base_stylesheet we have a problem. */
@@ -1091,8 +1084,6 @@ _st_theme_resolve_url (StTheme      *theme,
 
       parent = g_file_get_parent (base_file);
       resource = g_file_resolve_relative_path (parent, url);
-
-      g_object_unref (parent);
     }
   else
     {
-- 
2.24.1