[Webkit-unassigned] [Bug 61140] [GTK] Add HTML5 Notifications support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61140





--- Comment #36 from Gustavo Noronha (kov) <gns at gnome.org>  2011-07-07 09:32:23 PST ---
(From update of attachment 99453)
View in context: https://bugs.webkit.org/attachment.cgi?id=99453&action=review

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:144
> +static HashMap<KURL, GdkPixbuf* > pixbuf_cache;

This seems to only be used by this function? It would be better to make it be part of its scope, I believe.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:147
> +    g_assert(webkit_web_notification_is_html(self) == FALSE);

Any reason not to use ASSERT()? It should say !webkit_web_notification_is_html(self) instead of doing == FALSE.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:156
> +        SharedBuffer* icon_buffer = self->priv->coreNotification->iconData();
> +        GInputStream* icon_stream = g_memory_input_stream_new_from_data(icon_buffer->data(), icon_buffer->size(), NULL);
> +
> +        icon_pixbuf = gdk_pixbuf_new_from_stream(icon_stream, NULL, NULL);
> +        g_object_unref(icon_stream);

This is a bit awkward, how about using WebCore::Image::getGdkPixbuf? This way we also guarantee that the WebKit loaders are used. Also, how do you prune the pixbuf cache to avoid it growing infinitely?

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:162
> +}
> +/**

Missing newline.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:164
> + *

Missing @self here.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:169
> +const char* webkit_web_notification_get_url(WebKitWebNotification* self)

We standardized on 'uri' inside WebKitGTK+ instead of url.

> Source/WebKit/gtk/webkit/webkitwebnotification.cpp:234
> + * webkit_web_notification_get_replaceid:

replaceid is a bit awkward, should we call it replace_id?

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