Bug 66243 - [GTK] Change webview API tests to use "load-status" signal instead of "load-progress"
Summary: [GTK] Change webview API tests to use "load-status" signal instead of "load-p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Caio Marcelo de Oliveira Filho
URL:
Keywords:
Depends on: 28851
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-15 11:37 PDT by Caio Marcelo de Oliveira Filho
Modified: 2011-08-18 08:52 PDT (History)
2 users (show)

See Also:


Attachments
Patch (1.85 KB, patch)
2011-08-15 11:43 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
Patch (2.16 KB, patch)
2011-08-18 07:24 PDT, Caio Marcelo de Oliveira Filho
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Caio Marcelo de Oliveira Filho 2011-08-15 11:37:59 PDT
[GTK] Change webview API tests to use "load-status" signal instead of "load-progress"
Comment 1 Caio Marcelo de Oliveira Filho 2011-08-15 11:43:31 PDT
Created attachment 103935 [details]
Patch
Comment 2 Martin Robinson 2011-08-17 15:11:32 PDT
Comment on attachment 103935 [details]
Patch

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

> Source/WebKit/gtk/ChangeLog:10
> +        The idle_quit_loop_cb() already checks the "load-status" flag, so it should be
> +        enough to call it when the "load-status" change.
> +

I'm not sure I understand why this patch is an improvement. Perhaps you could expand a bit here.  It doesn't seem to remove any code. Does it make things more efficient or more correct somehow?

> Source/WebKit/gtk/tests/testwebview.c:170
> +    g_signal_connect(view, "notify::load-status", G_CALLBACK (idle_quit_loop_cb), NULL);

Do you mind also fixing the style issue here?  G_CALLBACK (idle_quit_loop_cb)  G_CALLBACK(idle_quit_loop_cb)
Comment 3 Caio Marcelo de Oliveira Filho 2011-08-18 07:24:36 PDT
Created attachment 104338 [details]
Patch
Comment 4 Caio Marcelo de Oliveira Filho 2011-08-18 07:31:30 PDT
Thanks for the review Martin.

(In reply to comment #2)
> I'm not sure I understand why this patch is an improvement. Perhaps you could expand a bit here.  It doesn't seem to remove any code. Does it make things more efficient or more correct somehow?

Improved with an explanation. It is related to bug 28851. Since it is using progress to check whether its load-status changed, I think it is also a good change to do even if bug 28851 doesn't land.

> Do you mind also fixing the style issue here?  G_CALLBACK (idle_quit_loop_cb)  G_CALLBACK(idle_quit_loop_cb)

Done.
Comment 5 Martin Robinson 2011-08-18 07:51:01 PDT
Comment on attachment 104338 [details]
Patch

Thank you!
Comment 6 Caio Marcelo de Oliveira Filho 2011-08-18 08:52:01 PDT
Committed r93311: <http://trac.webkit.org/changeset/93311>