Bug 110588 – REGRESSION (r143619): Crashes in three layout tests

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

Three layout tests are crashing after r143619 landed.
http://trac.webkit.org/changeset/143619

The affected tests:
http/tests/misc/window-open-then-write.html
http/tests/misc/iframe-reparenting-id-collision.html
http/tests/xmlhttprequest/request-from-popup.html

file:///dvt/webkit/webkit/Tools/TestResultServer/static-dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=http%2Ftests%2Fmisc%2Fwindow-open-then-write.html%2Chttp%2Ftests%2Fmisc%2Fiframe-reparenting-id-collision.html%2Chttp%2Ftests%2Fxmlhttprequest%2Frequest-from-popup.html

Crash log for DumpRenderTree (pid 31322):

...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/Programs/D'.
Program terminated with signal 11, Segmentation fault.
#0  0x00002b3ed6590d4a in ?? () from /lib/x86_64-linux-gnu/libc.so.6

...

Thread 1 (Thread 0x2b3ede36fde0 (LWP 31322)):
#0  0x00002b3ed6590d4a in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00002b3ed4efba70 in g_str_equal () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#2  0x00002b3ecfc01138 in WebKit::FrameLoaderClient::dispatchDidFailLoading (this=0x82c2160, loader=0x8263bc0, identifier=3199, error=...) at ../../Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1049
#3  0x00002b3ed04f4f88 in WebCore::ResourceLoadNotifier::didFailToLoad (this=0x82c04d8, loader=0x83e0e40, error=...) at ../../Source/WebCore/loader/ResourceLoadNotifier.cpp:98
#4  0x00002b3ed04f3b0c in WebCore::ResourceLoader::cancel (this=0x83e0e40, error=...) at ../../Source/WebCore/loader/ResourceLoader.cpp:410
#5  0x00002b3ed04f390b in WebCore::ResourceLoader::cancel (this=0x83e0e40) at ../../Source/WebCore/loader/ResourceLoader.cpp:369
#6  0x00002b3ed048f20c in WebCore::cancelAll (loaders=...) at ../../Source/WebCore/loader/DocumentLoader.cpp:76
#7  0x00002b3ed0492382 in WebCore::DocumentLoader::stopLoadingSubresources (this=0x8263bc0) at ../../Source/WebCore/loader/DocumentLoader.cpp:827
#8  0x00002b3ed048ffd1 in WebCore::DocumentLoader::stopLoading (this=0x8263bc0) at ../../Source/WebCore/loader/DocumentLoader.cpp:267
#9  0x00002b3ed04acffb in WebCore::FrameLoader::stopAllLoaders (this=0x82c0280, clearProvisionalItemPolicy=WebCore::ShouldClearProvisionalItem) at ../../Source/WebCore/loader/FrameLoader.cpp:1560
#10 0x00002b3ed04ad05a in WebCore::FrameLoader::stopForUserCancel (this=0x82c0280, deferCheckLoadComplete=false) at ../../Source/WebCore/loader/FrameLoader.cpp:1571
#11 0x00002b3ecfc3fc20 in webkit_web_view_stop_loading (webView=0x1e9d210) at ../../Source/WebKit/gtk/webkit/webkitwebview.cpp:4201
#12 0x00002b3ecfbe3ffd in WebKit::ChromeClient::closeWindowSoon (this=0x839b190) at ../../Source/WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:297
#13 0x00002b3ed05374b7 in WebCore::Chrome::closeWindowSoon (this=0x83babd0) at ../../Source/WebCore/page/Chrome.cpp:304
#14 0x00002b3ed055b320 in WebCore::DOMWindow::close (this=0x832cf50, context=0x8389660) at ../../Source/WebCore/page/DOMWindow.cpp:992
#15 0x00002b3ed0ac7d6e in WebCore::jsDOMWindowPrototypeFunctionClose (exec=0x2b3f2248e0a0) at DerivedSources/WebCore/JSDOMWindow.cpp:13063
#16 0x00002b3ee0a780e5 in ?? ()
#17 0x00007fffd1574ed0 in ?? ()
#18 0x00002b3ecef75174 in llint_op_call () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Debug/.libs/libjavascriptcoregtk-3.0.so.0
#19 0x00002b3f2248e060 in ?? ()
#20 0x0000000001f22b50 in ?? ()
#21 0x00007fffd1574e90 in ?? ()
#22 0x00002b3ecef1a007 in JSC::JSStack::installTrapsAfterFrame (this=0x0, frame=0x0) at ../../Source/JavaScriptCore/interpreter/JSStackInlines.h:213
#23 0x00002b3ecef18d74 in JSC::JITCode::execute (this=0x2b3f22a4fb90, stack=0x1f22b50, callFrame=0x2b3f2248e060, globalData=0x1f520a0) at ../../Source/JavaScriptCore/jit/JITCode.h:135
#24 0x00002b3ecef16531 in JSC::Interpreter::executeCall (this=0x1f22b40, callFrame=0x2b3f2291ea78, function=0x2b3f2299f970, callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/interpreter/Interpreter.cpp:1059
#25 0x00002b3eceffa8ed in JSC::call (exec=0x2b3f2291ea78, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/JavaScriptCore/runtime/CallData.cpp:40
#26 0x00002b3ecfcbefd5 in WebCore::JSMainThreadExecState::call (exec=0x2b3f2291ea78, functionObject=..., callType=JSC::CallTypeJS, callData=..., thisValue=..., args=...) at ../../Source/WebCore/bindings/js/JSMainThreadExecState.h:56
#27 0x00002b3ecfcee46b in WebCore::JSEventListener::handleEvent (this=0x8362a10, scriptExecutionContext=0x8389660, event=0x81dd760) at ../../Source/WebCore/bindings/js/JSEventListener.cpp:130
#28 0x00002b3ed0018b22 in WebCore::EventTarget::fireEventListeners (this=0x8406050, event=0x81dd760, d=0x84062b8, entry=...) at ../../Source/WebCore/dom/EventTarget.cpp:256
#29 0x00002b3ed00187ad in WebCore::EventTarget::fireEventListeners (this=0x8406050, event=0x81dd760) at ../../Source/WebCore/dom/EventTarget.cpp:203
#30 0x00002b3ed00183de in WebCore::EventTarget::dispatchEvent (this=0x8406050, event=...) at ../../Source/WebCore/dom/EventTarget.cpp:155
#31 0x00002b3ed08ddea3 in WebCore::XMLHttpRequestProgressEventThrottle::dispatchEvent (this=0x8406300, event=...) at ../../Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp:96
#32 0x00002b3ed08ddd3b in WebCore::XMLHttpRequestProgressEventThrottle::dispatchReadyStateChangeEvent (this=0x8406300, event=..., progressEventAction=WebCore::FlushProgressEvent) at ../../Source/WebCore/xml/XMLHttpRequestProgressEventThrottle.cpp:83
#33 0x00002b3ed08d6662 in WebCore::XMLHttpRequest::callReadyStateChangeListener (this=0x8406050) at ../../Source/WebCore/xml/XMLHttpRequest.cpp:425
#34 0x00002b3ed08d6539 in WebCore::XMLHttpRequest::changeState (this=0x8406050, newState=WebCore::XMLHttpRequest::DONE) at ../../Source/WebCore/xml/XMLHttpRequest.cpp:413
#35 0x00002b3ed08d9e49 in WebCore::XMLHttpRequest::didFinishLoading (this=0x8406050, identifier=3199) at ../../Source/WebCore/xml/XMLHttpRequest.cpp:1157
#36 0x00002b3ed049eed3 in WebCore::DocumentThreadableLoader::didFinishLoading (this=0x82b8fb0, identifier=3199, finishTime=0) at ../../Source/WebCore/loader/DocumentThreadableLoader.cpp:324
#37 0x00002b3ed049ed70 in WebCore::DocumentThreadableLoader::notifyFinished (this=0x82b8fb0, resource=0x83dd5c0) at ../../Source/WebCore/loader/DocumentThreadableLoader.cpp:311
#38 0x00002b3ed046ce82 in WebCore::CachedResource::checkNotify (this=0x83dd5c0) at ../../Source/WebCore/loader/cache/CachedResource.cpp:378
#39 0x00002b3ed046cedc in WebCore::CachedResource::data (this=0x83dd5c0, allDataReceived=true) at ../../Source/WebCore/loader/cache/CachedResource.cpp:387
#40 0x00002b3ed046975e in WebCore::CachedRawResource::data (this=0x83dd5c0, data=..., allDataReceived=true) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:72
#41 0x00002b3ed04fe60e in WebCore::SubresourceLoader::didFinishLoading (this=0x83e0e40, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:278
#42 0x00002b3ed04f3e53 in WebCore::ResourceLoader::didFinishLoading (this=0x83e0e40, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:466
#43 0x00002b3ed0ebf14e in WebCore::readCallback (asyncResult=0x2b3f246cd730, data=0x83fd0a0) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1326
#44 0x00002b3ed4d2ee5f in async_ready_callback_wrapper () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#45 0x00002b3ed4d497ea in g_simple_async_result_complete () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#46 0x00002b3ed4d49836 in complete_in_idle_cb () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgio-2.0.so.0
#47 0x00002b3ed4f0ffd1 in g_idle_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#48 0x00002b3ed4f0d903 in g_main_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#49 0x00002b3ed4f0e4b3 in g_main_context_dispatch () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#50 0x00002b3ed4f0e6a3 in g_main_context_iterate () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#51 0x00002b3ed4f0ead3 in g_main_loop_run () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libglib-2.0.so.0
#52 0x00002b3ed3ef8e22 in gtk_main () from /home/slave/webkitgtk/gtk-linux-64-debug-wk1/build/WebKitBuild/Dependencies/Root/lib64/libgtk-3.so.0
#53 0x000000000049ee0b in runTest (inputLine=...) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:770
#54 0x000000000049e4c1 in runTestingServerLoop () at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:553
#55 0x00000000004a17ef in main (argc=2, argv=0x7fffd1576568) at ../../Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1512

[Martin Robinson]

Created attachment 190109 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 190109 [details]
Patch

Makes sense, took me a while to understand which g_str_equal it was crashing on.

[Carlos Garcia Campos]

Comment on attachment 190109 [details]
Patch

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

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1024
> -    if (!g_str_equal(identifierString.get(), webView->priv->mainResourceIdentifier.data()))
> -        webkit_web_view_remove_resource(webView, identifierString.get());
> +    webkitWebViewRemoveSubresource(webView, identifierString.get());

I think that at this point identifierString should never be null, this might be hiding a bug in the WebCore loader.

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1048
> -    if (!g_str_equal(identifierString.get(), webView->priv->mainResourceIdentifier.data()))
> -        webkit_web_view_remove_resource(webView, identifierString.get());
> +    webkitWebViewRemoveSubresource(webView, identifierString.get());

Ditto.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:5075
> +    if (!mainResource.isNull() && g_str_equal(identifier, mainResource.data()))

Instead of checking null and then compare we might use g_strcmp0 that already handles null pointers gracefully.

[WebKit Review Bot]

Comment on attachment 190109 [details]
Patch

Clearing flags on attachment: 190109

Committed r144055: <http://trac.webkit.org/changeset/144055>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Martin Robinson]

Comment on attachment 190109 [details]
Patch

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

>> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1024
>> +    webkitWebViewRemoveSubresource(webView, identifierString.get());
> 
> I think that at this point identifierString should never be null, this might be hiding a bug in the WebCore loader.

Possibly.

>> Source/WebKit/gtk/webkit/webkitwebview.cpp:5075
>> +    if (!mainResource.isNull() && g_str_equal(identifier, mainResource.data()))
> 
> Instead of checking null and then compare we might use g_strcmp0 that already handles null pointers gracefully.

I did initially use g_strcmp0, but decided on this approach, because it seemed clearer. If mainResource wasn't a CString I would have used g_strcmp0.

[Martin Robinson]

*** Bug 111870 has been marked as a duplicate of this bug. ***