Bug 132519 - [EFL][WK2] Refactor favicon database APIs
Summary: [EFL][WK2] Refactor favicon database APIs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-02 22:45 PDT by Ryuan Choi
Modified: 2014-05-05 01:09 PDT (History)
6 users (show)

See Also:


Attachments
Patch (24.33 KB, patch)
2014-05-02 22:56 PDT, Ryuan Choi
gyuyoung.kim: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2014-05-02 22:45:55 PDT
There are two ways to get favicon database.

The first option is ewk_view_favicon_get with favicon,changed signal.
It always register the callback internally although applications does not want to use it.

The other option is ewk_favicon_database_icon_get with ewk_favicon_database_icon_change_callback_add API.
But callback don't know which icondatabase calls icon chages.

I will remove first option and improve ewk_favicon_database_icon_change_callback.
Comment 1 Ryuan Choi 2014-05-02 22:56:43 PDT
Created attachment 230746 [details]
Patch
Comment 2 Gyuyoung Kim 2014-05-02 23:22:50 PDT
Comment on attachment 230746 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230746&action=review

> Source/WebKit2/ChangeLog:11
> +        It's simple to use but it always adds the callback to EwkView although applications does not use favicon.

I agree that we don't need to support two options to get favicon data. However, I don't think first option has big overhead to add favicon callback in application side. Anyway, I don't have pros to keep first way. So, r+ed.

> Tools/ChangeLog:8
> +        * MiniBrowser/efl/main.c: Use ewk_view_favicon_get instead of ewk_view_favicon_get.

Use ewk_view_favicon_get instead of ewk_view_favicon_get. => Use ewk_favicon_database_icon_get instead of ewk_view_favicon_get. ?

> Tools/MiniBrowser/efl/main.c:327
> +    Evas_Object* favicon = ewk_favicon_database_icon_get(database, url, evas_object_evas_get(ewk_view));

Looks wrong * place in Evas_Object* favicon
Comment 3 Ryuan Choi 2014-05-05 01:09:11 PDT
Committed r168265: <http://trac.webkit.org/changeset/168265>