Bug 239089 - Use WebKit::blockedError instead of ResourceLoader::blockedError in WebLoaderStrategy::scheduleLoadFromNetworkProcess
Summary: Use WebKit::blockedError instead of ResourceLoader::blockedError in WebLoader...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-04-11 14:37 PDT by Alex Christensen
Modified: 2022-04-11 16:46 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.93 KB, patch)
2022-04-11 14:38 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (1.93 KB, patch)
2022-04-11 15:26 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2022-04-11 14:37:01 PDT
Use WebKit::blockedError instead of ResourceLoader::blockedError in WebLoaderStrategy::scheduleLoadFromNetworkProcess
Comment 1 Alex Christensen 2022-04-11 14:38:42 PDT
Created attachment 457288 [details]
Patch
Comment 2 Alex Christensen 2022-04-11 14:38:46 PDT
<rdar://problem/91295875>
Comment 3 Chris Dumez 2022-04-11 14:45:13 PDT
Comment on attachment 457288 [details]
Patch

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

> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:329
> +            RunLoop::main().dispatch([resourceLoader = Ref { resourceLoader }, request] {

May have been easier to capture `resourceLoader->blockedError()` in the lambda and it would have been more obvious that the new code is identical in behavior to the previous one.
Comment 4 Darin Adler 2022-04-11 14:46:30 PDT
Comment on attachment 457288 [details]
Patch

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

>> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:329
>> +            RunLoop::main().dispatch([resourceLoader = Ref { resourceLoader }, request] {
> 
> May have been easier to capture `resourceLoader->blockedError()` in the lambda and it would have been more obvious that the new code is identical in behavior to the previous one.

This copies the ResourceRequest. Is that necessary?
Comment 5 Chris Dumez 2022-04-11 14:47:47 PDT
Comment on attachment 457288 [details]
Patch

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

>>> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:329
>>> +            RunLoop::main().dispatch([resourceLoader = Ref { resourceLoader }, request] {
>> 
>> May have been easier to capture `resourceLoader->blockedError()` in the lambda and it would have been more obvious that the new code is identical in behavior to the previous one.
> 
> This copies the ResourceRequest. Is that necessary?

With my proposal, we would effectively just "move" a ResourceError in, which seems better.
Comment 6 Alex Christensen 2022-04-11 15:24:36 PDT
I like just capturing the error.  This is the same behavior without the possibility of null dereferencing because WebFrameLoaderClient::blockedError just calls WebKit::blockedError.
Comment 7 Alex Christensen 2022-04-11 15:26:04 PDT
Created attachment 457293 [details]
Patch
Comment 8 EWS 2022-04-11 16:46:15 PDT
Committed r292738 (249522@main): <https://commits.webkit.org/249522@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 457293 [details].