Use URLSessionTask.count* properties to track upload/download progress #1138
Conversation
|
cc @saiHemak |
| @@ -454,7 +453,7 @@ fileprivate extension _EasyHandle { | |||
| } | |||
| var length = Double() | |||
| try! CFURLSession_easy_getinfo_double(handle.rawHandle, CFURLSessionInfoCONTENT_LENGTH_DOWNLOAD, &length).asError() | |||
There was a problem hiding this comment.
Just an aside... This API returns the ContentLength as a Double, which seems crazy. There is a newer API which has a better return type. It was introduced in libcurl 7.55.0; could we switch to it instead?
There was a problem hiding this comment.
I guess Ubuntu 16.04 comes with a default libcurl 7.47. I am not sure if we should be using an API that is newer than 7.47.
There was a problem hiding this comment.
I didn't realise that we rely on the system libcurl. Ubuntu 14.04 LTS ships 7.35 which is even older 😢
| guard case .transferInProgress(let ts) = internalState else { fatalError("Received body data, but no transfer in progress.") } | ||
| task!.countOfBytesExpectedToReceive = contentLength > 0 ? contentLength : NSURLSessionTransferSizeUnknown |
There was a problem hiding this comment.
I would be more defensive:
guard let task = task else { fatalError("No task") }
task.countOfBytesExpectedToReceive = contentLength > 0 ? contentLength : NSURLSessionTransferSizeUnknown|
|
||
| s.delegateQueue.addOperation { | ||
| downloadDelegate.urlSession(s, downloadTask: task, didWriteData: Int64(data.count), totalBytesWritten: Int64(self.totalDownloaded), | ||
| totalBytesExpectedToWrite: Int64(self.easyHandle.fileLength)) | ||
| downloadDelegate.urlSession(s, downloadTask: task, didWriteData: Int64(data.count), totalBytesWritten: Int64(task.countOfBytesReceived), |
There was a problem hiding this comment.
Isn't task.countOfBytesReceived an Int64 already?
There was a problem hiding this comment.
Yes! Thanks for catching that.
pushkarnk
force-pushed
the
uploadtask-delegate
branch
from
6a42dc8 to
93c55fe
Compare
| guard case .transferInProgress(let ts) = internalState else { fatalError("Received body data, but no transfer in progress.") } | ||
| guard let task = task else { fatalError("Receieved header data but no task available") } |
There was a problem hiding this comment.
Also, add a full stop?
| fileHandle.closeFile() | ||
| self.properties[.temporaryFileURL] = self.tempFileURL | ||
| } | ||
| } | ||
| } | ||
|
|
||
| func didReceive(headerData data: Data) -> _EasyHandle._Action { | ||
| func didReceive(headerData data: Data, contentLength: Int64) -> _EasyHandle._Action { | ||
| guard case .transferInProgress(let ts) = internalState else { fatalError("Received body data, but no transfer in progress.") } |
There was a problem hiding this comment.
Should this say "Received header data"?
| @@ -501,10 +500,10 @@ extension _HTTPURLProtocol: _EasyHandleDelegate { | |||
| fileprivate func notifyDelegate(aboutUploadedData count: Int64) { | |||
| let session = self.task?.session as! URLSession | |||
There was a problem hiding this comment.
How about:
fileprivate func notifyDelegate(aboutUploadedData count: Int64) {
guard let task = self.task as? URLSessionUploadTask,
let session = task.session as? URLSession,
case .taskDelegate(let delegate) = session.behaviour(for: task) else { return }
task.countOfBytesSent += count
session.delegateQueue.addOperation {
delegate.urlSession(session, task: task, didSendBodyData: count,
totalBytesSent: task.countOfBytesSent,
totalBytesExpectedToSend: task.countOfBytesExpectedToSend)
}
}(not compiled)
pushkarnk
force-pushed
the
uploadtask-delegate
branch
from
93c55fe to
565d03c
Compare
|
@ianpartridge Agree with your suggestions. I've included them. Thanks! |
|
@swift-ci please test |
This is a kind of an extension to #1121
We had defined three private properties in URLSessionTask to track upload & download progress and make delegate calls. It turns out that the URLSessionTask API defines four properties to do the same. This PR replaces the uses of the private properties with these.
Also, the constant
URLSessionTransferSizeUnknownis changed toNSURLSessionTransferSizeUnknownto match Darwin.