Summary: | Implement downloads with NetworkSession | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | beidson | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Alex Christensen
2016-02-19 14:39:16 PST
Created attachment 271804 [details]
Patch
Comment on attachment 271804 [details]
Patch
first patch is WIP. Something is still wrong :(
Created attachment 272031 [details]
Patch
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!) Created attachment 272034 [details]
Patch
|