Make thread-safe. Fixes a crash I encountered this morning.

2004-04-20  Jeffrey Stedfast  <fejj@ximian.com>

	* e-icon-factory.c (e_icon_factory_get_icon): Make
	thread-safe. Fixes a crash I encountered this morning.
	(e_icon_factory_get_icon_filename): Same.
	(e_icon_factory_get_icon_list): Here too.

svn path=/trunk/; revision=25541

3 files changed, 179 insertions, 182 deletions

diff --git a/e-util/ChangeLog b/e-util/ChangeLog
index dd154f1c62..19d07fadd7 100644
--- a/e-util/ChangeLog
+++ b/e-util/ChangeLog
@@ -1,3 +1,10 @@
+2004-04-20  Jeffrey Stedfast  <fejj@ximian.com>
+
+	* e-icon-factory.c (e_icon_factory_get_icon): Make
+	thread-safe. Fixes a crash I encountered this morning.
+	(e_icon_factory_get_icon_filename): Same.
+	(e_icon_factory_get_icon_list): Here too.
+
 2004-04-19  Jeffrey Stedfast  <fejj@ximian.com>
 
 	* e-icon-factory.h: Add an enum for icon sizes (temporarily has
diff --git a/e-util/e-icon-factory.c b/e-util/e-icon-factory.c
index 11b1ee7b5b..a558e046b0 100644
--- a/e-util/e-icon-factory.c
+++ b/e-util/e-icon-factory.c
@@ -25,49 +25,52 @@
 #endif
 
 #include <string.h>
+#include <pthread.h>
+
 #include <libgnomeui/gnome-icon-theme.h>
 #include <e-util/e-icon-factory.h>
 #include "art/empty.xpm"
 
-
-/* One icon.  Keep both a small (16x16) and a large (48x48) version around.  */
-struct _Icon {
-	char *name;
-	GdkPixbuf *pixbuf_16;
-	GdkPixbuf *pixbuf_24;
-	GdkPixbuf *pixbuf_48;
+
+static int sizes[E_ICON_NUM_SIZES] = {
+	16, /* menu */
+	20, /* button */
+	18, /* small toolbar */
+	24, /* large toolbar */
+	32, /* dnd */
+	48, /* dialog */
 };
-typedef struct _Icon Icon;
+
+
+typedef struct {
+	char *name;
+	GdkPixbuf *pixbuf[E_ICON_NUM_SIZES];
+} Icon;
 
 /* Hash of all the icons.  */
 static GHashTable     *name_to_icon = NULL;
 static GnomeIconTheme *icon_theme   = NULL;
 static GdkPixbuf      *empty_pixbuf = NULL;
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+
 
-
 /* Creating and destroying icons.  */
 
+/* Note: takes ownership of the pixbufs (eg. does not ref them) */
 static Icon *
-icon_new (const gchar *name,
-	  GdkPixbuf *pixbuf_16,
-	  GdkPixbuf *pixbuf_24,
-	  GdkPixbuf *pixbuf_48)
+icon_new (const char *name, GdkPixbuf **pixbufs)
 {
 	Icon *icon;
-
-	icon = g_new (Icon, 1);
-	icon->name      = g_strdup (name);
-	icon->pixbuf_16 = pixbuf_16;
-	icon->pixbuf_24 = pixbuf_24;
-	icon->pixbuf_48 = pixbuf_48;
-
-	if (pixbuf_16 != NULL)
-		g_object_ref (pixbuf_16);
-	if (pixbuf_24 != NULL)
-		g_object_ref (pixbuf_24);
-	if (pixbuf_48 != NULL)
-		g_object_ref (pixbuf_48);
-
+	int i;
+	
+	icon = g_malloc0 (sizeof (Icon));
+	icon->name = g_strdup (name);
+	
+	if (pixbufs != NULL) {
+		for (i = 0; i < E_ICON_NUM_SIZES; i++)
+			icon->pixbuf[i] = pixbufs[i];
+	}
+	
 	return icon;
 }
 
@@ -78,73 +81,69 @@ icon_new (const gchar *name,
 static void
 icon_free (Icon *icon)
 {
+	int i;
+	
 	g_free (icon->name);
-
-	if (icon->pixbuf_16 != NULL)
-		g_object_unref (icon->pixbuf_16);
-	if (icon->pixbuf_24 != NULL)
-		g_object_unref (icon->pixbuf_24);
-	if (icon->pixbuf_48 != NULL)
-		g_object_unref (icon->pixbuf_48);
-
+	
+	for (i = 0; i < E_ICON_NUM_SIZES; i++) {
+		if (icon->pixbuf[i] != NULL)
+			g_object_unref (icon->pixbuf[i]);
+	}
+	
 	g_free (icon);
 }
 
 #endif
 
-
 /* Loading icons.  */
 
 static Icon *
-load_icon (const gchar *icon_name)
+load_icon (const char *icon_name)
 {
-	GdkPixbuf *unscaled;
-	GdkPixbuf *pixbuf_16;
-	GdkPixbuf *pixbuf_24;
-	GdkPixbuf *pixbuf_48;
-	gchar *filename;
-	Icon *icon;
-
-	filename = gnome_icon_theme_lookup_icon (icon_theme, icon_name, 16, 
-	                                         NULL, NULL);
-
-	if (filename == NULL)
-		return NULL;
-	unscaled = gdk_pixbuf_new_from_file (filename, NULL);
-	pixbuf_16 = gdk_pixbuf_scale_simple (unscaled, 16, 16, GDK_INTERP_BILINEAR);
-	g_object_unref (unscaled);
-	g_free (filename);
+	GdkPixbuf *pixbufs[E_ICON_NUM_SIZES];
+	char *filename;
+	int i, j;
 	
-	filename = gnome_icon_theme_lookup_icon (icon_theme, icon_name, 24, 
-	                                         NULL, NULL);
-
-	if (filename == NULL)
-		return NULL;
-	unscaled = gdk_pixbuf_new_from_file (filename, NULL);
-	pixbuf_24 = gdk_pixbuf_scale_simple (unscaled, 24, 24, GDK_INTERP_BILINEAR);
-	g_object_unref (unscaled);
-	g_free (filename);
+	for (i = 0; i < E_ICON_NUM_SIZES; i++) {
+		GdkPixbuf *unscaled;
+		int size = sizes[i];
+		
+		if (!(filename = gnome_icon_theme_lookup_icon (icon_theme, icon_name, size, NULL, NULL)))
+			goto exception;
+		
+		unscaled = gdk_pixbuf_new_from_file (filename, NULL);
+		pixbufs[i] = gdk_pixbuf_scale_simple (unscaled, size, size, GDK_INTERP_BILINEAR);
+		g_object_unref (unscaled);
+		g_free (filename);
+	}
 	
-	filename = gnome_icon_theme_lookup_icon (icon_theme, icon_name, 48, 
-	                                         NULL, NULL);
-
-	if (filename == NULL)
-		return NULL;
-	unscaled = gdk_pixbuf_new_from_file (filename, NULL);
-	pixbuf_48 = gdk_pixbuf_scale_simple (unscaled, 48, 48, GDK_INTERP_BILINEAR);
-	g_object_unref (unscaled);
-	g_free (filename);
-
-	icon = icon_new (icon_name, pixbuf_16, pixbuf_24, pixbuf_48);
+	return icon_new (icon_name, pixbufs);
+	
+ exception:
+	
+	for (j = 0; j < i; j++)
+		g_object_unref (pixbufs[j]);
+	
+	return NULL;
+}
 
-	g_object_unref (pixbuf_16);
-	g_object_unref (pixbuf_24);
-	g_object_unref (pixbuf_48);
 
-	return icon;
+/* termporary workaround for code that has not yet been ported to the new icon_size API */
+static int
+pixel_size_to_icon_size (int pixel_size)
+{
+	int i, icon_size = -1;
+	
+	for (i = 0; i < E_ICON_NUM_SIZES; i++) {
+		if (pixel_size == sizes[i]) {
+			icon_size = i;
+			break;
+		}
+	}
+	
+	return icon_size;
 }
 
-
 /* Public API.  */
 
 void
@@ -154,26 +153,39 @@ e_icon_factory_init (void)
 		/* Already initialized.  */
 		return;
 	}
-
+	
 	name_to_icon = g_hash_table_new (g_str_hash, g_str_equal);
 	icon_theme   = gnome_icon_theme_new ();
 	empty_pixbuf = gdk_pixbuf_new_from_xpm_data ((const char **) empty_xpm);
 }
 
-gchar *
-e_icon_factory_get_icon_filename (const gchar *icon_name,
-			          gint        icon_size)
+
+/**
+ * e_icon_factory_get_icon_filename:
+ * @icon_name: name of the icon
+ * @size: MENU/SMALL_TOOLBAR/etc
+ *
+ * Looks up the icon to use based on name and size.
+ *
+ * Returns the requested icon pixbuf.
+ **/
+char *
+e_icon_factory_get_icon_filename (const char *icon_name, int icon_size)
 {
-	gchar *filename;
+	char *filename;
 	
 	g_return_val_if_fail (icon_name != NULL, NULL);
 	g_return_val_if_fail (strcmp (icon_name, ""), NULL);
 	
-	filename = gnome_icon_theme_lookup_icon (icon_theme,
-	                                         icon_name,
-	                                         icon_size,
-	                                         NULL,
-	                                         NULL);
+	if (icon_size >= E_ICON_NUM_SIZES) {
+		g_warning ("e_icon_factory_get_icon_filename(): Invalid argument for icon_size: use one of the E_ICON_SIZE_* enum values");
+		if ((icon_size = pixel_size_to_icon_size (icon_size)) == -1)
+			return NULL;
+	}
+	
+	pthread_mutex_lock (&lock);
+	filename = gnome_icon_theme_lookup_icon (icon_theme, icon_name, sizes[icon_size], NULL, NULL);
+	pthread_mutex_unlock (&lock);
 	
 	return filename;
 }
@@ -182,107 +194,85 @@ e_icon_factory_get_icon_filename (const gchar *icon_name,
    The returned icon is guaranteed to be the requested size and exist.  If
    the themed icon cannot be found, an empty icon is returned. */
 GdkPixbuf *
-e_icon_factory_get_icon (const gchar *icon_name,
-			 gint        icon_size)
+e_icon_factory_get_icon (const char *icon_name, int icon_size)
 {
-	if (icon_name != NULL && strcmp (icon_name, "")) {
-		Icon *icon;
-
-		icon = g_hash_table_lookup (name_to_icon, icon_name);
-		if (icon == NULL) {
-			icon = load_icon (icon_name);
-			if (icon == NULL) {
-				g_warning ("Icon not found -- %s", icon_name);
-
-				/* Create an empty icon so that we don't keep spitting
-				   out the same warning over and over, every time this
-				   icon is requested.  */
-
-				icon = icon_new (icon_name, NULL, NULL, NULL);
-				g_hash_table_insert (name_to_icon, icon->name, icon);
-			}
-			else {
-				g_hash_table_insert (name_to_icon, icon->name, icon);
-			}
-		}
-
-		if (icon->pixbuf_16) {
-			gchar *filename;
-			GdkPixbuf *pixbuf, *scaled;
-			
-			switch (icon_size) {
-			case 16:
-				return g_object_ref (icon->pixbuf_16);
-			
-			case 24:
-				return g_object_ref (icon->pixbuf_24);
+	GdkPixbuf *pixbuf;
+	Icon *icon;
+	int size;
+	
+	if (icon_size >= E_ICON_NUM_SIZES) {
+		g_warning ("e_icon_factory_get_icon(): Invalid argument for icon_size: use one of the E_ICON_SIZE_* enum values");
+		if ((icon_size = pixel_size_to_icon_size (icon_size)) == -1)
+			return NULL;
+	}
+	
+	if (icon_name == NULL || !strcmp (icon_name, "")) {
+		size = sizes[icon_size];
+		return gdk_pixbuf_scale_simple (empty_pixbuf, size, size, GDK_INTERP_NEAREST);
+	}
+	
+	pthread_mutex_lock (&lock);
+	
+	if (!(icon = g_hash_table_lookup (name_to_icon, icon_name))) {
+		if (!(icon = load_icon (icon_name))) {
+			g_warning ("Icon not found -- %s", icon_name);
 			
-			case 48:
-				return g_object_ref (icon->pixbuf_48);
+			/* Create an empty icon so that we don't keep spitting
+			   out the same warning over and over, every time this
+			   icon is requested.  */
 			
-			default:
-				/* Non-standard size.  Do a non-cached load. */
-				
-				filename = gnome_icon_theme_lookup_icon (icon_theme,
-				                                         icon_name,
-				                                         icon_size,
-				                                         NULL,
-				                                         NULL);
-				if (filename == NULL)
-					break;
-				
-				pixbuf = gdk_pixbuf_new_from_file (filename, NULL);
-				g_free (filename);
-				if (pixbuf == NULL)
-					break;
-				
-				scaled = gdk_pixbuf_scale_simple (pixbuf, icon_size, icon_size, GDK_INTERP_BILINEAR);
-				g_object_unref (pixbuf);
-				
-				return scaled;
-			}
+			icon = icon_new (icon_name, NULL);
+			g_hash_table_insert (name_to_icon, icon->name, icon);
+		} else {
+			g_hash_table_insert (name_to_icon, icon->name, icon);
 		}
 	}
 	
-	/* icon not found -- create an empty icon */
-	return gdk_pixbuf_scale_simple (empty_pixbuf, icon_size, icon_size, GDK_INTERP_NEAREST);
+	if ((pixbuf = icon->pixbuf[icon_size]))
+		g_object_ref (pixbuf);
+	
+	pthread_mutex_unlock (&lock);
+	
+	return pixbuf;
 }
 
+
 GList *
-e_icon_factory_get_icon_list (const gchar *icon_name)
+e_icon_factory_get_icon_list (const char *icon_name)
 {
-	if (icon_name != NULL && strcmp (icon_name, "")) {
-		Icon *icon;
-
-		icon = g_hash_table_lookup (name_to_icon, icon_name);
-		if (icon == NULL) {
-			icon = load_icon (icon_name);
-			if (icon == NULL) {
-				g_warning ("Icon not found -- %s", icon_name);
-
-				/* Create an empty icon so that we don't keep spitting
-				   out the same warning over and over, every time this
-				   icon is requested.  */
-
-				icon = icon_new (icon_name, NULL, NULL, NULL);
-				g_hash_table_insert (name_to_icon, icon->name, icon);
-			}
-			else {
-				g_hash_table_insert (name_to_icon, icon->name, icon);
-			}
-		}
-		
-		if (icon->pixbuf_16) {
-			GList *list = NULL;
+	GList *list = NULL;
+	Icon *icon;
+	int i;
+	
+	if (!icon_name || !strcmp (icon_name, ""))
+		return NULL;
+	
+	pthread_mutex_lock (&lock);
+	
+	if (!(icon = g_hash_table_lookup (name_to_icon, icon_name))) {
+		if (!(icon = load_icon (icon_name))) {
+			g_warning ("Icon not found -- %s", icon_name);
 			
-			list = g_list_append (list, g_object_ref (icon->pixbuf_48));
-			list = g_list_append (list, g_object_ref (icon->pixbuf_24));
-			list = g_list_append (list, g_object_ref (icon->pixbuf_16));
+			/* Create an empty icon so that we don't keep spitting
+			   out the same warning over and over, every time this
+			   icon is requested.  */
 			
-			return list;
+			icon = icon_new (icon_name, NULL);
+			g_hash_table_insert (name_to_icon, icon->name, icon);
+			return NULL;
+		} else {
+			g_hash_table_insert (name_to_icon, icon->name, icon);
 		}
 	}
 	
-	/* icons not found */
-	return NULL;
+	for (i = 0; i < E_ICON_NUM_SIZES; i++) {
+		if (icon->pixbuf[i]) {
+			list = g_list_prepend (list, icon->pixbuf[i]);
+			g_object_ref (icon->pixbuf[i]);
+		}
+	}
+	
+	pthread_mutex_unlock (&lock);
+	
+	return list;
 }
diff --git a/e-util/e-icon-factory.h b/e-util/e-icon-factory.h
index 866fc32aba..809576d92a 100644
--- a/e-util/e-icon-factory.h
+++ b/e-util/e-icon-factory.h
@@ -26,12 +26,12 @@
 #include <gdk-pixbuf/gdk-pixbuf.h>
 
 enum {
-	E_ICON_SIZE_MENU          = 16,
-	E_ICON_SIZE_BUTTON        = 20,
-	E_ICON_SIZE_SMALL_TOOLBAR = 18,
-	E_ICON_SIZE_LARGE_TOOLBAR = 24,
-	E_ICON_SIZE_DND           = 32,
-	E_ICON_SIZE_DIALOG        = 48,
+	E_ICON_SIZE_MENU,
+	E_ICON_SIZE_BUTTON,
+	E_ICON_SIZE_SMALL_TOOLBAR,
+	E_ICON_SIZE_LARGE_TOOLBAR,
+	E_ICON_SIZE_DND,
+	E_ICON_SIZE_DIALOG,
 	E_ICON_NUM_SIZES
 };