Skip to content

Commit

Permalink
Semi-workaround for #205
Browse files Browse the repository at this point in the history 
This solution still isn't technically correct because it could take more
than 40 seconds to download a file. I could simply not revoke any files,
but then users would run into issues once they have saved a cumulative
total of over 500MB (
https://bugs.chromium.org/p/chromium/issues/detail?id=375297 )

Complain to @sicking and @arunranga for creating and standardizing an
incomplete revocation API.
  • Loading branch information
@eligrey
eligrey committed Mar 28, 2016
1 parent 7ef5187 commit 62d219a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 10 deletions.
28 changes: 19 additions & 9 deletions FileSaver.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* FileSaver.js
* A saveAs() FileSaver implementation.
* 1.1.20160319
* 1.1.20160328
*
* By Eli Grey, http://eligrey.com
* License: MIT
Expand Down Expand Up @@ -40,10 +40,8 @@ var saveAs = saveAs || (function(view) {
}
, force_saveable_type = "application/octet-stream"
, fs_min_size = 0
// See https://code.google.com/p/chromium/issues/detail?id=375297#c7 and
// https://github.com/eligrey/FileSaver.js/commit/485930a#commitcomment-8768047
// for the reasoning behind the timeout and revocation flow
, arbitrary_revoke_timeout = 500 // in ms
// the Blob API is fundamentally broken as there is no "downloadfinished" event to subscribe to
, arbitrary_revoke_timeout = 1000 * 40 // in ms
, revoke = function(file) {
var revoker = function() {
if (typeof file === "string") { // file is an object URL
Expand All @@ -52,11 +50,23 @@ var saveAs = saveAs || (function(view) {
file.remove();
}
};
if (view.chrome) {
revoker();
} else {
setTimeout(revoker, arbitrary_revoke_timeout);
/* // Take note W3C:
var
uri = typeof file === "string" ? file : file.toURL()
, revoker = function(evt) {
// idealy DownloadFinishedEvent.data would be the URL requested
if (evt.data === uri) {
if (typeof file === "string") { // file is an object URL
get_URL().revokeObjectURL(file);
} else { // file is a File
file.remove();
}
}
}
;
view.addEventListener("downloadfinished", revoker);
*/
setTimeout(revoker, arbitrary_revoke_timeout);
}
, dispatch = function(filesaver, event_types, event) {
event_types = [].concat(event_types);
Expand Down
2 changes: 1 addition & 1 deletion FileSaver.min.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 comments on commit 62d219a

@sicking
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also hard to fix problems in the spec if no one lets us know about the problem. Looking at the patch I wasn't able to figure out what problem you are referring to.

Please file an issue on the spec if there's a problem with Blob.close()

@eligrey
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sicking Blob.close() works fine, but how am I supposed to know when it's safe to call it? My library saves Blobs, and I need to revoke object URLs and Blobs to make room for more (500MB limit in Chrome, similar or higher limits in other browsers) in case the user saves multiple files. There's no event for me to listen to, so I'm just arbitrarily waiting for 40 seconds.

@sicking
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you save the blob? Generally one a read has started it should be safe to call Blob.close(), it should not affect read operations, like that of FileReader or <a href="blob:..." download>, that are already in progress.

If it does then please file a bug on the browser.

@eligrey
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you save the blob?

For browsers that support a[download], here is what I do, simplified:

var link = document.createElementNS(HTML, "a");
link.download = filename;
link.href = object URL;
link.click();
setTimeout(revoke, some random number that I felt was enough);

Generally one a read has started it should be safe to call Blob.close()

Not a single browser implements saving blobs like this (not Chrome, not Firefox, not the old Opera, not the new Chromium-based Opera, n/a in Edge & IE as they implement saveAs natively (as navigator.msSaveBlob), and n/a in Safari as Safari doesn't support a[download]). If you revoke the object URL and close the Blob immediately after starting a 50MB download, you can be sure that the download will fail with an error.

@sicking
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do make sure that the download is actually started before revoking the URL, or calling .close(). In both Firefox and Chrome, if I revoke the URL after the download has actually started the download finishes just fine.

Firefox pops up a save-as dialog which the user has to interact with before the download starts. I wouldn't be surprised if there's a small delay between when .click() is called and chrome actually starts the download too.

I agree that this is a problem since there's no notification about when the download starts. But there wouldn't be much we could do in the Blob API to indicate when it's safe to revoke a URL or to call .close() if these other APIs haven't even started to read from the blob.

The fix here would either be to introduce a save-as API which takes an actual Blob object, or to fire some form of event from the when the download actually has started (i.e. after any needed user approval).

@eligrey
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sicking Would you mind raising this on a relevant W3C mailing list when you get the time?

@sicking
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed a bug to have an event when the download starts or finishes. If you're still able to cancel the download by revoking the URL in chrome, then please file a bug there. I wasn't able to reproduce so I didn't file.

@eligrey
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Can I get a link to the event bug?

@sicking
Copy link

@sicking sicking commented on 62d219a Mar 29, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.