Bug 237234 - Add SPI _WKDataTask
Summary: Add SPI _WKDataTask
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-25 21:35 PST by Alex Christensen
Modified: 2022-03-02 00:10 PST (History)
7 users (show)

See Also:

Attachments
Patch (85.40 KB, patch)
2022-02-25 21:38 PST, Alex Christensen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (86.16 KB, patch)
2022-02-25 21:51 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (86.19 KB, patch)
2022-02-26 07:02 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (86.21 KB, patch)
2022-02-26 07:03 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (96.80 KB, patch)
2022-03-01 12:58 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Patch (98.02 KB, patch)
2022-03-01 22:42 PST, Alex Christensen 		no flags Details | Formatted Diff | Diff
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2022-02-25 21:35:43 PST 
Add SPI _WKDataTask
Comment 1 Alex Christensen 2022-02-25 21:38:56 PST 
Created attachment 453286 [details]
Patch
Comment 2 Alex Christensen 2022-02-25 21:51:54 PST 
Created attachment 453287 [details]
Patch
Comment 3 Alex Christensen 2022-02-26 07:02:44 PST 
Created attachment 453293 [details]
Patch
Comment 4 Alex Christensen 2022-02-26 07:03:41 PST 
Created attachment 453294 [details]
Patch
Comment 5 Tim Horton 2022-02-28 23:23:59 PST 
Comment on attachment 453294 [details]
Patch

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

> Source/WebKit/UIProcess/API/Cocoa/_WKDataTask.h:41
> +// Calling this will often delegate's dataTask:didCompleteWithError: with { NSURLErrorDomain, NSURLErrorCancelled }

often?
Comment 6 Tim Horton 2022-02-28 23:44:22 PST 
Comment on attachment 453294 [details]
Patch

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

> Source/WebKit/NetworkProcess/cocoa/WKURLSessionTaskDelegate.mm:76
> +- (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPerformHTTPRedirection:(NSHTTPURLResponse *)response newRequest:(NSURLRequest *)request completionHandler:(void (^)(NSURLRequest * _Nullable))completionHandler

No nullability in implementation files (because it's viral).

> Source/WebKit/UIProcess/API/APIDataTask.cpp:56
> +    , m_client(DataTaskClient::create()) { }

Odd place for those curly braces

> Source/WebKit/UIProcess/API/APIDataTaskClient.h:50
> +    virtual void willPerformHTTPRedirection(DataTask&, WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, CompletionHandler<void(bool)>&& completionHandler) const { completionHandler(true); }

Is `HTTP` proper WebKit style or `Http`? (I like the all-caps version but I think WebKit style does not)

> Source/WebKit/UIProcess/API/Cocoa/_WKDataTaskDelegate.h:51
> +- (void)dataTask:(_WKDataTask *)dataTask didCompleteWithError:(nullable NSError *)error;

Why both nullable and _Nullable?
Comment 7 Tim Horton 2022-02-28 23:44:35 PST 
Probably should get a Brady review too
Comment 8 Alex Christensen 2022-03-01 09:14:50 PST 
(In reply to Tim Horton from comment #6)
> > Source/WebKit/UIProcess/API/Cocoa/_WKDataTaskDelegate.h:51
> > +- (void)dataTask:(_WKDataTask *)dataTask didCompleteWithError:(nullable NSError *)error;
> 
> Why both nullable and _Nullable?
nullable for selector parameters, _Nullable for block parameters
Comment 9 Alex Christensen 2022-03-01 09:15:16 PST 
I think I'm also going to make the delegate strong until cancelled or completed, kind of like NSURLSessionTask, and I'll make cancel give you no more callbacks.
Comment 10 Alex Christensen 2022-03-01 12:58:32 PST 
Created attachment 453528 [details]
Patch
Comment 11 Radar WebKit Bug Importer 2022-03-01 15:30:38 PST 
<rdar://problem/89645347>
Comment 12 Alex Christensen 2022-03-01 22:42:20 PST 
Created attachment 453571 [details]
Patch
Comment 13 EWS 2022-03-02 00:10:21 PST 
Committed r290722 (247969@main): <https://commits.webkit.org/247969@main>

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