[webkit-reviews] review requested: [Bug 98063] [GTK] [WK2] Add favicon support to the MiniBrowser : [Attachment 166669] Patch v2

[bugzilla-daemon at webkit.org]

Alberto Garcia <agarcia at igalia.com> has asked  for review:
Bug 98063: [GTK] [WK2] Add favicon support to the MiniBrowser
https://bugs.webkit.org/show_bug.cgi?id=98063

Attachment 166669: Patch v2
https://bugs.webkit.org/attachment.cgi?id=166669&action=review

------- Additional Comments from Alberto Garcia <agarcia at igalia.com>
(In reply to comment #2)
> I guess you should also unref the favicon pixbuf in finalize().

> Use a single line here:
> cairo_surface_t *surface = webkit_web_view_get_favicon(window->webView);

> > +	 g_signal_connect(window->uriEntry, "notify::text",
G_CALLBACK(uriEntryTextChanged), (gpointer)window);
> Don't use the gpointer cast, it's not needed

Ok, here's the new patch with these three changes.

I also decided not to clear the icon when the URI entry is changed. So
for this to work correctly, WebKitWebView has to emit notify::favicon
also when a new page is loaded and it has no favicon.

The emission of that signal need anyway to be changed since it's not
being emitted in most cases at the moment, but it should not affect
the MiniBrowser code.