Remove ambiguity when stopping html5 uploads #408

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

MCF commented Oct 19, 2011

Currently there is an ambiguity in the upload queue when an html5 upload is stopped. Specifically In plupload.js the stop() action will set the uploader's state to STOPPED, and reset any files that have a status of UPLOADING back to QUEUED. In html5's UploadFile function there are two possible paths once the uploader is stopped. It will either complete the current upload or, if you catch it early enough, it will not begin the actual upload. But in both cases the file.status is initially set to UPLOADING.

If you wish to both stop the upload and clear the queue (for example with a cancel button), it is impossible to determine if a file that is currently in an UPLOADING state will actual complete the upload - or if it will return early without completing the upload. And there is no notification if it returns without uploading. This makes it impossible to determine when the upload has truly been stopped.

Essentially, with the html5 runtime there are 4 states for a successful upload: QUEUED, UPLOADPREP, UPLOADING and DONE. But it seems needlessly complicated to deal with an UPLOADPREP state separately. And the last minute check has minimal impact on stopping the uploads any faster as the overwhelming majority of the time in the UploadingFile function is spent uploading the actual file.

This pull request removes the ambiguity by removing the check of the uploader's state in html5's UploadFile function. Once a file is set to UPLOADING it will upload to completion and any calling routines can count on that to determine when the upload is truly cancelled and the last in-progress upload is complete.

Owner

jayarjo commented Dec 26, 2011

If we remove the check for up.state == plupload.STOPPED we won't be able to stop the upload process if stop procedure was requested in the middle of chunked upload. It is not always possible to abort the upload immediately, but we tried to release it as soon as possible, so once the chunk that is being uploaded at the moment of the stop request is indeed uploaded, we are able to stop there.

Contributor

MCF commented Dec 27, 2011

Be that as it may, for non-chunked uploads leaving the check in introduces a race condition when stopping uploads and clearing the queue (i.e. Canceling an upload). Perhaps the conditional should be changed to only check if the upload state is stopped if you are also using chunking?

Contributor

gonchuki commented Jan 13, 2012

is this still needed now that there's proper XHR abort functionality in place? (and similar "really stop it now, I mean it" functionality in other runtimes)

Contributor

MCF commented Jan 14, 2012

I can't answer this question as I'm still using the older version.

Owner

jayarjo commented Jan 27, 2013

With abort functionality in place, we can unambiguously stop uploading right there. So you can be sure that the rest of the file will not be uploaded.

For comparably big chunks, like around at least 1mb we could probably remove the whole conditional. But for smaller ones (~100kb) you can hit and miss the middle state. And if you miss, chunk will trigger onload event and invoke UploadNextChunk, which will proceed uploading, without knowing that state is STOPPED, or file artificially marked as FAILED. So we need that conditional there.

@jayarjo jayarjo closed this Jan 27, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment