Our API is a bit messy where load status is concerned, specially since error pages were introduced, because they cause their own load statuses to be emitted. I have a couple of patches to propose, to fix these issues.
Created attachment 46215 [details] do not call committedLoad on finishedLoading I made sure tests pass, unit tests pass, and Epiphany behaviour stays the same. This, along with the second patch will allow us to remove a number of work-arounds from Epiphany, though.
Attachment 46215 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:844: Declaration has space between type name and * in FrameLoader *loader [whitespace/declaration] [3] Total errors found: 1
Created attachment 46216 [details] avoid emitting load signals for error pages loads ChangeLog entry explains why. We can remove most of the workarounds we currently have in ephy with this.
Created attachment 46217 [details] do not call committedLoad on finishedLoading
Attachment 46217 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:149: Declaration has space between type name and * in FrameLoader *frameLoader [whitespace/declaration] [3] WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:844: Declaration has space between type name and * in FrameLoader *loader [whitespace/declaration] [3] Total errors found: 2
Why does this patch intentionally move the *? - FrameLoader* frameLoader = loader->frameLoader(); + FrameLoader *frameLoader = loader->frameLoader(); That seems like a violation of the style guide...
(In reply to comment #6) > Why does this patch intentionally move the *? > > - FrameLoader* frameLoader = loader->frameLoader(); > + FrameLoader *frameLoader = loader->frameLoader(); > > That seems like a violation of the style guide... Yeah, I saw the warning and was apparently too sleepy to make the correct fix =);
Created attachment 46232 [details] do not call committedLoad on finishedLoading Hopefully this one has not style issues.
Comment on attachment 46232 [details] do not call committedLoad on finishedLoading OK.
Created attachment 46269 [details] avoid emitting load status signals when loading error pages Aha, that's what happens when you kill changes and have to reimplement them. The patch had an issue. This fixes it.
Created attachment 46271 [details] epiphany patch to remove workarounds This is what we can do in Ephy.
Comment on attachment 46232 [details] do not call committedLoad on finishedLoading Landed as r53075.
I ended up rolling back the committedLoad fix, since it caused a couple ASSERTS I cannot reproduce locally to be hit. I'll investigate a bit more.
The rollout does not seem to have stopped the breakage: http://build.webkit.org/results/GTK%20Linux%2032-bit%20Debug/r53085%20(2279)/plugins/iframe-shims-stderr.txt
The iframe shims test broke with your initial checkin: http://build.webkit.org/results/GTK%20Linux%2032-bit%20Debug/r53075%20(2271)/plugins/iframe-shims-stderr.txt
(In reply to comment #15) > The iframe shims test broke with your initial checkin: > http://build.webkit.org/results/GTK%20Linux%2032-bit%20Debug/r53075%20(2271)/plugins/iframe-shims-stderr.txt Yeah, it did look like. The revert did revert the relevant change, though, so I'm at a loss. Xan is getting us a clean rebuild in the debug bots to see if the assert goes away.
Comment on attachment 46232 [details] do not call committedLoad on finishedLoading Re-landed as r53138, after we figured the assert was not caused by this patch.
Comment on attachment 46269 [details] avoid emitting load status signals when loading error pages r=me
Last patch landed as r53294.