RESOLVED FIXED 108035
[EFL][WK2] Rely more on C API in ewk_favicon_database 
https://bugs.webkit.org/show_bug.cgi?id=108035
Summary [EFL][WK2] Rely more on C API in ewk_favicon_database
Chris Dumez
Reported 2013-01-27 11:02:01 PST
As per Bug 107657, we should start using the C API internally to avoid violating layering. ewk_favicon_database still has a few calls to the internal C++ API.
Attachments
Patch (7.82 KB, patch)
2013-01-27 11:15 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (7.83 KB, patch)
2013-01-27 11:24 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2013-01-27 11:15:37 PST
Created attachment 184917 [details] Patch
Chris Dumez
Comment 2 2013-01-27 11:24:21 PST
Created attachment 184918 [details] Patch
Benjamin Poulain
Comment 3 2013-01-27 16:15:00 PST
Comment on attachment 184918 [details] Patch I am not entirely convinced this is necessary. For port specific API, there is no need to force a C API between public APIs and Proxys (in my opinion). If GTK were to use the new, WKIconDatabaseTryGetCairoSurfaceForURL, then this would be helping consolidate WebKit2. I r+ because the patch seems correct and you handle your port abstractions as you see fit.
Chris Dumez
Comment 4 2013-01-27 23:20:36 PST
Comment on attachment 184918 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184918&action=review > Source/WebKit2/UIProcess/API/C/cairo/WKIconDatabaseCairo.h:34 > +WK_EXPORT cairo_surface_t* WKIconDatabaseTryGetCairoSurfaceForURL(WKIconDatabaseRef iconDatabase, WKURLRef url); Kenneth, I'm having doubts about this API now. Should we have an API that returns an Evas Object instead of a cairo surface? This makes more sense for EFL port and we really shouldn't expose cairo surfaces to applications.
Kenneth Rohde Christiansen
Comment 5 2013-01-28 00:32:25 PST
Comment on attachment 184918 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184918&action=review >> Source/WebKit2/UIProcess/API/C/cairo/WKIconDatabaseCairo.h:34 >> +WK_EXPORT cairo_surface_t* WKIconDatabaseTryGetCairoSurfaceForURL(WKIconDatabaseRef iconDatabase, WKURLRef url); > > Kenneth, I'm having doubts about this API now. Should we have an API that returns an Evas Object instead of a cairo surface? This makes more sense for EFL port and we really shouldn't expose cairo surfaces to applications. In the EFL API it might make more sense to return an Evas Object, but our C API is slightly lower level so returning a cairo_surface_t seems fine, especially for special browser cases or so where the browser/client might want to store the images locally or so.
Chris Dumez
Comment 6 2013-01-28 00:35:43 PST
(In reply to comment #5) > (From update of attachment 184918 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=184918&action=review > > >> Source/WebKit2/UIProcess/API/C/cairo/WKIconDatabaseCairo.h:34 > >> +WK_EXPORT cairo_surface_t* WKIconDatabaseTryGetCairoSurfaceForURL(WKIconDatabaseRef iconDatabase, WKURLRef url); > > > > Kenneth, I'm having doubts about this API now. Should we have an API that returns an Evas Object instead of a cairo surface? This makes more sense for EFL port and we really shouldn't expose cairo surfaces to applications. > > In the EFL API it might make more sense to return an Evas Object, but our C API is slightly lower level so returning a cairo_surface_t seems fine, especially for special browser cases or so where the browser/client might want to store the images locally or so. Ok, thanks for confirming. Let's land this then.
WebKit Review Bot
Comment 7 2013-01-28 01:04:03 PST
Comment on attachment 184918 [details] Patch Clearing flags on attachment: 184918 Committed r140952: <http://trac.webkit.org/changeset/140952>
WebKit Review Bot
Comment 8 2013-01-28 01:04:09 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.