There's a couple of issues with the new progress and load-status signals of the WebView: - A weird 10% progress is reported for pages before the status even reaches PROVISIONAL. Not sure where this comes from, but it should either go away or come after PROVISIONAL or COMMITTED I think. - FINISHED comes before 100% progress is reported. I believe the natural expectation would be to receive 100% while the status is still not finished, and *then* receive FINISHED. You can see both issues with the attached testcase.
Created attachment 31294 [details] testprogress.c A simple testcase. Output is like this: ** Message: Progress is 0.100000 ** Message: Status is WEBKIT_LOAD_PROVISIONAL ** Message: Status is WEBKIT_LOAD_COMMITTED ** Message: Progress is 0.500000 ** Message: Progress is 0.669581 ** Message: Progress is 0.698058 ** Message: Progress is 0.718557 ** Message: Progress is 0.746067 ** Message: Progress is 0.767503 ** Message: Progress is 0.791314 ** Message: Progress is 0.814049 ** Message: Progress is 0.816215 ** Message: Status is WEBKIT_LOAD_FINISHED ** Message: Progress is 1.000000
Created attachment 31594 [details] Initial test case for loading ChangeLog | 8 +++ GNUmakefile.am | 6 ++ WebKit/gtk/ChangeLog | 12 ++++ WebKit/gtk/tests/testloading.c | 117 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 0 deletions(-)
Comment on attachment 31594 [details] Initial test case for loading An initial test case for the loading API. We will want to also cover the progress here, in a future patch.
Comment on attachment 31594 [details] Initial test case for loading > index 802a56b..34d5ced 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,11 @@ > +2009-06-20 Gustavo Noronha Silva <gns@gnome.org> > + > + Reviewed by NOBODY (OOPS!). > + > + Initial test case for loading. Need to be more descriptive here. > +static void > +load_finished_cb(WebKitWebView* web_view, WebKitWebFrame* web_frame, gpointer data) > +{ Please put this in one line to match the other tests. > + return g_test_run (); remove spaces. r=me with these changes.
Landed in r44898.
Not really fixed.
Comment on attachment 31594 [details] Initial test case for loading Clearing flag so it will not appear in the commit queue.
Created attachment 33975 [details] Test case for FIRST_VISUALLY_NON_EMPTY_LAYOUT after FINISHED This is a test case for another issue I've found in load_status system. When loading more than one document at the same time, sometimes, for some of the documents, WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT status is reported after WEBKIT_LOAD_FINISHED. The test program receives gnome bugs numbers. $ ./test 102382 105129 106872 ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is WEBKIT_LOAD_COMMITTED ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is WEBKIT_LOAD_COMMITTED ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is WEBKIT_LOAD_FINISHED ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is WEBKIT_LOAD_COMMITTED ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is WEBKIT_LOAD_FINISHED ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is WEBKIT_LOAD_FINISHED In this case for bug 105129, FINISHED is sent before FIRST_VISUALLY_NON_EMPTY_LAYOUT
(In reply to comment #0) > - A weird 10% progress is reported for pages before the status even reaches > PROVISIONAL. Not sure where this comes from, but it should either go away or > come after PROVISIONAL or COMMITTED I think. > This is wanted behavior. See WebCore/loader/ProgressTracker.cpp. The current implementation clamps progress values to always be inside [0.1, 0.9] during an active load, so that you can actually see that something is happening.
> - A weird 10% progress is reported for pages before the status even reaches > PROVISIONAL. Not sure where this comes from, but it should either go away or > come after PROVISIONAL or COMMITTED I think. Arguably a provisional load is already doing *something*, this comes down to what you perceive as progress. A user may be confused when he hits return and the progress bar is empty because it takes ages to obtain a response from the remote end. So I don't really think it's wrong. > - FINISHED comes before 100% progress is reported. I believe the natural > expectation would be to receive 100% while the status is still not finished, > and *then* receive FINISHED. The FrameLoadClientGtk relies on FrameLoader. And FrameLoader actually has guards to catch this case, so it looks like a bug in WebCore. I mostly hacked the Gtk bits so I can't easily say where to look. I guess this needs some looking at debugging output.
(In reply to comment #10) > > - FINISHED comes before 100% progress is reported. I believe the natural > > expectation would be to receive 100% while the status is still not finished, > > and *then* receive FINISHED. > > The FrameLoadClientGtk relies on FrameLoader. And FrameLoader actually has > guards to catch this case, so it looks like a bug in WebCore. I mostly hacked > the Gtk bits so I can't easily say where to look. I guess this needs some > looking at debugging output. It does not seem to be a bug but a deliberate decision. This patch fixes it: diff --git a/WebCore/loader/FrameLoader.cpp b/WebCore/loader/FrameLoader.cpp index 4530c11..f187b5d 100644 --- a/WebCore/loader/FrameLoader.cpp +++ b/WebCore/loader/FrameLoader.cpp @@ -3386,13 +3386,14 @@ void FrameLoader::checkLoadCompleteForThisFrame() #ifndef NDEBUG m_didDispatchDidCommitLoad = false; #endif + if (Page* page = m_frame->page()) + page->progress()->progressCompleted(m_frame); + if (!error.isNull()) m_client->dispatchDidFailLoad(error); else m_client->dispatchDidFinishLoad(); - if (Page* page = m_frame->page()) - page->progress()->progressCompleted(m_frame); return; } I guess I'll poke someone to see if it would be reasonable to change this.
(In reply to comment #11) > I guess I'll poke someone to see if it would be reasonable to change this. I opened a bug for this, https://bugs.webkit.org/show_bug.cgi?id=28851
(In reply to comment #8) > Created an attachment (id=33975) [details] > Test case for FIRST_VISUALLY_NON_EMPTY_LAYOUT after FINISHED > > This is a test case for another issue I've found in load_status system. When > loading more than one document at the same time, sometimes, for some of the > documents, WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT status is reported after > WEBKIT_LOAD_FINISHED. The test program receives gnome bugs numbers. > > $ ./test 102382 105129 106872 > ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL > ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL > ** Message: Status for (null) is WEBKIT_LOAD_PROVISIONAL > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is > WEBKIT_LOAD_COMMITTED > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is > WEBKIT_LOAD_COMMITTED > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is > WEBKIT_LOAD_FINISHED > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is > WEBKIT_LOAD_COMMITTED > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is > WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is > WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=105129 is > WEBKIT_LOAD_FIRST_VISUALLY_NON_EMPTY_LAYOUT > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=106872 is > WEBKIT_LOAD_FINISHED > ** Message: Status for http://bugzilla.gnome.org/show_bug.cgi?id=102382 is > WEBKIT_LOAD_FINISHED > > In this case for bug 105129, FINISHED is sent before > FIRST_VISUALLY_NON_EMPTY_LAYOUT So, after looking a bit at the code in FrameView.cpp I'm not sure there's anything wrong with this. It simple means we received all the data before WebCore had a chance to do the first visually non empty layout, so the order of the notifications makes sense I'd say. We should probably add a note to the docs and adapt Epiphany to better take this into account.
(In reply to comment #13) > So, after looking a bit at the code in FrameView.cpp I'm not sure there's > anything wrong with this. It simple means we received all the data before > WebCore had a chance to do the first visually non empty layout, so the order of > the notifications makes sense I'd say. We should probably add a note to the > docs and adapt Epiphany to better take this into account. I asked on IRC and aroben confirmed that yes, this is perfectly OK. Since the 10% stuff is also intentional and I have opened a different bug for the DONE/100% ordering issue, I'll close this bug now.