Merge lp:~midori/midori/colorful_tabs_scale_fix into lp:midori

[Paweł Forysiuk]

Handle gdk_pixbuf_scale_simple failure in colorfull tabs

[Cris Dywan (kalikiana)]

Do you have any idea what causes the failure in such cases?

Maybe it would make sense to avoid scaling altogether and instead manually locating a color in a defined way. As a side effect we'd have more control over corner cases.

review: Needs Information

[Paweł Forysiuk (tuxator)]

Not sure, i guess i can attach a backtrace later on (i dont have it on this machine).

I reproduced this issuse on two different machines, one with fedora one with ubuntu.

In case of fedora crash it would crash when trying to scale icon for gitarzysci.pl webpage.
It was a gif (not sure if animated i think not) and i did not found an easy way to check the type of pixmap before
attempting to scale it.

On ubuntu it was some other site i dont remember. Documentations says that scale may fail when
there is not enough memory to do it. It could also be the case that the favicon in the database is corrupted since when using such fix on ubuntu the offending website had no favicon displayed on the tab.

[gue5t gue5t (gue5t)]

Scaling an icon down to 1x1px shouldn't take a significant amount of memory, so I doubt that's the problem here. That said, it seems fine to check for NULL since the function is documented to potentially return NULL in OOM situations.

This may be related to the fact that on current trunk (as a result of the recent tab icon scaling commit), different parts of midori-view.c disagree about whether view->icon is a GdkPixbuf or a GIcon (midori_view_get_icon returns the GIcon pointer field as a GdkPixbuf pointer), so we have type errors. I'll write a patch for that when I get a chance to.

[gue5t gue5t (gue5t)]

Specifically, we downcast a GIcon (which is an interface implemented by both GdkPixbuf and GThemedIcon) to a GdkPixbuf in several places in midori-view.c where it isn't always true that the icon actually is a GdkPixbuf—there's no pixbuf underlying GThemedIcons. This seems likely to be what's happening here; if so, I'd expect to see failed GDK_IS_PIXBUF assertions when this issue occurs.

[Paweł Forysiuk (tuxator)]

Yeah i saw such assertions. To be fair i did not investigate this enough. It was not breaking with release version so it all makes sense imho.

[Cris Dywan (kalikiana)]

Let's have it in any case.

review: Approve

=== modified file 'extensions/colorful-tabs.c'
--- extensions/colorful-tabs.c	2013-10-30 00:19:27 +0000
+++ extensions/colorful-tabs.c	2016-02-24 21:50:29 +0000
@@ -74,6 +74,10 @@
     guchar* pixels;
 
     newpix = gdk_pixbuf_scale_simple (icon, 1, 1, GDK_INTERP_BILINEAR);
+
+    // Sometimes gdk_pixbuf_scale may fail, return gracefully
+    g_return_if_fail (newpix != NULL);
+    
     pixels = gdk_pixbuf_get_pixels (newpix);
     color->red = pixels[0] * 255;
     color->green = pixels[1] * 255;