Closed gwdp closed 2 years ago
This also resolved a rather new issues I was facing with Chrome 98 and StreamSaver.js. Thank you for sharing the patch.
@jimmywarting Can we get a merge on it? 🥺
yea, sure. it's a quick and easy fix i would rather want to rewrite the hole communication as discussed in https://github.com/jimmywarting/StreamSaver.js/issues/13#issuecomment-1013531389 but haven't had the time to do it...
also this needs to be backwards compatible also cuz it depends on a public service worker that everyone depends on. so a rewrite would be tough... also i don't know how much love i want to put into it now that we have transferable streams and even https://github.com/whatwg/fs/ at our disposal
Since #13 was reported, Chrome had an issue where https://github.com/jimmywarting/StreamSaver.js/blob/cd8e32a84f951541c5be183b2b409bd17dbb16cd/sw.js#L66 would not be fired, and I believe, because of that nothing was ever implemented on this handler although it was always called by Firefox.
Also, reported on #13, ticket was filled in for the chrome team (ticket #638494) a few years ago and it got merged last week.
Testing on Canary version
99.0.4828.0
, cancellation event still not being triggered but worker seems to be killed on user cancellation causing stream saver to write to throw atwrite()
. (more investigation on this behaviour is actually needed but it does not allow more writing on the main thread as expected).Now, to make sure we do abort the operation at user cancellation on Firefox, that does not kill the worker but just closes the file descriptor and maintains the worker running without sending any signal to the main thread the proposed changes are implemented.