Bug 66243 – [GTK] Change webview API tests to use "load-status" signal instead of "load-progress"

• Component: New Bugs
• Status: RESOLVED FIXED

[Caio Marcelo de Oliveira Filho]

[GTK] Change webview API tests to use "load-status" signal instead of "load-progress"

[Caio Marcelo de Oliveira Filho]

Created attachment 103935 [details]
Patch

[Martin Robinson]

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)

[Caio Marcelo de Oliveira Filho]

Created attachment 104338 [details]
Patch

[Caio Marcelo de Oliveira Filho]

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.

[Martin Robinson]

Comment on attachment 104338 [details]
Patch

Thank you!

[Caio Marcelo de Oliveira Filho]

Committed r93311: <http://trac.webkit.org/changeset/93311>