[webkit-reviews] review granted: [Bug 96477] [WK2][GTK] Add API to get the favicon for a WebKitWebView : [Attachment 165591] Patch proposal + Unit test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 26 02:12:20 PDT 2012


Carlos Garcia Campos <cgarcia at igalia.com> has granted Mario Sanchez Prada
<msanchez at igalia.com>'s request for review:
Bug 96477: [WK2][GTK] Add API to get the favicon for a WebKitWebView
https://bugs.webkit.org/show_bug.cgi?id=96477

Attachment 165591: Patch proposal + Unit test
https://bugs.webkit.org/attachment.cgi?id=165591&action=review

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=165591&action=review


Patch looks great, the unit tests part will probably need to be updated to the
changes in bug #96476.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:241
> +    iconFromWebView = webkit_web_view_get_favicon(test->m_webView.get());
> +    g_assert(iconFromWebView);
> +    cairo_surface_destroy(iconFromWebView);

I could check the icon contents, or simply that
cairo_image_surface_get_width/height are > 0

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:272
> +    FaviconDatabaseTest::add("WebKitFaviconDatabase",
"get-favicon-from-webview", testGetFaviconFromWebView);

This could be FaviconDatabaseTest::add("WebKitWebView", "favicon",
testWebViewFavicon); since wew are testing web view api even if it's run in the
favicon database test. We do that in other tests.


More information about the webkit-reviews mailing list