[Webkit-unassigned] [Bug 50623] [GTK] Use gtk_icon_set_render_icon() to render icons in RenderThemeGtk

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 8 05:19:48 PST 2010


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
  Attachment #75890|review?                     |review-
               Flag|                            |

--- Comment #12 from Martin Robinson <mrobinson at webkit.org>  2010-12-08 05:19:49 PST ---
(From update of attachment 75890)
View in context: https://bugs.webkit.org/attachment.cgi?id=75890&action=review

Looking good!

> WebCore/platform/gtk/RenderThemeGtk.cpp:73
> +    PlatformRefPtr<GdkPixbuf> icon = gtk_icon_set_render_icon(iconSet, style, direction, state, iconSize, 0, 0);

If gtk_icon_set_render_icon returns a new reference this should be wrapped in adoptPlatformRef. If it doesn't, this can just be a raw GdkPixbuf* since we don't need to take a reference.

> WebCore/platform/gtk/RenderThemeGtk.cpp:102
> +    if (!iconsInitialized) {

Please use an early return here.

> WebCore/platform/gtk/RenderThemeGtk.cpp:103
> +        GtkIconFactory* iconFactory = gtk_icon_factory_new();

This is a situation where a PlatformRefPtr with adoptPlatformRef would make sense.

> WebCore/platform/gtk/RenderThemeGtk.cpp:480
> +        return IntPoint(rect.x(), rect.y());

This should just be rect.topLeft()

> WebCore/platform/gtk/RenderThemeGtk.h:175
> +    bool paintMediaButton(RenderObject*, GraphicsContext*, const IntRect&, const char* iconName, Color panelColor, int mediaIconSize);

It looks like panelColor is always m_panelColor and mediaIconSize is always m_mediaIconSize, so you can remove these parameters.

Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

More information about the webkit-unassigned mailing list