Summary: | [GTK] WebKitWebFrame's load-status is not properly notified to the tests | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | abarth, commit-queue, eric, mrobinson, webkit.review.bot | ||||||||||||||
Priority: | P3 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | PC | ||||||||||||||||
OS: | Linux | ||||||||||||||||
Attachments: |
|
Description
Sergio Villar Senin
2010-10-21 01:57:49 PDT
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
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.
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. 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? Comment on attachment 71417 [details]
DRT fixes and refactorings
Indeed, we can better do that in the object_connect already present there
Created attachment 71584 [details]
DRT fixes and refactorings
moved signal connection to createWebView
Comment on attachment 71584 [details]
DRT fixes and refactorings
Great!
Comment on attachment 71584 [details]
DRT fixes and refactorings
Clearing commit-queue flag for now.
Created attachment 72006 [details]
Fix frame-created signal emission
frame-created must be emitted before loading any data on the frame.
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
Comment on attachment 71415 [details]
New provisional-load-fail status signal.
Flipping the bit on this one, because of the API break.
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. Comment on attachment 72006 [details]
Fix frame-created signal emission
As discussed via Jabber, I believe this change should be combined into one patch.
Created attachment 72156 [details]
Fix frame-created issuing. load-status refactoring and fixes
Comment on attachment 72156 [details]
Fix frame-created issuing. load-status refactoring and fixes
Thanks!
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 ap@webkit.org. The commit-queue is continuing to process your patch. 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> All reviewed patches have been landed. Closing bug. 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 |