[EFL][WK2] Rely more on C API in ewk_favicon_database

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 184917 [details]
Patch

[Chris Dumez]

Created attachment 184918 [details]
Patch

[Benjamin Poulain]

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 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 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]

(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 on attachment 184918 [details]
Patch

Clearing flags on attachment: 184918

Committed r140952: <http://trac.webkit.org/changeset/140952>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.