RESOLVED FIXED 237234
Add SPI _WKDataTask 
https://bugs.webkit.org/show_bug.cgi?id=237234
Summary Add SPI _WKDataTask
Alex Christensen
Reported 2022-02-25 21:35:43 PST
Add SPI _WKDataTask
Attachments
Patch (85.40 KB, patch)
2022-02-25 21:38 PST, Alex Christensen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (86.16 KB, patch)
2022-02-25 21:51 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (86.19 KB, patch)
2022-02-26 07:02 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (86.21 KB, patch)
2022-02-26 07:03 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (96.80 KB, patch)
2022-03-01 12:58 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Patch (98.02 KB, patch)
2022-03-01 22:42 PST, Alex Christensen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.
Alex Christensen
Comment 1 2022-02-25 21:38:56 PST
Created attachment 453286 [details] Patch
Alex Christensen
Comment 2 2022-02-25 21:51:54 PST
Created attachment 453287 [details] Patch
Alex Christensen
Comment 3 2022-02-26 07:02:44 PST
Created attachment 453293 [details] Patch
Alex Christensen
Comment 4 2022-02-26 07:03:41 PST
Created attachment 453294 [details] Patch
Tim Horton
Comment 5 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?
Tim Horton
Comment 6 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?
Tim Horton
Comment 7 2022-02-28 23:44:35 PST
Probably should get a Brady review too
Alex Christensen
Comment 8 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
Alex Christensen
Comment 9 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.
Alex Christensen
Comment 10 2022-03-01 12:58:32 PST
Created attachment 453528 [details] Patch
Radar WebKit Bug Importer
Comment 11 2022-03-01 15:30:38 PST
<rdar://problem/89645347>
Alex Christensen
Comment 12 2022-03-01 22:42:20 PST
Created attachment 453571 [details] Patch
EWS
Comment 13 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].
Note You need to log in before you can comment on or make changes to this bug.