Bug 237062

Summary: PingLoader is failing to call completion handler in error case
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: WebKit Misc.Assignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, bfulgham, cdumez, darin, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Brent Fulgham 2022-02-22 15:18:29 PST 
WebKit is failing to trigger the completion handler when PingLoad handles a redirect that results in an error.

<rdar://50157253>
Comment 1 Brent Fulgham 2022-02-22 15:26:24 PST 
Created attachment 452907 [details]
Patch
Comment 2 Chris Dumez 2022-02-22 15:31:26 PST 
Comment on attachment 452907 [details]
Patch

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

> Source/WebKit/NetworkProcess/PingLoad.cpp:143
> +            completionHandler({ });

Is there a reason we need to call this before the didFinish() call?
Would seem more logical the other way around for me.
Comment 3 Brent Fulgham 2022-02-22 15:32:18 PST 
Comment on attachment 452907 [details]
Patch

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

>> Source/WebKit/NetworkProcess/PingLoad.cpp:143
>> +            completionHandler({ });
> 
> Is there a reason we need to call this before the didFinish() call?
> Would seem more logical the other way around for me.

I just made this call int he same place as the previous error section (immediately above). I'll change it before landing.
Comment 4 Brent Fulgham 2022-02-22 15:32:51 PST 
Created attachment 452908 [details]
Patch for landing
Comment 5 EWS 2022-02-22 16:28:13 PST 
Committed r290338 (247659@main): <https://commits.webkit.org/247659@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452908 [details].
Comment 6 Darin Adler 2022-02-22 17:11:58 PST 
Can we add some tests?