Bug 113616

Summary: Crash when "willSendRequest" causes the ResourceLoader to be cancelled.
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebKit2Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch v1 ap: review+

Description Brady Eidson 2013-03-29 15:47:34 PDT
Crash when "willSendRequest" causes the ResourceLoader to be cancelled.

This can happen a lot of ways.  One example is with the url http://www.rackspace.com/blog/mathematics-cannot-be-patented-case-dismissed/ where an XHR fails a redirect check and therefore the loader is cancelled.

When the WebCore loader is cancelled, the WebResourceLoader is destroyed but then it tries to access freed objects.

This was only introduced with the asynchronous ResourceHandleClient model so it's very new.

By adding a RefPtr<> protector we avoid the crash completely.  By adding a "detachFromCoreLoader" to call when the WebCore loader is aborting we can avoid unnecessary messaging back to the NetworkProcess.
Comment 1 Brady Eidson 2013-03-29 15:53:22 PDT
Created attachment 195811 [details]
Patch v1
Comment 2 Alexey Proskuryakov 2013-03-29 15:57:23 PDT
Comment on attachment 195811 [details]
Patch v1

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

> Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp:161
> +    ASSERT(loader);
> +    if (!loader)
> +        return;

How strongly do you feel about this? We usually do not not add such checks without knowing that this actually happens, and waiting for reproducible cases.
Comment 3 Brady Eidson 2013-03-29 15:59:01 PDT
(In reply to comment #2)
> (From update of attachment 195811 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195811&action=review
> 
> > Source/WebKit2/WebProcess/Network/WebResourceLoadScheduler.cpp:161
> > +    ASSERT(loader);
> > +    if (!loader)
> > +        return;
> 
> How strongly do you feel about this? We usually do not not add such checks without knowing that this actually happens, and waiting for reproducible cases.

I don't feel strongly about it, I'll remove it. (leaving the ASSERT)
Comment 4 Brady Eidson 2013-03-29 16:00:58 PDT
http://trac.webkit.org/changeset/147257