Apache Cordova
  1. Apache Cordova
  2. CB-622

FileTransfer interface should provide progress monitoring

    Details

      Description

      The FileTransfer upload and download interface seems to provide no way to monitor progress of the upload/download, other than waiting for the entire transfer to complete or fail.

      Being able to pass another callback for progress monitoring could be a useful interface, this might get called with a byte count or something.

        Issue Links

          Activity

          Brion Vibber created issue -
          Hide
          Shazron Abdullah added a comment -

          This is more of a cross-platform discussion that needs to be had. I can't find the File Transfer W3C API (didn't see anything here http://www.w3.org/TR/FileAPI/) but we're probably following the W3C.

          Show
          Shazron Abdullah added a comment - This is more of a cross-platform discussion that needs to be had. I can't find the File Transfer W3C API (didn't see anything here http://www.w3.org/TR/FileAPI/ ) but we're probably following the W3C.
          Shazron Abdullah made changes -
          Field Original Value New Value
          Component/s Android [ 12316401 ]
          Component/s BlackBerry [ 12316402 ]
          Component/s iOS [ 12316400 ]
          Component/s WP7 [ 12316404 ]
          Hide
          Brion Vibber added a comment -

          I don't think FileTransfer is actually part of W3C's FileAPI ... it feels very PhoneGappy in how it takes two callbacks and has a couple extra paramaeters tacked on.

          I think the FileAPI-friendly way of doing this is to pass File or Blob objects into a FormData object, then send that into an XMLHTTPRequest: https://developer.mozilla.org/En/XMLHttpRequest/Using_XMLHttpRequest#Using_FormData_objects

          This seems to live in the XMLHttpRequest2 spec: http://www.w3.org/TR/XMLHttpRequest2/#interface-formdata

          This also would sidestep FileTransfer-specific feature requests like:
          https://issues.apache.org/jira/browse/CB-51 (support PUT as well as POST)
          https://issues.apache.org/jira/browse/CB-52 (HTTP basic auth)
          https://issues.apache.org/jira/browse/CB-200 (support uploading multiple files in one request)

          since all those things can be done with FormData+XHR.

          ... there may already be native FormData implementations in the WebKit available (confirmed it claims to be there in iOS 4.3, iOS 5.1, and Android 4.0.2) so I don't know how hard it would be to supplement them or make them interact with other objects.

          Show
          Brion Vibber added a comment - I don't think FileTransfer is actually part of W3C's FileAPI ... it feels very PhoneGappy in how it takes two callbacks and has a couple extra paramaeters tacked on. I think the FileAPI-friendly way of doing this is to pass File or Blob objects into a FormData object, then send that into an XMLHTTPRequest: https://developer.mozilla.org/En/XMLHttpRequest/Using_XMLHttpRequest#Using_FormData_objects This seems to live in the XMLHttpRequest2 spec: http://www.w3.org/TR/XMLHttpRequest2/#interface-formdata This also would sidestep FileTransfer-specific feature requests like: https://issues.apache.org/jira/browse/CB-51 (support PUT as well as POST) https://issues.apache.org/jira/browse/CB-52 (HTTP basic auth) https://issues.apache.org/jira/browse/CB-200 (support uploading multiple files in one request) since all those things can be done with FormData+XHR. ... there may already be native FormData implementations in the WebKit available (confirmed it claims to be there in iOS 4.3, iOS 5.1, and Android 4.0.2) so I don't know how hard it would be to supplement them or make them interact with other objects.
          Shazron Abdullah made changes -
          Priority Major [ 3 ] Minor [ 4 ]
          Hide
          Filip Maj added a comment -

          FileTransfer is our homegrown API.

          A progress monitoring would be an API addition and that discussion should take place on the mailing list first before we move on working on this.

          Show
          Filip Maj added a comment - FileTransfer is our homegrown API. A progress monitoring would be an API addition and that discussion should take place on the mailing list first before we move on working on this.
          Hide
          Scott Davis added a comment -

          Progress monitoring would be fantastic for larger downloads. I'm not an objective-C dev, but I know JavaScript pretty well. If there's anything that I can do to help, let me know.

          Show
          Scott Davis added a comment - Progress monitoring would be fantastic for larger downloads. I'm not an objective-C dev, but I know JavaScript pretty well. If there's anything that I can do to help, let me know.
          Hide
          Scott Davis added a comment -

          Anyone willing to take this on? I can help with the JavaScript side of things. Really need this for a current project due to deliver in October.

          Show
          Scott Davis added a comment - Anyone willing to take this on? I can help with the JavaScript side of things. Really need this for a current project due to deliver in October.
          Hide
          Priscilia added a comment -

          I really need this too but I'm too bad in objective-C :/

          Show
          Priscilia added a comment - I really need this too but I'm too bad in objective-C :/
          Hide
          Brion Vibber added a comment -

          Consensus on the mailing list seems to be that FileTransfer should get deprecated in favor of a polyfill for XHR2, but until then it sounds like lots of folks would still love a progress interface on FileTransfer.

          Here's an in-progress patch for Android: https://github.com/brion/incubator-cordova-android/commit/c8c1769a65535e2ef1ccf915248ab97ef473ef22

          I'm having FileTransfer.upload take a 'progress' option parameter, which if set causes it to fire the success callback during upload (every buffer push) with the sent bytes so far. The progress events can be distinguished from the final success because the result object's responseCode property is -1 rather than an HTTP response code.

          I ran this technique past a couple folks on IRC and they told me it wasn't completely crazy, so opening this for discussion.

          Seems to work for me; should be easy to do the same for download. Not sure about iOS but it's probably not super hard there either.

          Please give me feedback on whether this seems sane, or any tweaks such as:

          • changing the -1 status code on progress events?
          • passing a callback into the FileUploadOptions object to make it simpler? (will need some JS changes I think)
          • etc
          Show
          Brion Vibber added a comment - Consensus on the mailing list seems to be that FileTransfer should get deprecated in favor of a polyfill for XHR2, but until then it sounds like lots of folks would still love a progress interface on FileTransfer. Here's an in-progress patch for Android: https://github.com/brion/incubator-cordova-android/commit/c8c1769a65535e2ef1ccf915248ab97ef473ef22 I'm having FileTransfer.upload take a 'progress' option parameter, which if set causes it to fire the success callback during upload (every buffer push) with the sent bytes so far. The progress events can be distinguished from the final success because the result object's responseCode property is -1 rather than an HTTP response code. I ran this technique past a couple folks on IRC and they told me it wasn't completely crazy, so opening this for discussion. Seems to work for me; should be easy to do the same for download. Not sure about iOS but it's probably not super hard there either. Please give me feedback on whether this seems sane, or any tweaks such as: changing the -1 status code on progress events? passing a callback into the FileUploadOptions object to make it simpler? (will need some JS changes I think) etc
          Hide
          Scott Davis added a comment -

          Sounds great to me! I can help with the JS stuff.

          Show
          Scott Davis added a comment - Sounds great to me! I can help with the JS stuff.
          Hide
          Brion Vibber added a comment -

          I've done some further work on this in my filetransfer-progress branch:

          https://github.com/brion/incubator-cordova-js/commits/filetransfer-progress
          https://github.com/brion/incubator-cordova-android/commits/filetransfer-progress

          Also includes an 'abort' method for CB-836.

          Adds an 'onprogress' property which can be set to a callback to receive the progress callbacks; they're internally sent to the success callback, but a wrapper splits them off from the original caller's callback. These progress events will only be sent if an object id argument was sent to the upload() method, so people running old JS on new .jar shouldn't get unexpected events. (Not sure if that's necessary.)

          Questions:

          • should the 'onprogress' also be settable through filetransfer.addEventListener('progress', ...) or is only an 'onprogress' property the way to go?

          Still to do:

          • create a ProgressEvent instead of passing through the FileUploadResult on progress events
          • implement for iOS, WP7
          Show
          Brion Vibber added a comment - I've done some further work on this in my filetransfer-progress branch: https://github.com/brion/incubator-cordova-js/commits/filetransfer-progress https://github.com/brion/incubator-cordova-android/commits/filetransfer-progress Also includes an 'abort' method for CB-836 . Adds an 'onprogress' property which can be set to a callback to receive the progress callbacks; they're internally sent to the success callback, but a wrapper splits them off from the original caller's callback. These progress events will only be sent if an object id argument was sent to the upload() method, so people running old JS on new .jar shouldn't get unexpected events. (Not sure if that's necessary.) Questions: should the 'onprogress' also be settable through filetransfer.addEventListener('progress', ...) or is only an 'onprogress' property the way to go? Still to do: create a ProgressEvent instead of passing through the FileUploadResult on progress events implement for iOS, WP7
          Hide
          Brion Vibber added a comment -

          and still to do:

          • implement for download!
          Show
          Brion Vibber added a comment - and still to do: implement for download!
          Hide
          Andrew Grieve added a comment -

          Had a look at the JS part of your change and added some comments on github. Mostly just minor things though, good work!

          I don't think we should worry about addEventListener, or else we would need to worry about implementing all of the features that go along with it. When we look at doing an XHR2 polyfill, then let's tackle that.

          I think the -1 statusCode might get us into trouble down the road since statusCode isn't really an enum type. How about looking for the presence of a progress payload on the event callback to identify it? e.g., assuming your callback JSON looks like:

          {lengthComputable:true, total:0, loaded:0}

          Then you could do:
          if ('lengthComputable' in e) {
          if (self.onprogress)

          { self.onprogress(e) }

          }

          If that's too wishwashy for you, adding an explicit "type" field to the callback JSON would work as well.

          Passing the callback into the FileUploadOptions would allow the native side to not fire progress events when there is no progress callback. However, it would move the API further from XHR, and I think if progress events are causing performance problems, then they should be throttled on the native side either way. So, +1 to how you have it now, with an onprogress property.

          Show
          Andrew Grieve added a comment - Had a look at the JS part of your change and added some comments on github. Mostly just minor things though, good work! I don't think we should worry about addEventListener, or else we would need to worry about implementing all of the features that go along with it. When we look at doing an XHR2 polyfill, then let's tackle that. I think the -1 statusCode might get us into trouble down the road since statusCode isn't really an enum type. How about looking for the presence of a progress payload on the event callback to identify it? e.g., assuming your callback JSON looks like: {lengthComputable:true, total:0, loaded:0} Then you could do: if ('lengthComputable' in e) { if (self.onprogress) { self.onprogress(e) } } If that's too wishwashy for you, adding an explicit "type" field to the callback JSON would work as well. Passing the callback into the FileUploadOptions would allow the native side to not fire progress events when there is no progress callback. However, it would move the API further from XHR, and I think if progress events are causing performance problems, then they should be throttled on the native side either way. So, +1 to how you have it now, with an onprogress property.
          Hide
          Brion Vibber added a comment -

          Great, thanks for the feedback!

          Passing the different event type back directly and firing to the success or onprogress based on its content should be fine, I'll try that.

          Show
          Brion Vibber added a comment - Great, thanks for the feedback! Passing the different event type back directly and firing to the success or onprogress based on its content should be fine, I'll try that.
          Hide
          Joe Bowser added a comment -

          Since you appear to be working on it, I'm assigning it to you. I also needed to test something on JIRA!

          Show
          Joe Bowser added a comment - Since you appear to be working on it, I'm assigning it to you. I also needed to test something on JIRA!
          Joe Bowser made changes -
          Assignee Andrew Grieve [ agrieve ]
          Hide
          Brion Vibber added a comment -

          Updated & moved to Wikimedia's github:
          https://github.com/wikimedia/incubator-cordova-js/commits/filetransfer-progress
          https://github.com/wikimedia/incubator-cordova-android/commits/filetransfer-progress

          This implements an 'onprogress' callback property, sending ProgressEvent objects for both uploads and downloads, and an 'abort' method which should work for both uploads and downloads. After aborting, the original upload/download's error callback will be called.

          If this is looking reasonably sane, I'll go ahead and try an iOS implementation...

          Show
          Brion Vibber added a comment - Updated & moved to Wikimedia's github: https://github.com/wikimedia/incubator-cordova-js/commits/filetransfer-progress https://github.com/wikimedia/incubator-cordova-android/commits/filetransfer-progress This implements an 'onprogress' callback property, sending ProgressEvent objects for both uploads and downloads, and an 'abort' method which should work for both uploads and downloads. After aborting, the original upload/download's error callback will be called. If this is looking reasonably sane, I'll go ahead and try an iOS implementation...
          Hide
          Samik R added a comment -

          Needed this today, searched and arrived here from this thread: http://groups.google.com/group/phonegap/browse_thread/thread/951727920144538f
          Progress looks great, thanks for all the hard work.

          Show
          Samik R added a comment - Needed this today, searched and arrived here from this thread: http://groups.google.com/group/phonegap/browse_thread/thread/951727920144538f Progress looks great, thanks for all the hard work.
          Hide
          Andrew Grieve added a comment -

          Again, looking really good Brion! Sorry it took me a while to look again, but I should be able to give you quick turn-around times going forward

          Some more comments:
          -Use a HashSet for abortTriggered instead of a map
          -Make the AbortException class static and put it at the class level instead of nested in a function
          -Set the total and lengthComputable fields for download by looking for the "Content-Length" response header.
          -Tabs vs spaces has been an ongoing issue for our codebase because we have no checks in place. You've fallen victim to using both within your changes as well. Could you do a replace-all for tabs and change them to 4 spaces?

          It would be really great if you could update the mobile-spec FileTransfer tests so that it checks that progress events are firing and also add a test that ensures that abort works. It's fine that these tests will fail on non-Android platforms. This should come before an iOS implementation.

          When you're ready for this to be merged, please file pull requests for them through github. I don't think we need to wait on iOS before starting these merges, but mobile-spec tests would be good.

          Bonus points if you could clean up your commit log before requesting a merge. Specifically, splitting up abort vs progress events into distinct commits, and beefing up commit descriptions so that one doesn't need to look at the diff to know what the commit changed. If you can't figure out the git magic though, we would, of course, still love to accept your patch

          Show
          Andrew Grieve added a comment - Again, looking really good Brion! Sorry it took me a while to look again, but I should be able to give you quick turn-around times going forward Some more comments: -Use a HashSet for abortTriggered instead of a map -Make the AbortException class static and put it at the class level instead of nested in a function -Set the total and lengthComputable fields for download by looking for the "Content-Length" response header. -Tabs vs spaces has been an ongoing issue for our codebase because we have no checks in place. You've fallen victim to using both within your changes as well. Could you do a replace-all for tabs and change them to 4 spaces? It would be really great if you could update the mobile-spec FileTransfer tests so that it checks that progress events are firing and also add a test that ensures that abort works. It's fine that these tests will fail on non-Android platforms. This should come before an iOS implementation. When you're ready for this to be merged, please file pull requests for them through github. I don't think we need to wait on iOS before starting these merges, but mobile-spec tests would be good. Bonus points if you could clean up your commit log before requesting a merge. Specifically, splitting up abort vs progress events into distinct commits, and beefing up commit descriptions so that one doesn't need to look at the diff to know what the commit changed. If you can't figure out the git magic though, we would, of course, still love to accept your patch
          Hide
          Cory Thompson added a comment - - edited

          I've implemented the IOS version of this code.
          https://github.com/coryjthompson/incubator-cordova-ios/compare/filetransfer-progress

          First time i've done anything in Objective C so any feedback would be appreciated.

          The javascript code has no modifications except it must merge the latest version from the master.
          https://github.com/apache/incubator-cordova-js/commit/8c46a970a0719d0f16a225b75421ecf6f12dcc02

          eg.
          exec(win, fail, 'FileTransfer', 'upload', [filePath, server, fileKey, fileName, mimeType, params, trustAllHosts, chunkedMode, header, this._id]);

          Thanks Brion for all your work towards this.

          Show
          Cory Thompson added a comment - - edited I've implemented the IOS version of this code. https://github.com/coryjthompson/incubator-cordova-ios/compare/filetransfer-progress First time i've done anything in Objective C so any feedback would be appreciated. The javascript code has no modifications except it must merge the latest version from the master. https://github.com/apache/incubator-cordova-js/commit/8c46a970a0719d0f16a225b75421ecf6f12dcc02 eg. exec(win, fail, 'FileTransfer', 'upload', [filePath, server, fileKey, fileName, mimeType, params, trustAllHosts, chunkedMode, header, this._id] ); Thanks Brion for all your work towards this.
          Hide
          Andrew Grieve added a comment -

          Great stuff Cory! I just left a couple of comments on your patch via github. Pretty much good to go!

          Brion - if you'd like help with writing mobile-spec tests, let me know and I'll whip some up.

          Show
          Andrew Grieve added a comment - Great stuff Cory! I just left a couple of comments on your patch via github. Pretty much good to go! Brion - if you'd like help with writing mobile-spec tests, let me know and I'll whip some up.
          Hide
          Andrew Grieve added a comment -

          Brion - also realized something after looking at Cory's iOS impl. I think the abortTriggered variable should be non-static so that it tracks only requests made by a single plugin instance (a single WebView).

          Show
          Andrew Grieve added a comment - Brion - also realized something after looking at Cory's iOS impl. I think the abortTriggered variable should be non-static so that it tracks only requests made by a single plugin instance (a single WebView).
          Hide
          Scott Davis added a comment -

          Anyone know how this implementation is coming for Android and iOS? Any idea if it's going to make it into the next release?

          Show
          Scott Davis added a comment - Anyone know how this implementation is coming for Android and iOS? Any idea if it's going to make it into the next release?
          Hide
          Andrew Grieve added a comment -

          I've got comments out to Brion and Cory before their changes get pulled in. Just waiting to see what their status is. It's still on track for getting into the next release.

          Show
          Andrew Grieve added a comment - I've got comments out to Brion and Cory before their changes get pulled in. Just waiting to see what their status is. It's still on track for getting into the next release.
          Hide
          Scott Davis added a comment -

          Fantastic!

          Show
          Scott Davis added a comment - Fantastic!
          Hide
          Andrew Grieve added a comment -

          Brion & Cory - Would love to get going on this. One thing I forgot to ask you earlier though, is that I need you to both sign the contributor agreement before I can patch in your contributions. You'll need to sign the ICLA, and if you contribute on company time, you'll also have to get a CCLA signed.

          http://www.apache.org/licenses/#clas

          Show
          Andrew Grieve added a comment - Brion & Cory - Would love to get going on this. One thing I forgot to ask you earlier though, is that I need you to both sign the contributor agreement before I can patch in your contributions. You'll need to sign the ICLA, and if you contribute on company time, you'll also have to get a CCLA signed. http://www.apache.org/licenses/#clas
          Hide
          Cory Thompson added a comment -

          Implemented those few changes. Tell me what you think.
          https://github.com/coryjthompson/incubator-cordova-ios/tree/filetransfer-progress

          Sorry for the delay, been a massive few weeks. I've also emailed the ICLA

          Show
          Cory Thompson added a comment - Implemented those few changes. Tell me what you think. https://github.com/coryjthompson/incubator-cordova-ios/tree/filetransfer-progress Sorry for the delay, been a massive few weeks. I've also emailed the ICLA
          Hide
          Andrew Grieve added a comment -

          Thanks Cory, last round of comments:

          • Can you make the new properties readonly (doesn't look like there's a need to set them).
          • You have setObject:forKey: twice in download (remove the first one)
          • in Abort, I think the a PluginResult needs to be sent.
          • Need to add progress callbacks to download() as well, or just tell me that you'd like to leave it as a TODO
          Show
          Andrew Grieve added a comment - Thanks Cory, last round of comments: Can you make the new properties readonly (doesn't look like there's a need to set them). You have setObject:forKey: twice in download (remove the first one) in Abort, I think the a PluginResult needs to be sent. Need to add progress callbacks to download() as well, or just tell me that you'd like to leave it as a TODO
          Hide
          Scott Davis added a comment -

          Adding the progress callbacks for download would be great! If you can it would be great to get them in before the next release. Thanks!

          Show
          Scott Davis added a comment - Adding the progress callbacks for download would be great! If you can it would be great to get them in before the next release. Thanks!
          Hide
          Andrew Grieve added a comment -

          Sorry for this Scott, but the release timeline for 2.1 got moved up a few weeks mostly due to some windows phone fixes that need to get out sooner rather than later. Starting tomorrow evening we'll have a 2.1 feature freeze and this won't make it in

          Show
          Andrew Grieve added a comment - Sorry for this Scott, but the release timeline for 2.1 got moved up a few weeks mostly due to some windows phone fixes that need to get out sooner rather than later. Starting tomorrow evening we'll have a 2.1 feature freeze and this won't make it in
          Hide
          Cory Thompson added a comment -

          I'll makes these changes over the weekend and hopefully have downloading going with progress. What is the scheduled release date for 2.2? Thanks

          Show
          Cory Thompson added a comment - I'll makes these changes over the weekend and hopefully have downloading going with progress. What is the scheduled release date for 2.2? Thanks
          Hide
          Michael Brooks added a comment -

          2.2.0 will probably land on the first week of October.

          Show
          Michael Brooks added a comment - 2.2.0 will probably land on the first week of October.
          Hide
          Brion Vibber added a comment -

          I'll take a look at code cleanup on the Android code this weekend/next week; may or may not have time to get to the mobile spec tests (later in September will be freer for me). My ICLA's emailed in.

          Show
          Brion Vibber added a comment - I'll take a look at code cleanup on the Android code this weekend/next week; may or may not have time to get to the mobile spec tests (later in September will be freer for me). My ICLA's emailed in.
          Hide
          Brion Vibber added a comment - - edited

          Updated Android and JS branches here:
          https://github.com/wikimedia/incubator-cordova-android/commits/filetransfer-work
          https://github.com/wikimedia/incubator-cordova-js/commits/filetransfer-work
          (fixed URLs)

          I've squashed, rebased, and fixed whitespace on the original commits and made some updates based on feedback above:

          • HashMap -> HashSet
          • move AbortException to class level
          • use Content-Length header in download when possible

          Note that since my earlier branch, another parameter has been added to FileTransfer.upload's exec() call – this rebase adds the new object id after it (headers).

          I haven't tested these latest updates, but they compile. When it passes inspection and all works right, I can make a clean squashed version that's ready to merge.

          Show
          Brion Vibber added a comment - - edited Updated Android and JS branches here: https://github.com/wikimedia/incubator-cordova-android/commits/filetransfer-work https://github.com/wikimedia/incubator-cordova-js/commits/filetransfer-work (fixed URLs) I've squashed, rebased, and fixed whitespace on the original commits and made some updates based on feedback above: HashMap -> HashSet move AbortException to class level use Content-Length header in download when possible Note that since my earlier branch, another parameter has been added to FileTransfer.upload's exec() call – this rebase adds the new object id after it (headers). I haven't tested these latest updates, but they compile. When it passes inspection and all works right, I can make a clean squashed version that's ready to merge.
          Hide
          Cory Thompson added a comment -

          Fixed those few bugs.
          https://github.com/coryjthompson/incubator-cordova-ios/commit/e2c5b1efec9218447764d9bfe643e746a57b764e

          Unfortunately unable to test because I no longer have an IOS device available to me. I'll borrow a friends during the week and work on file download progress.

          Show
          Cory Thompson added a comment - Fixed those few bugs. https://github.com/coryjthompson/incubator-cordova-ios/commit/e2c5b1efec9218447764d9bfe643e746a57b764e Unfortunately unable to test because I no longer have an IOS device available to me. I'll borrow a friends during the week and work on file download progress.
          Hide
          Andrew Grieve added a comment -

          Cory - had a look. Looks great! Feel free to just test on the simulator.

          Brion - JS changes look excellent!
          Java comments:

          • abortTriggered - should probably make this non-static or else multiple webviews could abort each other's requests.
          • upload - doesn't look like progress events ever get lengthComputable or total fields set on them?
          • download - great point about gzip. Did a bit of Googling and found this: http://stackoverflow.com/questions/12124534/monitor-gzip-download-progress-in-java. From what I can tell, it just means we're not even properly supporting gzipped responses! I'm going to follow up with investigating if that's true and file a separate bug if so.

          Good stuff! I think we're pretty much ready to go. Just need to wait for 2.1.0 to be released now.

          Show
          Andrew Grieve added a comment - Cory - had a look. Looks great! Feel free to just test on the simulator. Brion - JS changes look excellent! Java comments: abortTriggered - should probably make this non-static or else multiple webviews could abort each other's requests. upload - doesn't look like progress events ever get lengthComputable or total fields set on them? download - great point about gzip. Did a bit of Googling and found this: http://stackoverflow.com/questions/12124534/monitor-gzip-download-progress-in-java . From what I can tell, it just means we're not even properly supporting gzipped responses! I'm going to follow up with investigating if that's true and file a separate bug if so. Good stuff! I think we're pretty much ready to go. Just need to wait for 2.1.0 to be released now.
          Hide
          Brion Vibber added a comment -

          > upload - doesn't look like progress events ever get lengthComputable or total fields set on them?

          Yeah, I wasn't sure how to get a length reliably, especially for content: URIs (photos selected from gallery).

          Show
          Brion Vibber added a comment - > upload - doesn't look like progress events ever get lengthComputable or total fields set on them? Yeah, I wasn't sure how to get a length reliably, especially for content: URIs (photos selected from gallery).
          Hide
          Andrew Grieve added a comment -

          Good point. we should just follow up with that with a separate bug later on. Might be worth adding a comment to that effect in the code.

          Show
          Andrew Grieve added a comment - Good point. we should just follow up with that with a separate bug later on. Might be worth adding a comment to that effect in the code.
          Hide
          Cory Thompson added a comment -

          Managed to add download progress for IOS last weekend. Tested and seems to work fine. Let me know why you think.

          https://github.com/coryjthompson/incubator-cordova-ios/commit/e2b1eb30f8d5ac5241c77a5837478d02b07d495f

          Show
          Cory Thompson added a comment - Managed to add download progress for IOS last weekend. Tested and seems to work fine. Let me know why you think. https://github.com/coryjthompson/incubator-cordova-ios/commit/e2b1eb30f8d5ac5241c77a5837478d02b07d495f
          Hide
          Andrew Grieve added a comment -

          Looks great Cory!

          Show
          Andrew Grieve added a comment - Looks great Cory!
          Hide
          Andrew Grieve added a comment -

          I did some more investigation into the state of gzip.

          The case right now on Android is that gzip does not happen on older version (pre-froyo I think). And on newer version it happens transparently. If we set the accept-encoding:gzip header ourselves on the connection, then it will not do the transparent handling of gzip, and we need to deflate it ourselves. This is good news because it means we can do as the stackoverflow article suggests in order to track get progress of the download.

          The case for iOS though, is that there is no way to track the actual bytes downloaded except for moving network stacks to something like ASI's. I'm wondering if in this case a decent option is to re-gzip the response locally to approximate. seems lame though, and I think we'd want to wait for users to ask for this first.

          Show
          Andrew Grieve added a comment - I did some more investigation into the state of gzip. The case right now on Android is that gzip does not happen on older version (pre-froyo I think). And on newer version it happens transparently. If we set the accept-encoding:gzip header ourselves on the connection, then it will not do the transparent handling of gzip, and we need to deflate it ourselves. This is good news because it means we can do as the stackoverflow article suggests in order to track get progress of the download. The case for iOS though, is that there is no way to track the actual bytes downloaded except for moving network stacks to something like ASI's. I'm wondering if in this case a decent option is to re-gzip the response locally to approximate. seems lame though, and I think we'd want to wait for users to ask for this first.
          Hide
          Andrew Grieve added a comment -

          Picking this back up again now that tagging is over. Plan on patching in all changes, writing a mobile-spec test, and then committing

          Show
          Andrew Grieve added a comment - Picking this back up again now that tagging is over. Plan on patching in all changes, writing a mobile-spec test, and then committing
          Hide
          Andrew Grieve added a comment -

          JS commits:

          iOS commits:

          Android commits:

          mobile-spec commits:

          Show
          Andrew Grieve added a comment - JS commits: https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=commit;h=a9db8e3d85a08cab6ccf86f29cc476c1178d2d57 iOS commits: https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=33afb30726b9480368ee315eb19c7651650ba340 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=de69937e7e987ef6648a6e3623d0febea5bef513 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=7c4fa19d5988037b1a69b406d337d9a7e0158e36 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=d1ff5a711e3175c0769e5f2036798a032b00fdd7 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=31f894cf5e6c546904cc036f98384078c9de3393 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=56602c020ef50ae6d62c3a1532719df5ccf46518 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-ios.git;a=commit;h=360eab9b6fa118338f895ec884dcc7e3617cf0cc Android commits: https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-android.git;a=commit;h=610e0c984a9cb9c6e202a0f6e2a711f8e326d9d6 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-android.git;a=commit;h=17af41723547410778e194aa77c2b5f55309ca43 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-android.git;a=commit;h=df9d314361dab8307e4fa045591f3b65c63a5a7b mobile-spec commits: https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-mobile-spec.git;a=commit;h=6ac22a5a6f6df37f870f775de95ff2883bcefab0 https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-mobile-spec.git;a=commit;h=e134e02e5c2263b435df8968fa3dceee24b8fbd2
          Andrew Grieve made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 2.2.0 [ 12322549 ]
          Resolution Fixed [ 1 ]
          Hide
          Scott Davis added a comment -

          Sweet! Thanks to everyone that worked on this issue. This will be a huge improvement!

          Show
          Scott Davis added a comment - Sweet! Thanks to everyone that worked on this issue. This will be a huge improvement!
          Hide
          Cory Thompson added a comment -

          I started to port my app over to android and has issues with the upload function. The total bytes to send wasn't being set.

          https://github.com/coryjthompson/incubator-cordova-android/commits/filetransfer-progress

          Just one line of code but works for me now.

          Show
          Cory Thompson added a comment - I started to port my app over to android and has issues with the upload function. The total bytes to send wasn't being set. https://github.com/coryjthompson/incubator-cordova-android/commits/filetransfer-progress Just one line of code but works for me now.
          Hide
          Andrew Grieve added a comment -

          Good stuff! I've made the fix, and also fixed us from assuming all content: URLs are FileInputStreams.

          Show
          Andrew Grieve added a comment - Good stuff! I've made the fix, and also fixed us from assuming all content: URLs are FileInputStreams.
          Hide
          Cory Thompson added a comment -

          Awesome! thanks

          Show
          Cory Thompson added a comment - Awesome! thanks
          Shazron Abdullah made changes -
          Link This issue relates to CB-1613 [ CB-1613 ]
          Hide
          Greg added a comment -

          Sorry to hijack this thread - but is there any docs how to do this with 2.2.0rc1?

          Show
          Greg added a comment - Sorry to hijack this thread - but is there any docs how to do this with 2.2.0rc1?
          Hide
          Andrew Grieve added a comment -

          Hey Greg, thanks for asking! I looks like updating the docs was overlooked. I've created a new bug to do so: https://issues.apache.org/jira/browse/CB-1704

          Show
          Andrew Grieve added a comment - Hey Greg, thanks for asking! I looks like updating the docs was overlooked. I've created a new bug to do so: https://issues.apache.org/jira/browse/CB-1704
          Hide
          Samik R added a comment -

          Cory/Anyone else: I just tried out file download progress bar. I first tested on android and works great. Then I tested on iPhone simulator (iOS 6.0), and it seems to me that progressEvent.lengthComputable is false, so I don't get any value for progressEvent.loaded. Does anyone know if iPhone simulator (and hence in iPhone/iPod etc.) webview always uses gzip encoding (assuming that is the issue here)?

          Show
          Samik R added a comment - Cory/Anyone else: I just tried out file download progress bar. I first tested on android and works great. Then I tested on iPhone simulator (iOS 6.0), and it seems to me that progressEvent.lengthComputable is false, so I don't get any value for progressEvent.loaded. Does anyone know if iPhone simulator (and hence in iPhone/iPod etc.) webview always uses gzip encoding (assuming that is the issue here)?
          Hide
          Andrew Grieve added a comment -

          gzip encoding is almost certainly the problem. There's another bug just for this: https://issues.apache.org/jira/browse/CB-1518
          It's unfortunately a low priority right now, so no one is working on it. If you'd like to take a stab at fixing it though, that would be awesome!

          If not, two ideas to help here:
          -Would using XHR2 address your use-case?
          -Might be able to disable gzip by setting the Accept-Encoding header to an empty string.

          Show
          Andrew Grieve added a comment - gzip encoding is almost certainly the problem. There's another bug just for this: https://issues.apache.org/jira/browse/CB-1518 It's unfortunately a low priority right now, so no one is working on it. If you'd like to take a stab at fixing it though, that would be awesome! If not, two ideas to help here: -Would using XHR2 address your use-case? -Might be able to disable gzip by setting the Accept-Encoding header to an empty string.

            People

            • Assignee:
              Andrew Grieve
              Reporter:
              Brion Vibber
            • Votes:
              6 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development