txiki.js icon indicating copy to clipboard operation
txiki.js copied to clipboard

ObjectURL api

Open EmixamPP opened this issue 1 year ago • 6 comments

ObjectURL API doc: https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL_static

It is more complicated than expected to solve #479.

I wanted to use the JS Blob.text() method to retrieve the content of the stored blob (as a JSValue) in a global blob store. But the function is async, and I'm not sure the JS_Call method can resolve and wait such a function, since it raises tjs__promise_rejection_tracker: Assertion '(JS_IsException(event)) == (0)' failed..

Anyway, here is a draft PR that shows what I've tried so far, maybe you will be able to help me. Or my solution is too bad (I have the feeling that it is maybe a bit too hacky?), and it will be easier for you to start from scratch when you will have time later (don't worry if this is the case).

While I'm waiting for your review, I'll keep trying to fix and improve the solution.

EmixamPP avatar Jun 17 '24 15:06 EmixamPP

Good start!

What I had in mind was perhaps a bit simpler:

  • Use the builtin SQLite module to store the Blob (probably in parts?) in an in-memory table
  • Use crypto.randomUUID() for the identifier

Then when creating the worker you can grab all the Blob parts straight from the SQLite DB and re-construct the content. SInce the sqlite module is sync there is no need to await anyhing.

saghul avatar Jun 20 '24 09:06 saghul

Sorry, I had to push force because I authored then committed with a wrong email.

So I got the api almost working in every case. Actually, in the test, I've added the revoke of the url after creating the worker. However, if you execute it a lot, sometimes, it will be done a bit too soon; before being queried by the eval. I will fix that.

I've used the crypto.randomUUID() for the url generation directly at the js side. And I will investigate then to use the sql module instead of using a global hash map as blob store.

I've made the new commits as fixup for their previous related commit, like this when the review is done, I can squash them automatically to keep a clean commit history, while being able to review them easily.

EmixamPP avatar Jun 20 '24 15:06 EmixamPP

Feel free to keep pushing changes, we can squash when merging.

saghul avatar Jun 20 '24 18:06 saghul

I have the feeling that going for an in-memory sqlite database may be overkill, so here I simply use a js Map(). Also, mainly because, after a re-read of the api here: https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL_static. I read that the liftime of the url is linked to the scope of the document (in the browser), so globally scoped across runtime object is unnecessary. Let me know what you think about that.

I got a working implementation. Now, maybe some work on the style and structure are needed? E.g. I made the use of a magic string between url.js and objecturl.c to know the name of the storage map. So, it may be better to create the storage map from C during the loading of the core module, but then, even if the API is not used, an empty map will be created.

I wanted to use the sessionStorage but that one only store string, so that is a problem here, since the string value cannot be retrieved synchronously in js.

EmixamPP avatar Jun 26 '24 11:06 EmixamPP

Rebase on master.

EmixamPP avatar Jun 27 '24 13:06 EmixamPP

Thank you! I did a full review round and left some comments.

saghul avatar Jun 27 '24 21:06 saghul

@EmixamPP I discovered the source of the problem! The fflate package which we use as part of the compression streams polyfill was using the Worker API while the engine was not fully ready, in order to detect support.

I disabled it since it didn't actually work, and squashed and force-pushed!

saghul avatar Jul 09 '24 21:07 saghul