Bug 154473 – Implement downloads with NetworkSession

• Component: New Bugs
• Status: RESOLVED FIXED

[Alex Christensen]

Implement downloads with NetworkSession

[Alex Christensen]

Created attachment 271804 [details]
Patch

[Alex Christensen]

Comment on attachment 271804 [details]
Patch

first patch is WIP.  Something is still wrong :(

[Alex Christensen]

Created attachment 272031 [details]
Patch

[Brady Eidson]

Comment on attachment 272031 [details]
Patch

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

Mostly looks fine, but there's a bit of an anti-pattern here I think should be cleaned up instead of landed as-is

> Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:128
> -    client().willPerformHTTPRedirection(redirectResponse, request, completionHandler);
> +    if (client())
> +        client()->willPerformHTTPRedirection(redirectResponse, request, completionHandler);

Inside NetworkDataTask, you can access m_client directly instead of calling client(), right?

> Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:104
> +    if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier)) {
> +        if (auto* client = networkDataTask->client())
> +            client->didSendData(totalBytesSent, totalBytesExpectedToSend);
> +    }

This is a pattern repeated multiple times in this patch - The URLSession delegate null checking the data task's client.

Instead, I think it should just always call a method in NetworkDataTask, which then null checks its own client. e.g.
if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier))
    networkDataTask->client().didSendData(totalBytesSent, totalBytesExpectedToSend);

...becomes...

if (auto* networkDataTask = _session->dataTaskForIdentifier(task.taskIdentifier))
    networkDataTask->didSendData(totalBytesSent, totalBytesExpectedToSend);

And then NetworkDataTask::didSendData(...) just null checks m_client at the start.

That would make a lot of code easier to read (and write!)

[Alex Christensen]

Created attachment 272034 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/changeset/196984

[Alex Christensen]

Removed unused capture in r198672.