Bug 117156 – [GTK] Migrate WebKitFaviconDatabase to GTask

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

Migrate WebKitFaviconDatabase to GTask

[Carlos Garcia Campos]

Created attachment 203601 [details]
Patch

[WebKit Commit Bot]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Gustavo Noronha (kov)]

Comment on attachment 203601 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:-172
> -        if (!g_cancellable_is_cancelled(data->cancellable.get())) {

I just couldn't figure out why you're no longer checking for cancellation here, the rest looks good to me.

[Carlos Garcia Campos]

(In reply to comment #3)
> (From update of attachment 203601 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=203601&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitFaviconDatabase.cpp:-172
> > -        if (!g_cancellable_is_cancelled(data->cancellable.get())) {
> 
> I just couldn't figure out why you're no longer checking for cancellation here, the rest looks good to me.

That was to make sure that cancelled error had precedence over any other error, so we just completed the operation with g_simple_async_result_complete(). With GTask cancellation always has precedence (when check_cancellable is TRUE, which is the default), because all g_task_propagate methods first check if task was cancelled, so here we ignore the cancelled and complete the operation either with g_task_return_error or g_task_return_boolean, in both cases cancellation will be handled in the finish method when calling g_task_propagate_boolean.

[Gustavo Noronha (kov)]

Comment on attachment 203601 [details]
Patch

OK, yay GTask doing the right thing =)

[Carlos Garcia Campos]

Committed r151725: <http://trac.webkit.org/changeset/151725>