GNOME Bugzilla – Bug 648653
Replace EphyFaviconCache by WebKit's icon database cache
Last modified: 2012-03-20 11:17:26 UTC
See webkit's https://bugs.webkit.org/show_bug.cgi?id=56200. Basically WebKit maintains its own favicon database, so instead of using our own better use the one provided by WebKit. Interestingly in order to use the webkit icon database cache we must ask it to keep the icons we are interested in, otherwise the favicon cache will delete them.
Created attachment 186627 [details] [review] Patch
Created attachment 187201 [details] [review] Patch New version updated to the latest changes in the WebKit patch
See https://bugs.webkit.org/show_bug.cgi?id=56200 Xan has patches for the new API.
(In reply to comment #3) > See https://bugs.webkit.org/show_bug.cgi?id=56200 > > Xan has patches for the new API. Xan could you post here your patches?
Created attachment 209148 [details] [review] Patch new version against latest wk patch. Created against the history-rewrite branch.
Review of attachment 209148 [details] [review]: This looks great in general, I only have a few small comments. ::: embed/ephy-web-view.c @@ +1521,2 @@ + uri = webkit_web_view_get_uri (WEBKIT_WEB_VIEW (view)); + priv->icon = webkit_icon_database_get_icon_pixbuf (webkit_get_icon_database (), uri, 16, 16); Can we define the 16 x 16 thing somewhere? We are using it in a bunch of separate places (before it was contained in the favicon cache). Also, we really need to solve the issue of who is caching the scaled version, I think someone has to. ::: src/bookmarks/ephy-bookmark-action.c @@ +76,1 @@ + /* TODO: review, where does the icon come from ? */ This just updates the icon URI, the actual favicon is retrieved in the _sync method. ::: src/ephy-completion-model.c @@ +176,3 @@ + GdkPixbuf *favicon = webkit_icon_database_get_icon_pixbuf_finish (webkit_get_icon_database (), result, NULL); + if (!favicon) + goto frees; Well, probably can just do if (favicon) { and save a goto here :) @@ +217,3 @@ + } + + data = g_slice_new(IconLoadData); space after new.
Created attachment 209192 [details] WK favicons
Comment on attachment 209192 [details] WK favicons Sorry wrong patch
Created attachment 209244 [details] [review] Replace EphyFaviconCache by WebKit's icon database cache
Created attachment 209938 [details] [review] Replace EphyFaviconCache by WebKit's icon database cache
Comment on attachment 209244 [details] [review] Replace EphyFaviconCache by WebKit's icon database cache obsoleting
Review of attachment 209938 [details] [review]: I think this is good to go. Remember to remove the references to the files in doc/reference too, otherwise distcheck will break.
Pushed to master, closing the bug.