Bug 159526

Summary: Fix crash when sending failure message to closed WebProcess
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=159739
https://bugs.webkit.org/show_bug.cgi?id=159691
https://bugs.webkit.org/show_bug.cgi?id=161494
Attachments:
Description Flags
Patch beidson: review+

Description Alex Christensen 2016-07-07 15:22:20 PDT
Fix crash when sending failure message to closed WebProcess
Comment 1 Alex Christensen 2016-07-07 15:23:19 PDT
Created attachment 283064 [details]
Patch
Comment 2 Brady Eidson 2016-07-07 15:28:24 PDT
Comment on attachment 283064 [details]
Patch

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

Please explain how this could be testable if we had HTTP tests in TestWebKitAPI

> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:409
> +    } else if (messageSenderConnection())
> +        messageSenderConnection()->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);

} else if (auto* connection = messageSenderConnection())
    connection->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);
Comment 3 Alex Christensen 2016-07-07 15:43:34 PDT
https://trac.webkit.org/changeset/202939
Comment 4 Alex Christensen 2016-07-07 16:00:45 PDT
Followed up in https://trac.webkit.org/changeset/202940
Comment 5 Chris Dumez 2016-07-13 14:57:38 PDT
Comment on attachment 283064 [details]
Patch

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

>> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:409
>> +        messageSenderConnection()->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);
> 
> } else if (auto* connection = messageSenderConnection())
>     connection->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);

I don't think messageSenderConnection() can return null unless |this| has been destroyed.
Comment 6 Chris Dumez 2016-07-13 15:02:17 PDT
Comment on attachment 283064 [details]
Patch

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

>>> Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp:409
>>> +        messageSenderConnection()->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);
>> 
>> } else if (auto* connection = messageSenderConnection())
>>     connection->send(Messages::WebResourceLoader::DidFailResourceLoad(error), 0, 0);
> 
> I don't think messageSenderConnection() can return null unless |this| has been destroyed.

My bet is that we were getting here with a destroyed NetworkResourceLoader because of https://bugs.webkit.org/show_bug.cgi?id=159691