Bug 177625 – [WK2][NetworkSession] Add support for resuming downloads

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Add support for resuming downloads for the WK2 NETWORK_SESSION code path.

[Chris Dumez]

<rdar://problem/34345975>

[Chris Dumez]

Created attachment 322122 [details]
WIP Patch

[Build Bot]

Attachment 322122 [details] did not pass style-queue:


ERROR: Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:44:  No space between ^ and block definition.  [whitespace/brackets] [4]
ERROR: Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:45:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 322129 [details]
WIP Patch

[Build Bot]

Attachment 322129 [details] did not pass style-queue:


ERROR: Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:44:  No space between ^ and block definition.  [whitespace/brackets] [4]
ERROR: Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:45:  Missing space after ,  [whitespace/comma] [3]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 322132 [details]
WIP Patch

[Chris Dumez]

Created attachment 322134 [details]
WIP Patch

[Chris Dumez]

Created attachment 322138 [details]
WIP Patch

[Chris Dumez]

Created attachment 322141 [details]
WIP Patch

[Alex Christensen]

See rdar://problem/23041906

[Chris Dumez]

Created attachment 322248 [details]
WIP Patch

[Chris Dumez]

Created attachment 322250 [details]
WIP Patch

[Chris Dumez]

Created attachment 322600 [details]
WIP Patch

[Chris Dumez]

Created attachment 322726 [details]
Patch

[Daniel Bates]

Comment on attachment 322726 [details]
Patch

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

> Source/WebKit/ChangeLog:13
> +        when cancelling an download over non-HTTP.

an => a

> Source/WebKit/ChangeLog:23
> +        (WebKit::Download::resume):

Please mention the workaround here.

> Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:46
> +        WTFLogAlways("Could not find network session with given sessionID");

sessionID => session ID

Would it be helpful to include the value of m_sessionID in the logged message?

> Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:53
> +    NSMutableDictionary *dictionary = [NSPropertyListSerialization propertyListWithData:nsData.get() options:NSPropertyListImmutable format:0 error:NULL];

NULL => nullptr 

(Since this is Objective-C++)

> Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:54
> +    [dictionary setObject:(NSString *)path forKey:@"NSURLSessionResumeInfoLocalPath"];

static_cast<NSString*>(...)

(Since this is Objective-C++)

> Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:55
> +    NSData *updatedData = [NSPropertyListSerialization dataWithPropertyList:dictionary format:NSPropertyListXMLFormat_v1_0 options:0 error:NULL];

NULL => nullptr

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h:63
> +    NSURLSessionDownloadTask *downloadTaskWithResumeData(NSData *);

* is on the wrong side.

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:279
> +                WebCore::AuthenticationChallenge authenticationChallenge(challenge);

Uniform initializer syntax?

> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:677
> +NSURLSessionDownloadTask *NetworkSessionCocoa::downloadTaskWithResumeData(NSData *resumeData)

*s are on the wrong side since this is a C++ member function.

[Chris Dumez]

Created attachment 322842 [details]
Patch

[Chris Dumez]

(In reply to Daniel Bates from comment #15)
> Comment on attachment 322726 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=322726&action=review
> 
> > Source/WebKit/ChangeLog:13
> > +        when cancelling an download over non-HTTP.
> 
> an => a
> 
> > Source/WebKit/ChangeLog:23
> > +        (WebKit::Download::resume):
> 
> Please mention the workaround here.
> 
> > Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:46
> > +        WTFLogAlways("Could not find network session with given sessionID");
> 
> sessionID => session ID
> 
> Would it be helpful to include the value of m_sessionID in the logged
> message?
> 
> > Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:53
> > +    NSMutableDictionary *dictionary = [NSPropertyListSerialization propertyListWithData:nsData.get() options:NSPropertyListImmutable format:0 error:NULL];
> 
> NULL => nullptr 
> 
> (Since this is Objective-C++)
> 
> > Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:54
> > +    [dictionary setObject:(NSString *)path forKey:@"NSURLSessionResumeInfoLocalPath"];
> 
> static_cast<NSString*>(...)
> 
> (Since this is Objective-C++)
> 
> > Source/WebKit/NetworkProcess/Downloads/cocoa/DownloadCocoa.mm:55
> > +    NSData *updatedData = [NSPropertyListSerialization dataWithPropertyList:dictionary format:NSPropertyListXMLFormat_v1_0 options:0 error:NULL];
> 
> NULL => nullptr
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.h:63
> > +    NSURLSessionDownloadTask *downloadTaskWithResumeData(NSData *);
> 
> * is on the wrong side.
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:279
> > +                WebCore::AuthenticationChallenge authenticationChallenge(challenge);
> 
> Uniform initializer syntax?
> 
> > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:677
> > +NSURLSessionDownloadTask *NetworkSessionCocoa::downloadTaskWithResumeData(NSData *resumeData)
> 
> *s are on the wrong side since this is a C++ member function.

Thanks, I merged all review comments.

[Chris Dumez]

Alex, ping review?

[WebKit Commit Bot]

Comment on attachment 322842 [details]
Patch

Clearing flags on attachment: 322842

Committed r223431: <https://trac.webkit.org/changeset/223431>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.