Bug 117156 - [GTK] Migrate WebKitFaviconDatabase to GTask
Summary: [GTK] Migrate WebKitFaviconDatabase to GTask
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 117149
  Show dependency treegraph
 
Reported: 2013-06-03 09:09 PDT by Carlos Garcia Campos
Modified: 2013-06-19 00:25 PDT (History)
5 users (show)

See Also:


Attachments
Patch (8.95 KB, patch)
2013-06-03 09:12 PDT, Carlos Garcia Campos
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-06-03 09:09:20 PDT
Migrate WebKitFaviconDatabase to GTask
Comment 1 Carlos Garcia Campos 2013-06-03 09:12:44 PDT
Created attachment 203601 [details]
Patch
Comment 2 WebKit Commit Bot 2013-06-03 09:14:12 PDT
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
Comment 3 Gustavo Noronha (kov) 2013-06-17 20:44:27 PDT
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.
Comment 4 Carlos Garcia Campos 2013-06-18 00:00:53 PDT
(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.
Comment 5 Gustavo Noronha (kov) 2013-06-18 05:29:55 PDT
Comment on attachment 203601 [details]
Patch

OK, yay GTask doing the right thing =)
Comment 6 Carlos Garcia Campos 2013-06-19 00:25:16 PDT
Committed r151725: <http://trac.webkit.org/changeset/151725>