Bug 146384

Summary: Crash in WebCore::ResourceError::cfError() after provisional load failed
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: Page LoadingAssignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, beidson, commit-queue, darin, koivisto
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 146391    
Attachments:
Description Flags
Patch v1 none

Description David Kilzer (:ddkilzer) 2015-06-27 07:32:17 PDT 
Crash in WebCore::ResourceError::cfError() after provisional load failed:

Thread 0 name:  Dispatch queue: com.apple.main-thread
Thread 0 Crashed ↩:
0   CoreFoundation                	0x000000018329058c __CFTypeCollectionRetain + 176 (CFRuntime.c:784)
1   CoreFoundation                	0x0000000183309a50 __CFBasicHashAddValue + 324 (CFBasicHash.c:383)
2   CoreFoundation                	0x0000000183309a50 __CFBasicHashAddValue + 324 (CFBasicHash.c:383)
3   CoreFoundation                	0x00000001831b4410 CFDictionarySetValue + 244 (CFDictionary.c:759)
4   WebCore                       	0x0000000195450d08 WebCore::ResourceError::cfError() const + 232 (ResourceErrorCF.cpp:158)
5   WebKit                        	0x000000018969d70c -[WKNSError _web_createTarget] + 24 (WKNSError.mm:37)
6   WebKit                        	0x000000018969e480 -[WKObject forwardingTargetForSelector:] + 64 (WKObject.mm:52)
7   CoreFoundation                	0x00000001832dad48 ___forwarding___ + 108 (NSForwarding.m:3115)
8   CoreFoundation                	0x00000001831deaf8 _CF_forwarding_prep_0 + 88 (NSForwarding.s:780)
9   Safari                        	0x00000001003a9b8c -[WebProcessPlugInBrowserPageController webProcessPlugInBrowserContextController:didFailProvisionalLoadWithErrorForFrame:error:] + 128 (WebProcessPlugInBrowserPageController.mm:312)
10  WebKit                        	0x00000001896bbf24 didFailProvisionalLoadWithErrorForFrame(OpaqueWKBundlePage const*, OpaqueWKBundleFrame const*, OpaqueWKError const*, void const**, void const*) + 120 (WKWebProcessPlugInBrowserContextController.mm:150)
11  WebKit                        	0x00000001894b8ec4 WebKit::InjectedBundlePageLoaderClient::didFailProvisionalLoadWithErrorForFrame(WebKit::WebPage*, WebKit::WebFrame*, WebCore::ResourceError const&, WTF::RefPtr<API::Object>&) + 120 (InjectedBundlePageLoaderClient.cpp:115)
12  WebKit                        	0x000000018958e220 WebKit::WebFrameLoaderClient::dispatchDidFailProvisionalLoad(WebCore::ResourceError const&) + 76 (WebFrameLoaderClient.cpp:479)
13  WebCore                       	0x00000001948cc0d8 WebCore::FrameLoader::checkLoadCompleteForThisFrame() + 872 (FrameLoader.cpp:2211)
14  WebCore                       	0x00000001948cbc38 WebCore::FrameLoader::checkLoadComplete() + 360 (FrameLoader.cpp:2449)
15  WebCore                       	0x0000000194d39e5c WebCore::FrameLoader::receivedMainResourceError(WebCore::ResourceError const&) + 556 (FrameLoader.cpp:2746)
16  WebCore                       	0x000000019493eee0 WebCore::CachedResource::checkNotify() + 280 (CachedResource.cpp:297)
17  WebCore                       	0x00000001949426c8 WebCore::SubresourceLoader::didCancel(WebCore::ResourceError const&) + 28 (SubresourceLoader.cpp:439)
18  WebCore                       	0x0000000194942048 WebCore::ResourceLoader::cancel(WebCore::ResourceError const&) + 488 (ResourceLoader.cpp:525)
19  WebCore                       	0x0000000194c18cd8 WebCore::DocumentLoader::cancelMainResourceLoad(WebCore::ResourceError const&) + 284 (DocumentLoader.cpp:1457)
20  WebCore                       	0x0000000194c18e08 WebCore::DocumentLoader::stopLoadingForPolicyChange() + 80 (DocumentLoader.cpp:779)
21  WebCore                       	0x0000000194c1a664 std::__1::__function::__func<WebCore::DocumentLoader::willSendRequest(WebCore::ResourceRequest&, WebCore::ResourceResponse const&)::$_0, std::__1::allocator<WebCore::DocumentLoader::willSendRequest(WebCore::ResourceRequest&, WebCore::ResourceResponse const&)::$_0>, void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>::operator()(WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>&&, bool&&) + 40 (DocumentLoader.cpp:564)
22  WebCore                       	0x00000001949155c8 WebCore::PolicyCallback::call(bool) + 88 (functional:1793)
23  WebCore                       	0x000000019491553c WebCore::PolicyChecker::continueAfterNavigationPolicy(WebCore::PolicyAction) + 604 (PolicyChecker.cpp:206)
24  WebKit                        	0x000000018958b12c WebKit::WebFrame::didReceivePolicyDecision(unsigned long long, WebCore::PolicyAction, unsigned long long, unsigned long long) + 200 (functional:1793)
25  WebKit                        	0x000000018958f640 WebKit::WebFrameLoaderClient::dispatchDecidePolicyForNavigationAction(WebCore::NavigationAction const&, WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::PolicyAction)>) + 864 (WebFrameLoaderClient.cpp:818)
26  WebCore                       	0x00000001953127bc WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, WebCore::DocumentLoader*, WTF::PassRefPtr<WebCore::FormState>, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>) + 2712 (PolicyChecker.cpp:122)
27  WebCore                       	0x0000000195311bd4 WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest const&, std::__1::function<void (WebCore::ResourceRequest const&, WTF::PassRefPtr<WebCore::FormState>, bool)>) + 156 (PolicyChecker.cpp:61)
28  WebCore                       	0x00000001949173fc WebCore::DocumentLoader::willSendRequest(WebCore::ResourceRequest&, WebCore::ResourceResponse const&) + 1336 (DocumentLoader.cpp:563)
29  WebCore                       	0x0000000194af29c0 WebCore::CachedRawResource::redirectReceived(WebCore::ResourceRequest&, WebCore::ResourceResponse const&) + 108 (CachedRawResource.cpp:164)
30  WebCore                       	0x00000001948a1830 WebCore::SubresourceLoader::willSendRequest(WebCore::ResourceRequest&, WebCore::ResourceResponse const&) + 404 (SubresourceLoader.cpp:182)
31  WebCore                       	0x000000019545651c WebCore::ResourceLoader::willSendRequest(WebCore::ResourceRequest&&, WebCore::ResourceResponse const&, std::__1::function<void (WebCore::ResourceRequest&)>) + 28 (ResourceLoader.cpp:342)
32  WebKit                        	0x000000018964f180 WebKit::WebResourceLoader::willSendRequest(WebCore::ResourceRequest const&, WebCore::ResourceResponse const&) + 464 (WebResourceLoader.cpp:92)
33  WebKit                        	0x000000018965003c void IPC::handleMessage<Messages::WebResourceLoader::WillSendRequest, WebKit::WebResourceLoader, void (WebKit::WebResourceLoader::*)(WebCore::ResourceRequest const&, WebCore::ResourceResponse const&)>(IPC::MessageDecoder&, WebKit::WebResourceLoader*, void (WebKit::WebResourceLoader::*)(WebCore::ResourceRequest const&, WebCore::ResourceResponse const&)) + 152 (HandleMessage.h:16)
34  WebKit                        	0x0000000189481fc8 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >) + 132 (Connection.cpp:870)
35  WebKit                        	0x0000000189483ecc IPC::Connection::dispatchOneMessage() + 116 (Connection.cpp:921)
36  JavaScriptCore                	0x0000000184e58fc8 WTF::RunLoop::performWork() + 456 (functional:1793)
37  JavaScriptCore                	0x0000000184e595e8 WTF::RunLoop::performWork(void*) + 32 (RunLoopCF.cpp:38)
38  CoreFoundation                	0x000000018328ed0c __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 20 (CFRunLoop.c:1767)
39  CoreFoundation                	0x000000018328e7a0 __CFRunLoopDoSources0 + 536 (CFRunLoop.c:1813)
40  CoreFoundation                	0x000000018328c474 __CFRunLoopRun + 720 (CFRunLoop.c:2542)
41  CoreFoundation                	0x00000001831b90cc CFRunLoopRunSpecific + 380 (CFRunLoop.c:2820)
42  Foundation                    	0x00000001841644dc -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 304 (NSRunLoop.m:366)
43  Foundation                    	0x00000001841b9e38 -[NSRunLoop(NSRunLoop) run] + 84 (NSRunLoop.m:388)
44  libxpc.dylib                  	0x00000001983d9210 _xpc_objc_main + 656 (main.m:176)
45  libxpc.dylib                  	0x00000001983daf20 xpc_main + 196 (init.c:1424)
46  com.apple.WebKit.WebContent   	0x0000000100097920 main + 52 (XPCServiceMain.mm:89)
47  libdyld.dylib                 	0x00000001981ba8b4 start + 0 (start_glue.s:78)
Comment 1 David Kilzer (:ddkilzer) 2015-06-27 07:32:46 PDT 
<rdar://problem/21523660>
Comment 2 David Kilzer (:ddkilzer) 2015-06-27 07:57:31 PDT 
Created attachment 255699 [details]
Patch v1
Comment 3 Darin Adler 2015-06-27 14:35:00 PDT 
Comment on attachment 255699 [details]
Patch v1

Not sure the logging is needed.
Comment 4 WebKit Commit Bot 2015-06-27 14:39:50 PDT 
Comment on attachment 255699 [details]
Patch v1

Clearing flags on attachment: 255699

Committed r186035: <http://trac.webkit.org/changeset/186035>
Comment 5 WebKit Commit Bot 2015-06-27 14:39:54 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 David Kilzer (:ddkilzer) 2015-06-27 19:37:24 PDT 
(In reply to comment #3)
> Comment on attachment 255699 [details]
> Patch v1
> 
> Not sure the logging is needed.

This code hadn't changed recently, so the reason why the URL is invalid (can't be parsed) seems interesting enough to warrant a log.

Or would you have preferred an ASSERT() with a NULL check instead?
Comment 7 David Kilzer (:ddkilzer) 2015-06-27 20:14:29 PDT 
(In reply to comment #6)
> (In reply to comment #3)
> > Comment on attachment 255699 [details]
> > Patch v1
> > 
> > Not sure the logging is needed.
> 
> This code hadn't changed recently, so the reason why the URL is invalid
> (can't be parsed) seems interesting enough to warrant a log.
> 
> Or would you have preferred an ASSERT() with a NULL check instead?

Oh, Darin removed the LOG statement in a build fix:

Committed r186036: <http://trac.webkit.org/changeset/186036>
Comment 8 Darin Adler 2015-06-28 11:09:38 PDT 
I agree that the reason why the URL can’t be parsed could be interesting, but please keep in mind that adding this log statement is unlikely to actually help us find these cases and also such a failure is unsurprising. The function we are using the convert the URL string to a URL is pretty picky and is something we long ago discovered we couldn’t generally use for URLs found on the web.

We could add the logging back, but I am not sure who exactly would be using this to probe the mystery and when. Better, I think, to fix the known problem by using the better functions for making URLs that we use for other purposes, as I allude to in the FIXME, which I think is what bug 146391 is about.