Bug 30989 – [Gtk, WML] Assert failure in http://wap.google.com/wml

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

[Nayan]

Following assert is seen while browsing http://wap.google.com/wml.

(gdb) n
ASSERTION FAILED: this == frameLoader()->activeDocumentLoader()
(../../WebCore/loader/DocumentLoader.cpp:393 void WebCore::DocumentLoader::updateLoading())

Program received signal SIGSEGV, Segmentation fault.
0xb6b1b7b3 in WebCore::DocumentLoader::updateLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:393
393	    ASSERT(this == frameLoader()->activeDocumentLoader());


Build information:
Revision ID : 50380
Port : GTK port

Steps to reproduce:
1. Open GtkLauncher.
2. Fetch http://wap.google.com/wml.

Actual Result : GtkLauncher crashes with assert failure.
Expected Result : GtkLauncher should fetch and render the page properly.

[Nayan]

I did a initial analysis of the issue, and below are my findings.

1. Frameloader creates 'DocuementLoader' object when it receives the load request for 'http://wap.google.com/wml' page.

Breakpoint 1, DocumentLoader (this=0x8372400, req=@0xbf9d82ec, substituteData=@0xbf9d81e4) at ../../WebCore/loader/DocumentLoader.cpp:151
151	    , m_applicationCacheHost(new ApplicationCacheHost(this))
Current language:  auto; currently c++
(gdb) bt
#0  DocumentLoader (this=0x8372400, req=@0xbf9d82ec, substituteData=@0xbf9d81e4) at ../../WebCore/loader/DocumentLoader.cpp:151
#1  0xb65e4093 in DocumentLoader (this=0x8372400, request=@0xbf9d82ec, substituteData=@0xbf9d81e4) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.cpp:42
#2  0xb65f505c in WebKit::DocumentLoader::create (request=@0xbf9d82ec, data=@0xbf9d81e4) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.h:48
#3  0xb65f1180 in WebKit::FrameLoaderClient::createDocumentLoader (this=0x8073300, request=@0xbf9d82ec, substituteData=@0xbf9d81e4) at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:118
#4  0xb6b3b88f in WebCore::FrameLoader::load (this=0x8089c2c, request=@0xbf9d82ec, substituteData=@0xbf9d81e4, lockHistory=false) at ../../WebCore/loader/FrameLoader.cpp:1931
#5  0xb6b3ba16 in WebCore::FrameLoader::load (this=0x8089c2c, request=@0xbf9d82ec, lockHistory=false) at ../../WebCore/loader/FrameLoader.cpp:1921
#6  0xb6602001 in webkit_web_frame_load_uri (frame=0x809e9b0, uri=0x832b678 "http://wap.google.com/wml") at ../../WebKit/gtk/webkit/webkitwebframe.cpp:514
#7  0xb66111fa in webkit_web_view_load_uri (webView=0x80a1000, uri=0x832b678 "http://wap.google.com/wml") at ../../WebKit/gtk/webkit/webkitwebview.cpp:2973


2. Loader finishes loading of the network resource and sends it to WML parser for parsing. Since, this WML page contains 'Refresh' tag, reload request is sent to FrameLoader. Frameloader now creates one DocumentLoader object.

Breakpoint 1, DocumentLoader (this=0x8424760, req=@0xbf9d86a8, substituteData=@0xbf9d85c4) at ../../WebCore/loader/DocumentLoader.cpp:151
151	    , m_applicationCacheHost(new ApplicationCacheHost(this))
(gdb) bt
#0  DocumentLoader (this=0x8424760, req=@0xbf9d86a8, substituteData=@0xbf9d85c4) at ../../WebCore/loader/DocumentLoader.cpp:151
#1  0xb65e4093 in DocumentLoader (this=0x8424760, request=@0xbf9d86a8, substituteData=@0xbf9d85c4) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.cpp:42
#2  0xb65f505c in WebKit::DocumentLoader::create (request=@0xbf9d86a8, data=@0xbf9d85c4) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.h:48
#3  0xb65f1180 in WebKit::FrameLoaderClient::createDocumentLoader (this=0x8073300, request=@0xbf9d86a8, substituteData=@0xbf9d85c4) at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:118
#4  0xb6b3b2a4 in WebCore::FrameLoader::reload (this=0x8089c2c, endToEndReload=false) at ../../WebCore/loader/FrameLoader.cpp:2144
#5  0xb6e47cc5 in WebCore::WMLRefreshElement::executeTask (this=0x83ad0c8) at ../../WebCore/wml/WMLRefreshElement.cpp:70
#6  0xb6e42109 in WebCore::WMLIntrinsicEventHandler::triggerIntrinsicEvent (this=0x83ad118, type=WebCore::WMLIntrinsicEventOnEnterForward) at ../../WebCore/wml/WMLIntrinsicEventHandler.cpp:51
#7  0xb6e39dd4 in WebCore::WMLCardElement::handleIntrinsicEventIfNeeded (this=0x83a54c8) at ../../WebCore/wml/WMLCardElement.cpp:168
#8  0xb6e3b1d6 in WebCore::WMLDocument::finishedParsing (this=0x8405a18) at ../../WebCore/wml/WMLDocument.cpp:83
#9  0xb69a8d61 in WebCore::XMLTokenizer::end (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:218
#10 0xb69a8d99 in WebCore::XMLTokenizer::finish (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:226
#11 0xb691de83 in WebCore::Document::finishParsing (this=0x8405a18) at ../../WebCore/dom/Document.cpp:1878
#12 0xb6b3501e in WebCore::FrameLoader::endIfNotLoadingMainResource (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:949
#13 0xb6b35057 in WebCore::FrameLoader::end (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:934
#14 0xb6b1bd42 in WebCore::DocumentLoader::finishedLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:330
#15 0xb6b3161a in WebCore::FrameLoader::finishedLoading (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:2697
#16 0xb6b4a52e in WebCore::MainResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/MainResourceLoader.cpp:393
#17 0xb6b536a4 in WebCore::ResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/ResourceLoader.cpp:403
#18 0xb6fca74b in finishedCallback (session=0x809a000, msg=0x83bd218, data=0x841de98) at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:325
#19 0xb565bb81 in final_finished (req=0x83bd218, user_data=0x8111e90) at soup-session-async.c:381
#20 0xb595a73f in IA__g_cclosure_marshal_VOID__VOID (closure=0x83bb638, return_value=0x0, n_param_values=1, param_values=0x841da50, invocation_hint=0xbf9d8c2c, marshal_data=0xb565bab0) at gmarshal.c:77


3. For some reason, one more DocumentLoader object gets created, in process of executing 'Refresh' tag. Not really sure why 2 'DocumentLoader's gets created here!

Breakpoint 1, DocumentLoader (this=0x8408f58, req=@0xbf9d7d08, substituteData=@0xbf9d7c24) at ../../WebCore/loader/DocumentLoader.cpp:151
151	    , m_applicationCacheHost(new ApplicationCacheHost(this))
(gdb) bt
#0  DocumentLoader (this=0x8408f58, req=@0xbf9d7d08, substituteData=@0xbf9d7c24) at ../../WebCore/loader/DocumentLoader.cpp:151
#1  0xb65e4093 in DocumentLoader (this=0x8408f58, request=@0xbf9d7d08, substituteData=@0xbf9d7c24) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.cpp:42
#2  0xb65f505c in WebKit::DocumentLoader::create (request=@0xbf9d7d08, data=@0xbf9d7c24) at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.h:48
#3  0xb65f1180 in WebKit::FrameLoaderClient::createDocumentLoader (this=0x8073300, request=@0xbf9d7d08, substituteData=@0xbf9d7c24) at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:118
#4  0xb6b3b2a4 in WebCore::FrameLoader::reload (this=0x8089c2c, endToEndReload=false) at ../../WebCore/loader/FrameLoader.cpp:2144
#5  0xb6e47cc5 in WebCore::WMLRefreshElement::executeTask (this=0x83ad0c8) at ../../WebCore/wml/WMLRefreshElement.cpp:70
#6  0xb6e42109 in WebCore::WMLIntrinsicEventHandler::triggerIntrinsicEvent (this=0x83ad118, type=WebCore::WMLIntrinsicEventOnEnterForward) at ../../WebCore/wml/WMLIntrinsicEventHandler.cpp:51
#7  0xb6e39dd4 in WebCore::WMLCardElement::handleIntrinsicEventIfNeeded (this=0x83a54c8) at ../../WebCore/wml/WMLCardElement.cpp:168
#8  0xb6e3b1d6 in WebCore::WMLDocument::finishedParsing (this=0x8405a18) at ../../WebCore/wml/WMLDocument.cpp:83
#9  0xb69a8d61 in WebCore::XMLTokenizer::end (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:218
#10 0xb69a8d99 in WebCore::XMLTokenizer::finish (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:226
#11 0xb691de83 in WebCore::Document::finishParsing (this=0x8405a18) at ../../WebCore/dom/Document.cpp:1878
#12 0xb6b36a8a in WebCore::FrameLoader::stop (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:573
#13 0xb6b36b26 in WebCore::FrameLoader::receivedMainResourceError (this=0x8089c2c, error=@0xbf9d7fdc, isComplete=true) at ../../WebCore/loader/FrameLoader.cpp:3309
#14 0xb6b4aac2 in WebCore::MainResourceLoader::didCancel (this=0x8424108, error=@0xbf9d7fdc) at ../../WebCore/loader/MainResourceLoader.cpp:101
#15 0xb6b53ea8 in WebCore::ResourceLoader::cancel (this=0x8424108, error=@0xbf9d8020) at ../../WebCore/loader/ResourceLoader.cpp:350
#16 0xb6b53733 in WebCore::ResourceLoader::cancel (this=0x8424108) at ../../WebCore/loader/ResourceLoader.cpp:340
#17 0xb6b1c27a in WebCore::DocumentLoader::stopLoading (this=0x8372400, databasePolicy=WebCore::DatabasePolicyStop) at ../../WebCore/loader/DocumentLoader.cpp:294
#18 0xb6b30d37 in WebCore::FrameLoader::stopAllLoaders (this=0x8089c2c, databasePolicy=WebCore::DatabasePolicyStop) at ../../WebCore/loader/FrameLoader.cpp:2258
#19 0xb6b3ab1c in WebCore::FrameLoader::continueLoadAfterNavigationPolicy (this=0x8089c2c, formState=@0xbf9d8158, shouldContinue=true) at ../../WebCore/loader/FrameLoader.cpp:3447
#20 0xb6b3ace2 in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy (argument=0x8089c2c, request=@0xbf9d81e8, formState=@0xbf9d8198, shouldContinue=true)
    at ../../WebCore/loader/FrameLoader.cpp:3404
#21 0xb6b4d7cf in WebCore::PolicyCallback::call (this=0xbf9d81e8, shouldContinue=true) at ../../WebCore/loader/PolicyCallback.cpp:101
#22 0xb6b4e36f in WebCore::PolicyChecker::continueAfterNavigationPolicy (this=0x8089c34, policy=WebCore::PolicyUse) at ../../WebCore/loader/PolicyChecker.cpp:160
#23 0xb6606f28 in webkit_web_policy_decision_use (decision=0x842e240) at ../../WebKit/gtk/webkit/webkitwebpolicydecision.cpp:89
#24 0xb65f03f7 in WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction (this=0x8073300, 
    policyFunction=0xb6b4e18e <WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)>, action=@0xbf9d8470, resourceRequest=@0x84249dc)
    at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:420
#25 0xb6b4e8f1 in WebCore::PolicyChecker::checkNavigationPolicy (this=0x8089c34, request=@0x84249dc, loader=0x8424760, formState=@0xbf9d8568, 
    function=0xb6b3ac90 <WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>, argument=0x8089c2c)
    at ../../WebCore/loader/PolicyChecker.cpp:88
#26 0xb6b3b12e in WebCore::FrameLoader::loadWithDocumentLoader (this=0x8089c2c, loader=0x8424760, type=WebCore::FrameLoadTypeReload, prpFormState=@0xbf9d8698) at ../../WebCore/loader/FrameLoader.cpp:2039
#27 0xb6b3b42d in WebCore::FrameLoader::reload (this=0x8089c2c, endToEndReload=false) at ../../WebCore/loader/FrameLoader.cpp:2157
#28 0xb6e47cc5 in WebCore::WMLRefreshElement::executeTask (this=0x83ad0c8) at ../../WebCore/wml/WMLRefreshElement.cpp:70
#29 0xb6e42109 in WebCore::WMLIntrinsicEventHandler::triggerIntrinsicEvent (this=0x83ad118, type=WebCore::WMLIntrinsicEventOnEnterForward) at ../../WebCore/wml/WMLIntrinsicEventHandler.cpp:51
#30 0xb6e39dd4 in WebCore::WMLCardElement::handleIntrinsicEventIfNeeded (this=0x83a54c8) at ../../WebCore/wml/WMLCardElement.cpp:168
#31 0xb6e3b1d6 in WebCore::WMLDocument::finishedParsing (this=0x8405a18) at ../../WebCore/wml/WMLDocument.cpp:83
#32 0xb69a8d61 in WebCore::XMLTokenizer::end (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:218
#33 0xb69a8d99 in WebCore::XMLTokenizer::finish (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:226
#34 0xb691de83 in WebCore::Document::finishParsing (this=0x8405a18) at ../../WebCore/dom/Document.cpp:1878
#35 0xb6b3501e in WebCore::FrameLoader::endIfNotLoadingMainResource (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:949
#36 0xb6b35057 in WebCore::FrameLoader::end (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:934
#37 0xb6b1bd42 in WebCore::DocumentLoader::finishedLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:330
#38 0xb6b3161a in WebCore::FrameLoader::finishedLoading (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:2697
#39 0xb6b4a52e in WebCore::MainResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/MainResourceLoader.cpp:393
#40 0xb6b536a4 in WebCore::ResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/ResourceLoader.cpp:403
#41 0xb6fca74b in finishedCallback (session=0x809a000, msg=0x83bd218, data=0x841de98) at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:325
#42 0xb565bb81 in final_finished (req=0x83bd218, user_data=0x8111e90) at soup-session-async.c:381
#43 0xb595a73f in IA__g_cclosure_marshal_VOID__VOID (closure=0x83bb638, return_value=0x0, n_param_values=1, param_values=0x841da50, invocation_hint=0xbf9d8c2c, marshal_data=0xb565b


4. And then 'updateLoading' gets called.

Breakpoint 3, WebCore::DocumentLoader::updateLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:389
389	    if (!m_frame) {
(gdb) bt
#0  WebCore::DocumentLoader::updateLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:389
#1  0xb6b1b94f in WebCore::DocumentLoader::removeSubresourceLoader (this=0x8372400, loader=0x8373428) at ../../WebCore/loader/DocumentLoader.cpp:743
#2  0xb6b56208 in WebCore::SubresourceLoader::didCancel (this=0x8373428, error=@0xbf9d7f4c) at ../../WebCore/loader/SubresourceLoader.cpp:233
#3  0xb6b53ea8 in WebCore::ResourceLoader::cancel (this=0x8373428, error=@0xbf9d7f90) at ../../WebCore/loader/ResourceLoader.cpp:350
#4  0xb6b53733 in WebCore::ResourceLoader::cancel (this=0x8373428) at ../../WebCore/loader/ResourceLoader.cpp:340
#5  0xb6b1bdfa in cancelAll (loaders=@0x8372414) at ../../WebCore/loader/DocumentLoader.cpp:123
#6  0xb6b1be66 in WebCore::DocumentLoader::stopLoadingSubresources (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:731
#7  0xb6b1c325 in WebCore::DocumentLoader::stopLoading (this=0x8372400, databasePolicy=WebCore::DatabasePolicyStop) at ../../WebCore/loader/DocumentLoader.cpp:304
#8  0xb6b30d37 in WebCore::FrameLoader::stopAllLoaders (this=0x8089c2c, databasePolicy=WebCore::DatabasePolicyStop) at ../../WebCore/loader/FrameLoader.cpp:2258
#9  0xb6b3ab1c in WebCore::FrameLoader::continueLoadAfterNavigationPolicy (this=0x8089c2c, formState=@0xbf9d8158, shouldContinue=true) at ../../WebCore/loader/FrameLoader.cpp:3447
#10 0xb6b3ace2 in WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy (argument=0x8089c2c, request=@0xbf9d81e8, formState=@0xbf9d8198, shouldContinue=true)
    at ../../WebCore/loader/FrameLoader.cpp:3404
#11 0xb6b4d7cf in WebCore::PolicyCallback::call (this=0xbf9d81e8, shouldContinue=true) at ../../WebCore/loader/PolicyCallback.cpp:101
#12 0xb6b4e36f in WebCore::PolicyChecker::continueAfterNavigationPolicy (this=0x8089c34, policy=WebCore::PolicyUse) at ../../WebCore/loader/PolicyChecker.cpp:160
#13 0xb6606f28 in webkit_web_policy_decision_use (decision=0x842e240) at ../../WebKit/gtk/webkit/webkitwebpolicydecision.cpp:89
#14 0xb65f03f7 in WebKit::FrameLoaderClient::dispatchDecidePolicyForNavigationAction (this=0x8073300, 
    policyFunction=0xb6b4e18e <WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction)>, action=@0xbf9d8470, resourceRequest=@0x84249dc)
    at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:420
#15 0xb6b4e8f1 in WebCore::PolicyChecker::checkNavigationPolicy (this=0x8089c34, request=@0x84249dc, loader=0x8424760, formState=@0xbf9d8568, 
    function=0xb6b3ac90 <WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy(void*, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>, argument=0x8089c2c)
    at ../../WebCore/loader/PolicyChecker.cpp:88
#16 0xb6b3b12e in WebCore::FrameLoader::loadWithDocumentLoader (this=0x8089c2c, loader=0x8424760, type=WebCore::FrameLoadTypeReload, prpFormState=@0xbf9d8698) at ../../WebCore/loader/FrameLoader.cpp:2039
#17 0xb6b3b42d in WebCore::FrameLoader::reload (this=0x8089c2c, endToEndReload=false) at ../../WebCore/loader/FrameLoader.cpp:2157
#18 0xb6e47cc5 in WebCore::WMLRefreshElement::executeTask (this=0x83ad0c8) at ../../WebCore/wml/WMLRefreshElement.cpp:70
#19 0xb6e42109 in WebCore::WMLIntrinsicEventHandler::triggerIntrinsicEvent (this=0x83ad118, type=WebCore::WMLIntrinsicEventOnEnterForward) at ../../WebCore/wml/WMLIntrinsicEventHandler.cpp:51
#20 0xb6e39dd4 in WebCore::WMLCardElement::handleIntrinsicEventIfNeeded (this=0x83a54c8) at ../../WebCore/wml/WMLCardElement.cpp:168
#21 0xb6e3b1d6 in WebCore::WMLDocument::finishedParsing (this=0x8405a18) at ../../WebCore/wml/WMLDocument.cpp:83
#22 0xb69a8d61 in WebCore::XMLTokenizer::end (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:218
#23 0xb69a8d99 in WebCore::XMLTokenizer::finish (this=0x8385378) at ../../WebCore/dom/XMLTokenizer.cpp:226
#24 0xb691de83 in WebCore::Document::finishParsing (this=0x8405a18) at ../../WebCore/dom/Document.cpp:1878
#25 0xb6b3501e in WebCore::FrameLoader::endIfNotLoadingMainResource (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:949
#26 0xb6b35057 in WebCore::FrameLoader::end (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:934
#27 0xb6b1bd42 in WebCore::DocumentLoader::finishedLoading (this=0x8372400) at ../../WebCore/loader/DocumentLoader.cpp:330
#28 0xb6b3161a in WebCore::FrameLoader::finishedLoading (this=0x8089c2c) at ../../WebCore/loader/FrameLoader.cpp:2697
#29 0xb6b4a52e in WebCore::MainResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/MainResourceLoader.cpp:393
#30 0xb6b536a4 in WebCore::ResourceLoader::didFinishLoading (this=0x8424108) at ../../WebCore/loader/ResourceLoader.cpp:403
#31 0xb6fca74b in finishedCallback (session=0x809a000, msg=0x83bd218, data=0x841de98) at ../../WebCore/platform/network/soup/ResourceHandleSoup.cpp:325
#32 0xb565bb81 in final_finished (req=0x83bd218, user_data=0x8111e90) at soup-session-async.c:381


5. Assert in 'updateLoading' causes failure.

(gdb) n
393	    ASSERT(this == frameLoader()->activeDocumentLoader());
(gdb) print frameLoader()->activeDocumentLoader()
$1 = (class WebCore::DocumentLoader *) 0x8408f58
(gdb) n
ASSERTION FAILED: this == frameLoader()->activeDocumentLoader()
(../../WebCore/loader/DocumentLoader.cpp:393 void WebCore::DocumentLoader::updateLoading())

Note that, updateLoading gets called for the first DocumentLoader (0x8372400). But, frameLoader()->activeDocumentLoader returns 3rd DocumentLoader, hence causing the assert fail.

[Nayan]

This assert wasn't seen in builds older revision 46418. There was a redesign of History handling / page cache / loading in WML pages, got fixed with bug #27707. This fix uncovered an existing issue in Frameloader which was causing this asset.

Root cause of the issue:
When page load completes, WML checks if there are any intrinsic event which needs to be handled (WebCore::WMLCardElement::handleIntrinsicEventIfNeeded). It finds that WML pages contains Refresh tag and starts 'Reload' requests Loading process repeats and handleIntrinsicEventIfNeeded again gets called to check if there are any intrinsic event yet to be handled. Ideally, we should have trigger one more 'Reload' request, as reload is already done. But, in this case, handleIntrinsicEventIfNeeded used to trigger one more reload request. (Hence creating one more DocumentLoader, as mentioned in point 3 in previous post).

This is because, FrameLoader::continueLoadAfterNavigationPolicy function used to set m_loadType to FrameLoadTypeReload after the call to stopAllLoaders().

....
....
FrameLoadType type = policyChecker()->loadType();
stopAllLoaders();
...
...
m_loadType = type;
...

Meanwhile, stopAllLoaders() will in-turn call 'handleIntrinsicEventIfNeeded'. 'handleInstrinsicEventIfNeeded' will trigger the reload request based on load type, which is still FrameLoadTypeStandard. Ideally, we need to set m_loadType variable of FrameLoader class before calling stopAllLoaders().

[Nayan]

Created attachment 43353 [details]
Initial patch 

Initial patch with the possible fix for the issue. Also, raising review request for the patch.

[Nayan]

Please note that, I was getting compilation error in wml enabled build. Following changes in patch resolves those errors. Need inputs whether these changes are really needed, and if so, whether they need to integrated as part of a separate bug.

Also, when I run 'onenterforward-refresh.html' layout test, it gives me an error message,

FAIL: Timed out waiting for notifyDone to be called

I am sure, this has to do something with the way I created the layout test. Please help me to solve this error, this is my first contribution to WebKit. :)

[Nayan]

Sorry, forgot to mention the files modified to resolve the compilation errors in WML enabled builds. Below are those,

wml/WMLAElement.cpp
wml/WMLInputElement.cpp

[Nikolas Zimmermann]

Hey Nayan,

I checked your patch, and it looks fine in general, though as I'm not a real expert in FrameLoader I think we should delegate this to reviewers that know more about this. The WML Part of this patch is just fine.
Eric, whom would you suggest to review?

Cheers,
Niko

[Eric Seidel (no email)]

Adam has been in FrameLoader a lot lately.

[Adam Barth]

I'm willing to believe this is ok, but FrameLoader is immensely complicated.  Thanks for the test.  Let's give folks a couple days to comment in case someone else has more insight.

[Adam Barth]

Comment on attachment 43353 [details]
Initial patch 

This patch contains tabs.  Please replace them with spaces.  :)

[Nayan]

Created attachment 43512 [details]
Patch with comments incorporated.

Thanks Niko, Eric, Adam for your comments.

I have removed all tabs and replaced them with spaces (Culprit being my emacs :)), and attached is the updated patch. Please review and let me know if you have any further comments.

Also, as mentioned in earlier comments, when I run this newly created layout test (onenterforward-refresh.html), I get following error message. Please help me to resolve this error.

FAIL: Timed out waiting for notifyDone to be called

[Nikolas Zimmermann]

Hi Nayan,

hm, that timeout means, you're never finishing the test. It just timeouts. I'll see if I can test the patch later on today - WML layout tests are really tricky, given the fact that WML does NOT support JavaScript, and we're just "misusing" JavaScript to be able to make dynamic WML testcases. I bet the bug is in there.

Going to comment, once I have a WML build with your patch applied.

Have a nice day,
Niko

[Nayan]

(In reply to comment #11)
> Hi Nayan,
> 
> hm, that timeout means, you're never finishing the test. It just timeouts. I'll
> see if I can test the patch later on today - WML layout tests are really
> tricky, given the fact that WML does NOT support JavaScript, and we're just
> "misusing" JavaScript to be able to make dynamic WML testcases. I bet the bug
> is in there.
> 
> Going to comment, once I have a WML build with your patch applied.
> 
> Have a nice day,
> Niko

Thanks Niko.

[Nikolas Zimmermann]

Comment on attachment 43512 [details]
Patch with comments incorporated.

r- for the LayoutTest. I've tested your patch and it works fine, though the testcase is flawed.
The purpose of using "createStaticWMLTestCase" is to create a testcase which never leaves the target document (only switching cards is allowed, as the existing onenterforward-* tests demonstrate). You need to dynamically create the WML testcase, otherwhise testing refresh tasks is not possible (due the reloads of the whole document, no one ever calls notifyDone).

The "wml/script-tests/input-format.js" uses a refresh task, and demonstrates how to write tests for it. Can you please look into that to create a valid testcase?

[Nayan]

Created attachment 43777 [details]
Dynamically genarated layout test.

Thanks Niko for your hints on correcting the test case. Based on your input, I have created a dynamic WML test case to test this scenario. This new test case is attached with this bug. 

However, with this test case, GtkLauncher is not crashing without the patch for this issue. It means that, there are some flaws still remaining in this test case also. (Note that, crash is still reproducible with the wml page created with "createStaticWMLTestCase"). Please go through the patch and let me know if can figure out what is going wrong.

Thanks.

Regards,
Nayan

[Nayan]

Deal All,

I am still unable to figure out reason why my 'createDynamicWMLTestCase' doesn't actually create the expected WML page. Are there any bugs in 'createDynamicWMLTestCase'? How to debug it further? Is there a way to dump the output of Javascript interpreter? I assume, Javascript interpreter will covert .js file to either .html or .wml. If we dump the output of javascript interpreter, it may narrow down the problem.

Regards
Nayan

[Nayan]

Deal All,

I investigated more on layout test, I feel it is not possible to write the layout test for this use case. Even with the dynamically generated layout test (https://bug-30989-attachments.webkit.org/attachment.cgi?id=43777), GTKLauncher is not crashing. I debugged this layout test with the aid of JavaScript logger tool jslogging (http://www.alistapart.com/articles/jslogging) and found that all element nodes and attributes are set to the expected values.

Also, I debugged this test with gdb. Here, there is only one DocumentLoader getting created. (In case of crashes, 2 DocumentLoaders were created).

Breakpoint 2, DocumentLoader (this=0x8157c00, req=..., substituteData=...)
    at ../../WebCore/loader/DocumentLoader.cpp:151
151	    , m_applicationCacheHost(new ApplicationCacheHost(this))
(gdb) bt
#0  DocumentLoader (this=0x8157c00, req=..., substituteData=...)
    at ../../WebCore/loader/DocumentLoader.cpp:151
#1  0x00e62ffd in DocumentLoader (this=0x8157c00, request=..., substituteData=...)
    at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.cpp:42
#2  0x00e712e1 in WebKit::DocumentLoader::create (request=..., data=...)
    at ../../WebKit/gtk/WebCoreSupport/DocumentLoaderGtk.h:48
#3  0x00e6be2e in WebKit::FrameLoaderClient::createDocumentLoader (this=0xb6782ab0, request=..., 
    substituteData=...) at ../../WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:118
#4  0x013bdd0c in WebCore::FrameLoader::reload (this=0xb6782ba4, endToEndReload=false)
    at ../../WebCore/loader/FrameLoader.cpp:2175
#5  0x016e5e14 in WebCore::WMLRefreshElement::executeTask (this=0xb672a4f0)
    at ../../WebCore/wml/WMLRefreshElement.cpp:70
#6  0x016e018a in WebCore::WMLIntrinsicEventHandler::triggerIntrinsicEvent (this=0xb672a540, 
    type=WebCore::WMLIntrinsicEventOnEnterForward)
    at ../../WebCore/wml/WMLIntrinsicEventHandler.cpp:51
#7  0x016d7847 in WebCore::WMLCardElement::handleIntrinsicEventIfNeeded (this=0x8158cd8)
    at ../../WebCore/wml/WMLCardElement.cpp:168
#8  0x016d9464 in WebCore::WMLDocument::initialize (this=0x817e008, aboutToFinishParsing=false)
    at ../../WebCore/wml/WMLDocument.cpp:110
#9  0x011b375e in WebCore::Document::initializeWMLPageState (this=0x817e008)
    at ../../WebCore/dom/Document.cpp:4496
#10 0x01947f7c in WebCore::jsDocumentPrototypeFunctionInitializeWMLPageState (exec=0xb51bb0f0, 
    thisValue=..., args=...) at DerivedSources/JSDocument.cpp:2104
---Type <return> to continue, or q <return> to quit---
#11 0x0775416e in ?? ()
#12 0x00f14991 in JSC::JITCode::execute (this=0xb67615f8, registerFile=0xb6727b34, 
    callFrame=0xb51bb048, globalData=0xb6725158, exception=0xbfffeecc)
    at ../../JavaScriptCore/jit/JITCode.h:79
#13 0x00f060dc in JSC::Interpreter::execute (this=0xb6727b28, program=0xb67615e8, 
    callFrame=0xb672dc54, scopeChain=0xb672d688, thisObj=0xb5140000, exception=0xbfffeecc)
    at ../../JavaScriptCore/interpreter/Interpreter.cpp:613
#14 0x00fce433 in JSC::evaluate (exec=0xb672dc54, scopeChain=..., source=..., thisValue=...)
    at ../../JavaScriptCore/runtime/Completion.cpp:60
#15 0x010a12b9 in WebCore::ScriptController::evaluateInWorld (this=0x80ec224, sourceCode=..., 
    world=0xb6727bec) at ../../WebCore/bindings/js/ScriptController.cpp:126
#16 0x010a26af in WebCore::ScriptController::executeScriptInWorld (this=0x80ec224, 
    world=0xb6727bec, script=..., forceUserGesture=false)
    at ../../WebCore/bindings/js/ScriptController.cpp:459
#17 0x0109b9c4 in WebCore::ScheduledAction::execute (this=0x81cb0d8, document=0xb671ff40)
    at ../../WebCore/bindings/js/ScheduledAction.cpp:129
#18 0x0109b5be in WebCore::ScheduledAction::execute (this=0x81cb0d8, context=0xb671ff70)
    at ../../WebCore/bindings/js/ScheduledAction.cpp:77
#19 0x013fc036 in WebCore::DOMTimer::fired (this=0x81cb0f8)
    at ../../WebCore/page/DOMTimer.cpp:149
#20 0x01492c28 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x80e0228)
    at ../../WebCore/platform/ThreadTimers.cpp:112
#21 0x01492b6f in WebCore::ThreadTimers::sharedTimerFired ()
---Type <return> to continue, or q <return> to quit---
    at ../../WebCore/platform/ThreadTimers.cpp:90
#22 0x018a0db5 in timeout_cb () at ../../WebCore/platform/gtk/SharedTimerGtk.cpp:48
#23 0x008360f1 in ?? () from /lib/libglib-2.0.so.0
#24 0x00837e78 in g_main_context_dispatch () from /lib/libglib-2.0.so.0
#25 0x0083b720 in ?? () from /lib/libglib-2.0.so.0
#26 0x0083bb8f in g_main_loop_run () from /lib/libglib-2.0.so.0
#27 0x003b7419 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#28 0x08049f29 in main (argc=2, argv=0xbffff484) at ../../WebKitTools/GtkLauncher/main.c:209

As it can be seen from the backtrace, WebCore::FrameLoader::reload gets triggered by JSC::JITCode::execute and not from FrameLoader. In back traces where crash was seen (Step 2 in initial posts in this thread), 'reload' was getting triggered from FrameLoader/DocumentLoader.

Given this, I propose to commit this change without layout test. Please let me know your consents.

[Nayan]

Created attachment 44754 [details]
Patch without layout test.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44754 [details] without any errors.

[Nayan]

Created attachment 44755 [details]
Sample test case.

Since it is not possible to write layout test, attached is a sample test case which can be used to reproduce the issue.

[Eric Seidel (no email)]

So there is no way to trigger this with non-WML content?  Changing cross-platform frameloader code always makes me a little antsy.

[Nayan]

(In reply to comment #20)
> So there is no way to trigger this with non-WML content?  Changing
> cross-platform frameloader code always makes me a little antsy.

No Eric, it is not possible to trigger this crash from non-WML contents. As mentioned in comment #2, redesign of History Handling/Page cache/loading in WML pages uncovered this bug in FrameLoader. Though I cannot test this fix in other platforms, I strongly feel issue should be reproducible in all platforms with WML enabled.

[Brady Eidson]

(In reply to comment #21)
> (In reply to comment #20)
> > So there is no way to trigger this with non-WML content?  Changing
> > cross-platform frameloader code always makes me a little antsy.
> 
> No Eric, it is not possible to trigger this crash from non-WML contents. 

Okay.

> As mentioned in comment #2, redesign of History Handling/Page cache/loading in WML
> pages uncovered this bug in FrameLoader. Though I cannot test this fix in other
> platforms, I strongly feel issue should be reproducible in all platforms with
> WML enabled.

Okay.

So you can only reproduce this bug in WML builds but you're changing everyone's build.  That's Eric's concern, which I share.

(In reply to comment #2)
> This assert wasn't seen in builds older revision 46418. There was a redesign of
> History handling / page cache / loading in WML pages, got fixed with bug
> #27707. This fix uncovered an existing issue in Frameloader which was causing
> this asset.

More correctly, the fix uncovered an existing issue in WML's assumptions about FrameLoader.

> ...
> 
> This is because, FrameLoader::continueLoadAfterNavigationPolicy function used
> to set m_loadType to FrameLoadTypeReload after the call to stopAllLoaders().
> 
> ....
> ....
> FrameLoadType type = policyChecker()->loadType();
> stopAllLoaders();
> ...
> ...
> m_loadType = type;
> ...

In this context, stopAllLoaders() is a call attached to the *PREVIOUS* load.  Which is why we don't set the new load type before it.

All sorts of interesting things happens from within stopAllLoaders() including unload() and other events being dispatched and related WebKit API calls being sent out to client applications.  Those client applications can then call back in to WebKit where this flag being wrong is... wrong!.

> Meanwhile, stopAllLoaders() will in-turn call 'handleIntrinsicEventIfNeeded'.
> 'handleInstrinsicEventIfNeeded' will trigger the reload request based on load
> type, which is still FrameLoadTypeStandard. Ideally, we need to set m_loadType
> variable of FrameLoader class before calling stopAllLoaders().

If "handleIntrinsicEventIfNeeded" needs to know about the load type that is ABOUT to occur as opposed to the load type that already occured, you need to find a way to get that information there without changing this behavior for all other ports.

[Brady Eidson]

Comment on attachment 44754 [details]
Patch without layout test.

r- based on the comments I just made.

[Nayan]

Thanks Eidson for your review comments, I completely I agree with your concerns.

'FrameLoader->policyChecker()->loadType' holds the information about the load type that is ABOUT to occur as opposed to the load type that already occurred. Hence, instead of checking for 'FrameLoader->loadType', I will check for 'FrameLoader->policyChecker()->loadType'. I guess this is the correct way to fix this issue.

(In reply to comment #23)
> (From update of attachment 44754 [details])
> r- based on the comments I just made.

[Nayan]

Created attachment 45273 [details]
Revisited fix for assert failure.

'FrameLoader->policyChecker()->loadType holds the information about the loadType that is ABOUT to occur. Hence modifying the patch to check for this variable, instead of 'FrameLoader->loadType'

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45273 [details] without any errors.

[Eric Seidel (no email)]

Comment on attachment 45273 [details]
Revisited fix for assert failure.

OK.

[WebKit Commit Bot]

Comment on attachment 45273 [details]
Revisited fix for assert failure.

Rejecting patch 45273 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 11820 test cases.
fast/dom/prototype-inheritance-2.html -> failed

Exiting early after 1 failures. 5595 tests run.
82.50s total testing time

5594 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
1 test case (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/143004

[Eric Seidel (no email)]

Comment on attachment 45273 [details]
Revisited fix for assert failure.

False rejection due to bug 30098.

[WebKit Commit Bot]

Comment on attachment 45273 [details]
Revisited fix for assert failure.

Clearing flags on attachment: 45273

Committed r52515: <http://trac.webkit.org/changeset/52515>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.