Fix wIconUpdate logic

This patch changes the logic for choosing the icon, and gets rid of
wIconChangeImage in favor of wIconChangeImageFile.

Old logic:
* On create, load either NET_WM_ICON or the file from disk.
* On update, choose (in order):
  1. WM_HINTS icon_window
  2. WM_HINTS icon_pixmap
  3. Whatever was loaded on creation, unless wIconChangeImage or
	 wIconChangeImageFile was called.
  4. Default icon.

New logic:
* On update, choose (in order):
  1. WM_HINTS icon_window
  2. NET_WM_ICON
  3. WM_HINTS icon_pixmap
  4. Icon file from disk.
  5. Default icon.

Signed-off-by: Brad Jorsch <anomie@users.sourceforge.net>

src/application.c

@@ -364,7 +364,6 @@ WApplication *wApplicationCreate(WWindow * wwin)
 	if (wapp->app_icon) {
 		char *tmp, *path;
 		struct stat dummy;
-		RImage *image;
 
 		tmp = wDefaultGetIconFile(scr, wapp->app_icon->wm_instance, wapp->app_icon->wm_class, True);
 
@@ -377,16 +376,8 @@ WApplication *wApplicationCreate(WWindow * wwin)
 			if (path) {
 				wfree(path);
 			}
-			image = wDefaultGetImage(scr, wapp->app_icon->wm_instance, wapp->app_icon->wm_class);
-			if (image) {
-				wIconChangeImage(wapp->app_icon->icon, image);
-				wAppIconPaint(wapp->app_icon);
-				/* TODO:
-				 * wIconChangeImage() should be rewriten to use retain/release
-				 * The way it is now is too confusing about where the icon is
-				 * finally released.  -Dan */
-				/* --this is wrong at the moment-- RReleaseImage(image); */
-			}
+			wIconUpdate(wapp->app_icon->icon);
+			wAppIconPaint(wapp->app_icon);
 		}
 
 		/* if the displayed icon was supplied by the client, save the icon */

src/defaults.c

@@ -1139,17 +1139,8 @@ void wDefaultUpdateIcons(WScreen * scr)
 		file = wDefaultGetIconFile(scr, aicon->wm_instance, aicon->wm_class, False);
 		if ((file && aicon->icon->file && strcmp(file, aicon->icon->file) != 0)
 		    || (file && !aicon->icon->file)) {
-			RImage *new_image;
-
-			if (aicon->icon->file)
-				wfree(aicon->icon->file);
-			aicon->icon->file = wstrdup(file);
-
-			new_image = wDefaultGetImage(scr, aicon->wm_instance, aicon->wm_class);
-			if (new_image) {
-				wIconChangeImage(aicon->icon, new_image);
-				wAppIconPaint(aicon);
-			}
+			wIconChangeImageFile(aicon->icon, file);
+			wAppIconPaint(aicon);
 		}
 		aicon = aicon->next;
 	}
@@ -1162,15 +1153,7 @@ void wDefaultUpdateIcons(WScreen * scr)
 			file = wDefaultGetIconFile(scr, wwin->wm_instance, wwin->wm_class, False);
 			if ((file && wwin->icon->file && strcmp(file, wwin->icon->file) != 0)
 			    || (file && !wwin->icon->file)) {
-				RImage *new_image;
-
-				if (wwin->icon->file)
-					wfree(wwin->icon->file);
-				wwin->icon->file = wstrdup(file);
-
-				new_image = wDefaultGetImage(scr, wwin->wm_instance, wwin->wm_class);
-				if (new_image)
-					wIconChangeImage(wwin->icon, new_image);
+				wIconChangeImageFile(wwin->icon, file);
 			}
 		}
 		wwin = wwin->prev;

src/icon.c

@@ -154,10 +154,7 @@ WIcon *wIconCreate(WWindow * wwin)
 #else
 	icon->show_title = 1;
 #endif
-	if (!icon->image && !WFLAGP(wwin, always_user_icon))
-		icon->image = RRetainImage(wwin->net_icon_image);
-	if (!icon->image)
-		icon->image = wDefaultGetImage(scr, wwin->wm_instance, wwin->wm_class);
+	icon->file_image = wDefaultGetImage(scr, wwin->wm_instance, wwin->wm_class);
 
 	file = wDefaultGetIconFile(scr, wwin->wm_instance, wwin->wm_class, False);
 	if (file) {
@@ -213,12 +210,12 @@ WIcon *wIconCreateWithIconFile(WScreen * scr, char *iconfile, int tile)
 	icon->core->stacking->child_of = NULL;
 
 	if (iconfile) {
-		icon->image = RLoadImage(scr->rcontext, iconfile, 0);
-		if (!icon->image) {
+		icon->file_image = RLoadImage(scr->rcontext, iconfile, 0);
+		if (!icon->file_image) {
 			wwarning(_("error loading image file \"%s\": %s"), iconfile, RMessageForError(RErrorCode));
 		}
 
-		icon->image = wIconValidateIconSize(scr, icon->image);
+		icon->file_image = wIconValidateIconSize(scr, icon->file_image);
 
 		icon->file = wstrdup(iconfile);
 	}
@@ -262,8 +259,8 @@ void wIconDestroy(WIcon * icon)
 	if (icon->file)
 		wfree(icon->file);
 
-	if (icon->image != NULL)
-		RReleaseImage(icon->image);
+	if (icon->file_image != NULL)
+		RReleaseImage(icon->file_image);
 
 	wCoreDestroy(icon->core);
 	wfree(icon);
@@ -357,18 +354,6 @@ void wIconChangeTitle(WIcon * icon, char *new_title)
 	wIconPaint(icon);
 }
 
-void wIconChangeImage(WIcon * icon, RImage * new_image)
-{
-	assert(icon != NULL);
-
-	if (icon->image)
-		RReleaseImage(icon->image);
-
-	icon->image = new_image;
-
-	wIconUpdate(icon);
-}
-
 RImage *wIconValidateIconSize(WScreen * scr, RImage * icon)
 {
 	RImage *tmp;
@@ -397,17 +382,20 @@ Bool wIconChangeImageFile(WIcon * icon, char *file)
 	char *path;
 	int error = 0;
 
+	if (icon->file_image)
+		RReleaseImage(icon->file_image);
+
 	if (!file) {
-		wIconChangeImage(icon, NULL);
+		icon->file_image = NULL;
+		wIconUpdate(icon);
 		return True;
 	}
 
 	path = FindImage(wPreferences.icon_path, file);
 
 	if (path && (image = RLoadImage(scr->rcontext, path, 0))) {
-		image = wIconValidateIconSize(icon->core->screen_ptr, image);
-
-		wIconChangeImage(icon, image);
+		icon->file_image = wIconValidateIconSize(icon->core->screen_ptr, image);
+		wIconUpdate(icon);
 	} else {
 		error = 1;
 	}
@@ -483,20 +471,23 @@ static char *getnameforicon(WWindow * wwin)
 char *wIconStore(WIcon * icon)
 {
 	char *path;
-	RImage *image;
+	RImage *image = NULL;
 	WWindow *wwin = icon->owner;
 
-	if (!wwin || !wwin->wm_hints || !(wwin->wm_hints->flags & IconPixmapHint)
-	    || wwin->wm_hints->icon_pixmap == None)
-		return NULL;
+	if (!wwin) return NULL;
 
 	path = getnameforicon(wwin);
 	if (!path)
 		return NULL;
 
-	image = RCreateImageFromDrawable(icon->core->screen_ptr->rcontext,
-					 wwin->wm_hints->icon_pixmap, (wwin->wm_hints->flags & IconMaskHint)
-					 ? wwin->wm_hints->icon_mask : None);
+	if (wwin->net_icon_image) {
+		image = RRetainImage(wwin->net_icon_image);
+	} else if (wwin->wm_hints && (wwin->wm_hints->flags & IconPixmapHint)
+	    && wwin->wm_hints->icon_pixmap != None) {
+		image = RCreateImageFromDrawable(icon->core->screen_ptr->rcontext,
+						 wwin->wm_hints->icon_pixmap, (wwin->wm_hints->flags & IconMaskHint)
+						 ? wwin->wm_hints->icon_mask : None);
+	}
 	if (!image) {
 		wfree(path);
 		return NULL;
@@ -571,11 +562,11 @@ void wIconUpdate(WIcon * icon)
 		XFreePixmap(dpy, icon->pixmap);
 	icon->pixmap = None;
 
-	if (wwin && (WFLAGP(wwin, always_user_icon) || wwin->net_icon_image))
+	if (wwin && WFLAGP(wwin, always_user_icon))
 		goto user_icon;
 
-	/* use client specified icon window */
 	if (icon->icon_win != None) {
+		/* use client specified icon window */
 		XWindowAttributes attr;
 		int resize = 0;
 		unsigned int width, height, depth;
@@ -626,6 +617,10 @@ void wIconUpdate(WIcon * icon)
 						  None, wCursor[WCUR_ARROW]);
 			}
 		}
+	} else if (wwin && wwin->net_icon_image) {
+		/* Use _NET_WM_ICON icon */
+		icon->pixmap = makeIcon(scr, wwin->net_icon_image, icon->show_title,
+						icon->shadowed, icon->tile_type, icon->highlighted);
 	} else if (wwin && wwin->wm_hints && (wwin->wm_hints->flags & IconPixmapHint)) {
 		int x, y;
 		unsigned int w, h;
@@ -685,8 +680,8 @@ void wIconUpdate(WIcon * icon)
 	} else {
  user_icon:
 
-		if (icon->image) {
-			icon->pixmap = makeIcon(scr, icon->image, icon->show_title,
+		if (icon->file_image) {
+			icon->pixmap = makeIcon(scr, icon->file_image, icon->show_title,
 						icon->shadowed, icon->tile_type, icon->highlighted);
 		} else {
 			/* make default icons */

src/icon.h

@@ -39,7 +39,7 @@ typedef struct WIcon {
     Window 		icon_win;      /* client suplied icon window */
 
     char		*file;	       /* the file with the icon image */
-    RImage 		*image;
+    RImage 		*file_image;   /* the image from the file */
 
     unsigned int 	tile_type:4;
     unsigned int 	show_title:1;
@@ -63,7 +63,6 @@ void wIconDestroy(WIcon *icon);
 void wIconPaint(WIcon *icon);
 void wIconUpdate(WIcon *icon);
 void wIconChangeTitle(WIcon *icon, char *new_title);
-void wIconChangeImage(WIcon *icon, RImage *new_image);
 Bool wIconChangeImageFile(WIcon *icon, char *file);
 void wIconSelect(WIcon *icon);