[GTK] imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html crash On my Ubuntu 17.10, imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html always crashes. GTK port, Release build, trunk@228417 Debug build doesn't crash. BuildBots doesn't crash. ./Tools/Scripts/run-webkit-tests --gtk --release imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html > Thread 1 (Thread 0x7fa846698ac0 (LWP 4687)): > #0 0x00007fa844eabc68 in WebCore::DocumentLoader::scheduleSubstituteResourceLoad(WebCore::ResourceLoader&, WebCore::SubstituteResource&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #1 0x00007fa844f0c0e1 in WebCore::ApplicationCacheHost::maybeLoadResource(WebCore::ResourceLoader&, WebCore::ResourceRequest const&, WebCore::URL const&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #2 0x00007fa84402c804 in WebKit::WebLoaderStrategy::scheduleLoad(WebCore::ResourceLoader&, WebCore::CachedResource*, bool) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #3 0x00007fa84402d000 in WTF::Function<void (WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)>::CallableWrapper<WebKit::WebLoaderStrategy::loadResource(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, WebCore::ResourceLoaderOptions const&, WTF::CompletionHandler<void (WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)>&&)::{lambda(WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)#1}>::call(WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #4 0x00007fa844efe0ea in WTF::Function<void (bool)>::CallableWrapper<WebCore::SubresourceLoader::create(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, WebCore::ResourceLoaderOptions const&, WTF::CompletionHandler<void (WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)>&&)::{lambda(bool)#1}>::call(bool) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #5 0x00007fa844efe9d6 in WTF::Function<void (bool)>::CallableWrapper<WebCore::SubresourceLoader::init(WebCore::ResourceRequest&&, WTF::CompletionHandler<void (bool)>&&)::{lambda(bool)#1}>::call(bool) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #6 0x00007fa844ef2f72 in WTF::Function<void (WebCore::ResourceRequest&&)>::CallableWrapper<WebCore::ResourceLoader::init(WebCore::ResourceRequest&&, WTF::CompletionHandler<void (bool)>&&)::{lambda(WebCore::ResourceRequest&&)#1}>::call(WebCore::ResourceRequest&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #7 0x00007fa844efa810 in WTF::Function<void (WebCore::ResourceRequest&&)>::CallableWrapper<WebCore::SubresourceLoader::willSendRequestInternal(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&)::{lambda(WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&, WebCore::ResourceRequest&&)#1}::operator()(WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&, WebCore::ResourceRequest&&)::{lambda(WebCore::ResourceRequest&&)#1}>::call(WebCore::ResourceRequest&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #8 0x00007fa844ef72c0 in WebCore::ResourceLoader::willSendRequestInternal(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #9 0x00007fa844f03b1c in WebCore::SubresourceLoader::willSendRequestInternal(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&)::{lambda(WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&, WebCore::ResourceRequest&&)#1}::operator()(WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&, WebCore::ResourceRequest&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #10 0x00007fa844f0473e in WebCore::SubresourceLoader::willSendRequestInternal(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, WTF::CompletionHandler<void (WebCore::ResourceRequest&&)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #11 0x00007fa844ef29c0 in WebCore::ResourceLoader::init(WebCore::ResourceRequest&&, WTF::CompletionHandler<void (bool)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #12 0x00007fa844f037f9 in WebCore::SubresourceLoader::init(WebCore::ResourceRequest&&, WTF::CompletionHandler<void (bool)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #13 0x00007fa844f038ed in WebCore::SubresourceLoader::create(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, WebCore::ResourceLoaderOptions const&, WTF::CompletionHandler<void (WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #14 0x00007fa84402848d in WebKit::WebLoaderStrategy::loadResource(WebCore::Frame&, WebCore::CachedResource&, WebCore::ResourceRequest&&, WebCore::ResourceLoaderOptions const&, WTF::CompletionHandler<void (WTF::RefPtr<WebCore::SubresourceLoader, WTF::DumbPtrTraits<WebCore::SubresourceLoader> >&&)>&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #15 0x00007fa844f38adf in WebCore::CachedResource::load(WebCore::CachedResourceLoader&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #16 0x00007fa844f3d4c1 in WebCore::CachedResourceLoader::requestResource(WebCore::CachedResource::Type, WebCore::CachedResourceRequest&&, WebCore::CachedResourceLoader::ForPreload, WebCore::CachedResourceLoader::DeferOption) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #17 0x00007fa844f3e9df in WebCore::CachedResourceLoader::requestRawResource(WebCore::CachedResourceRequest&&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #18 0x00007fa844ea7adb in WebCore::DocumentThreadableLoader::loadRequest(WebCore::ResourceRequest&&, WebCore::SecurityCheckPolicy) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #19 0x00007fa844eae25c in WebCore::DocumentThreadableLoader::DocumentThreadableLoader(WebCore::Document&, WebCore::ThreadableLoaderClient&, WebCore::DocumentThreadableLoader::BlockingBehavior, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::RefPtr<WebCore::SecurityOrigin, WTF::DumbPtrTraits<WebCore::SecurityOrigin> >&&, std::unique_ptr<WebCore::ContentSecurityPolicy, std::default_delete<WebCore::ContentSecurityPolicy> >&&, WTF::String&&, WebCore::DocumentThreadableLoader::ShouldLogError) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #20 0x00007fa844eae6cc in WebCore::DocumentThreadableLoader::create(WebCore::Document&, WebCore::ThreadableLoaderClient&, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::RefPtr<WebCore::SecurityOrigin, WTF::DumbPtrTraits<WebCore::SecurityOrigin> >&&, std::unique_ptr<WebCore::ContentSecurityPolicy, std::default_delete<WebCore::ContentSecurityPolicy> >&&, WTF::String&&, WebCore::DocumentThreadableLoader::ShouldLogError) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #21 0x00007fa844efed80 in WTF::Function<void (WebCore::ScriptExecutionContext&)>::CallableWrapper<WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge(WebCore::ThreadableLoaderClientWrapper&, WebCore::WorkerLoaderProxy&, WTF::String const&, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::String const&, WebCore::WorkerGlobalScope&)::{lambda(WebCore::ScriptExecutionContext&)#1}>::call(WebCore::ScriptExecutionContext&) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #22 0x00007fa83f0a56b5 in WTF::dispatchFunctionsFromMainThread() () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18 > #23 0x00007fa83f0eb173 in WTF::RunLoop::TimerBase::TimerBase(WTF::RunLoop&)::{lambda(void*)#1}::_FUN(void*) () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18 > #24 0x00007fa83fe75c35 in g_main_dispatch () at /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3148 > #25 g_main_context_dispatch () at /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3813 > #26 0x00007fa83fe76000 in g_main_context_iterate () at /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:3886 > #27 0x00007fa83fe76312 in g_main_loop_run () at /home/fujii/work/webkit/gb/WebKitBuild/DependenciesGTK/Source/glib-2.54.2/glib/gmain.c:4082 > #28 0x00007fa83f0eb600 in WTF::RunLoop::run() () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libjavascriptcoregtk-4.0.so.18 > #29 0x00007fa84430ed08 in WebProcessMainUnix () from /home/fujii/work/webkit/gb/WebKitBuild/Release/lib/libwebkit2gtk-4.0.so.37 > #30 0x00007fa8423ab1c1 in __libc_start_main (main=0x5590113a88c0 <main>, argc=3, argv=0x7ffe774b0378, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffe774b0368) at ../csu/libc-start.c:308 > #31 0x00005590113a894a in _start ()
Created attachment 334139 [details] debug patch
If this debug patch is applied, the assertion fails even in Debug builds.
Comment on attachment 334139 [details] debug patch Attachment 334139 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6569219 New failing tests: imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
Created attachment 334140 [details] Archive of layout-test-results from ews103 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 334139 [details] debug patch Attachment 334139 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6569229 New failing tests: imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
Created attachment 334141 [details] Archive of layout-test-results from ews105 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 334139 [details] debug patch Attachment 334139 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6569253 New failing tests: imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
Created attachment 334144 [details] Archive of layout-test-results from ews116 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 334139 [details] debug patch Attachment 334139 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/6569266 New failing tests: imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
Created attachment 334145 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
These EWS layout test failures mean Mac port also has the null dereference issue. This is not GTK specific bug.
request.url() was "http://localhost:8800/resources/testharness.js". > // Resources that match fallback namespaces or online whitelist entries are fetched from the network, > // unless they are also cached. > if (!resource && (cache->allowsAllNetworkRequests() || cache->urlMatchesFallbackNamespace(request.url()) || cache->isURLInOnlineWhitelist(request.url()))) > return false; resource was null, but following all conditions are false.
Created attachment 334257 [details] Patch
Comment on attachment 334257 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334257&action=review > Source/WebCore/loader/DocumentLoader.cpp:1432 > - m_pendingSubstituteResources.set(&loader, &resource); > + m_pendingSubstituteResources.set(&loader, resource); > deliverSubstituteResourcesAfterDelay(); I don't understand; why would you want to add nullptr to m_pendingSubstituteResources? Isn't it better to avoid calling scheduleSubstituteResourceLoad entirely in this case? Then you can make scheduleArchiveLoad return false, instead of true, to indicate failure. Does deliverSubstituteResourcesAfterDelay need to be called when the second argument is nullptr? Probably not, I guess, so it should probably be OK to skip the call to scheduleSubstituteResourceLoad entirely, right? > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:182 > if (!shouldLoadResourceFromApplicationCache(request, resource)) > return false; > > - m_documentLoader.scheduleSubstituteResourceLoad(loader, *resource); > + m_documentLoader.scheduleSubstituteResourceLoad(loader, resource); I wonder about this, though. Looks like resource could be null here, as well. I suspect we should skip the call to scheduleSubstituteResourceLoad and return false in this case, too. Not sure. > Source/WebCore/loader/appcache/ApplicationCacheHost.cpp:449 > - m_documentLoader.scheduleSubstituteResourceLoad(*loader, *resource); > + m_documentLoader.scheduleSubstituteResourceLoad(*loader, resource); resource will always be non-null here, so this should be fine.
Comment on attachment 334257 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334257&action=review Thank you for the review, Michael. >> Source/WebCore/loader/DocumentLoader.cpp:1432 >> deliverSubstituteResourcesAfterDelay(); > > I don't understand; why would you want to add nullptr to m_pendingSubstituteResources? Isn't it better to avoid calling scheduleSubstituteResourceLoad entirely in this case? Then you can make scheduleArchiveLoad return false, instead of true, to indicate failure. > > Does deliverSubstituteResourcesAfterDelay need to be called when the second argument is nullptr? Probably not, I guess, so it should probably be OK to skip the call to scheduleSubstituteResourceLoad entirely, right? Yes, DocumentLoader::scheduleSubstituteResourceLoad is tricky, there are a FIXME comment in DocumentLoader::substituteResourceDeliveryTimerFired. https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/loader/DocumentLoader.cpp?rev=228852#L1384 > // A null resource means that we should fail the load. > // FIXME: Maybe we should use another error here - something like "not in cache". > loader->didFail(loader->cannotShowURLError()); I will submit another less tricky patch.
Created attachment 334329 [details] Patch
Hm, I think this looks better. It would be better if someone more experienced with WebCore would review it, though. I see Ryosuke is already CCed; maybe he would want to look at it.
Comment on attachment 334329 [details] Patch Clearing flags on attachment: 334329 Committed r228975: <https://trac.webkit.org/changeset/228975>
All reviewed patches have been landed. Closing bug.
<rdar://problem/37860702>