Bug 237062 - PingLoader is failing to call completion handler in error case
Summary: PingLoader is failing to call completion handler in error case
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-22 15:18 PST by Brent Fulgham
Modified: 2022-02-22 17:11 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.59 KB, patch)
2022-02-22 15:26 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Patch for landing (1.96 KB, patch)
2022-02-22 15:32 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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?