SimonSimCity / Xamarin-CrossDownloadManager

A cross platform download manager for Xamarin
MIT License
149 stars 68 forks source link

Task.Resume() on resumed downloads and HttpMaximumConnectionsPerHost configuration #109

Closed frankHarmann closed 5 years ago

frankHarmann commented 5 years ago

Patch1: Calling Task.Resume() in GetTasks2(...)

Patch2: Provide configuration option for HttpMaximumConnectionsPerHost

SimonSimCity commented 5 years ago

Thanks very much for the contribution. Would be nice to get it as two separate patches in next time 😉 I would have merged the second patch immediately, but the first is something I want to get explained why you added this ...

frankHarmann commented 5 years ago

To be honest, i am not sure the task.Resume() is really needed at that point, though all other implementations i have seen always call task.Resume() in GetTasks2. I added it in order to workaround some problems with tasks, which are reconstructed using GetTasks2, but which do not want to start and will be there at next app start, even if canceled (for what reason ever, could be an emulator problem). Regarding the use of task.Resume in GetTasks2 see f.e. https://github.com/lduchosal/DownloadManager.Xamarin/blob/master/DownloadManager.iOS/NSUrlSessionManager.cs or https://github.com/aritchie/httptransfertasks/blob/master/Plugin.HttpTransferTasks/Platforms/iOS/HttpTransferTasks.cs. May be the reason for this is this part of the docs (https://developer.apple.com/documentation/foundation/nsurlsessiondownloadtask): You can also resume downloads that failed because of network connectivity problems.

SimonSimCity commented 5 years ago

I don't want to resume download-tasks randomly just because I can. If there's not a distinct reason to do so, please revert the change.

Pausing downloads is not yet part of the implementation here and if there is an error I want the user (or the programmer) to be aware of it.

frankHarmann commented 5 years ago

Removed task.Resume() as requested. I am still not sure this is wise. You say "pausing downloads is not yet part of the implementation". Are you sure this is true for real "discretionary" background transfers. I thought ios is deciding for itself when to pause this kind of downloads and is able to resume them (as long as the server understands range headers)?? Will ios do the task.Resume automatically for "discretionary" downloads?

SimonSimCity commented 5 years ago

I've seen a lot of programmers - not only by unexperienced programmers - disimproving frameworks just because they didn't consider enough the full scope of their changes.

If the system is that free to pause my downloads, it should also be kind enough to resume them. If you encounter anything specific there I'm open for a discussion where we also would have to consider the side effects this would have - in particular in regards to the compatibility to the multitude of the platforms I try to support.