Bug 48048 – [GTK] WebKitWebFrame's load-status is not properly notified to the tests

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

[Sergio Villar Senin]

There are several tests that do not show the expected results because the load status changes of inner frames are not properly handled in DRT. We're only currently notifying about the load status changes for the WebView, i.e, the main frame.

[Sergio Villar Senin]

Created attachment 71415 [details]
New provisional-load-fail status signal.

Added a new signal to be issued when provisional load fails

"frame-created" signal is also now issued before loading any data. Thus, any client will receive all the load related notifications properly

[Sergio Villar Senin]

Created attachment 71417 [details]
DRT fixes and refactorings

DRT now tracks the load-status signals of all the and not only the main frame.

Load status signal callbacks were also refactored in a single method.

[Martin Robinson]

Comment on attachment 71415 [details]
New provisional-load-fail status signal.

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

Looks good, though we need another nod from a GTK+ review for the API change.

> WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:596
> +

I think the extra line can be removed.

> WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1136
> +    notifyStatus(m_frame, WEBKIT_LOAD_PROVISIONAL_FAILED);
> +

Same here.

> WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1146
> +    if (m_loadingErrorPage)
> +        return;
> +
> +    notifyStatus(m_frame, WEBKIT_LOAD_FAILED);
> +

And here.

[Martin Robinson]

Comment on attachment 71417 [details]
DRT fixes and refactorings

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

> WebKitTools/DumpRenderTree/gtk/DumpRenderTree.cpp:1083
> +    // frame-created is not issued for main frame. That's why we must do this here
> +    g_signal_connect(mainFrame, "notify::load-status", G_CALLBACK(webFrameLoadStatusNotified), NULL);
>  

Is it possible to do this in createWebView?

[Sergio Villar Senin]

Comment on attachment 71417 [details]
DRT fixes and refactorings

Indeed, we can better do that in the object_connect already present there

[Sergio Villar Senin]

Created attachment 71584 [details]
DRT fixes and refactorings

moved signal connection to createWebView

[Martin Robinson]

Comment on attachment 71584 [details]
DRT fixes and refactorings

Great!

[Martin Robinson]

Comment on attachment 71584 [details]
DRT fixes and refactorings

Clearing commit-queue flag for now.

[Sergio Villar Senin]

Created attachment 72006 [details]
Fix frame-created signal emission

frame-created must be emitted before loading any data on the frame.

[Sergio Villar Senin]

Created attachment 72007 [details]
load-status refactoring and fix

load status changes are now handled in a single callback

With this patch we're also handling load-status signals for every frame created and not only the main one

[Martin Robinson]

Comment on attachment 71415 [details]
New provisional-load-fail status signal.

Flipping the bit on this one, because of the API break.

[Martin Robinson]

Xan pointed out smartly that adding a new load state breaks the API, which is precisely why I think we should move away from this in our API. Every time someone adds a new frame loader delegate method we cannot support it without breaking the API.

[Martin Robinson]

Comment on attachment 72006 [details]
Fix frame-created signal emission

As discussed via Jabber, I believe this change should be combined into one patch.

[Sergio Villar Senin]

Created attachment 72156 [details]
Fix frame-created issuing. load-status refactoring and fixes

[Martin Robinson]

Comment on attachment 72156 [details]
Fix frame-created issuing. load-status refactoring and fixes

Thanks!

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 72156 [details]:

http/tests/appcache/insert-html-element-with-manifest-2.html

Please file bugs against the tests.  These tests were authored by [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 72156 [details]
Fix frame-created issuing. load-status refactoring and fixes

Clearing flags on attachment: 72156

Committed r70788: <http://trac.webkit.org/changeset/70788>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

http://trac.webkit.org/changeset/70788 might have broken GTK Linux 32-bit Release
The following tests are not passing:
inspector/audits-panel-functional.html