Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update for structured cloning changes in HTML #170

Closed
domenic opened this issue Mar 20, 2017 · 8 comments
Closed

Update for structured cloning changes in HTML #170

domenic opened this issue Mar 20, 2017 · 8 comments
Assignees

Comments

@domenic
Copy link
Contributor

domenic commented Mar 20, 2017

In whatwg/html@97d644c HTML changed the setup around structured cloning significantly, in a way that will hopefully benefit IDB a lot. You can now use the StructuredSerialize and StructuredDeserialize operations directly.

Let me know if I can help with this.

@inexorabletash
Copy link
Member

inexorabletash commented Mar 20, 2017

Thanks. PRs would be welcome, of course, but I'll wrangle it. :)

The fun bit is where we currently do a clone then (1) look up properties against the clone to avoid side effects - primary key at https://w3c.github.io/IndexedDB/#dom-idbobjectstore-put and index keys at https://w3c.github.io/IndexedDB/#store-a-record-into-an-object-store which looks at the index keys, and (2) inject auto-generated keys in https://w3c.github.io/IndexedDB/#store-a-record-into-an-object-store

I'll have to see if this can be expressed in terms of serialized records or if we will end up doing a deserialization and reserialization.

@annevk
Copy link
Member

annevk commented Mar 21, 2017

The other thing Indexed DB should probably define is some kind of Store operation, to define how these records end up on disk (and what further changes need to be made for that).

@inexorabletash
Copy link
Member

define how these records end up on disk

Are you thinking of hooks for Storage (which makes sense - e.g. checking for sufficient quota / updating usage), or something specific to serializing the records (which seems like an implementation detail to me)?

@annevk
Copy link
Member

annevk commented Mar 21, 2017

In particular I think it's not quite an implementation-detail for File, Blob, and SharedArrayBuffer objects. E.g., does File continue to be a pointer or do we make a copy and the same question applies to SharedArrayBuffer. I think the likely answers are different for both.

But it would indeed also be good to integrate with Storage (which also makes it clearer what happens if you clear a box, that Indexed DB disappears too).

@inexorabletash
Copy link
Member

Of course, this had to materialize the day we were planning to advance Indexed DB 2.0 to CR. sigh

@inexorabletash
Copy link
Member

#171 for the minimum/initial set of changes, without digging into the work @annevk mentions

inexorabletash added a commit that referenced this issue Apr 10, 2017
Minimal changes to replace uses of StructuredClone with StructuredSerialize/StructuredDeserialize, per #170 to track latest [HTML] definitions.

Note that when put() (etc) is called that this does not simply serialize then operate on the output record - the record is immediately deserialized to a clone in an abstract targetRealm so that subsequent operations (extract a key, inject a key, etc) are not updated. I added non-normative details explaining this at the various sites.

(Somewhat coincidentally, that's how Blink is actually implemented, since the output of serialization is a opaque set of bytes rather than an easily-traversed structure.)

The definition of operations that operate on the clone could be changed to operate on the record instead. This would require redoing the "extract a key from a value" and "inject a key into a value" section (replace "value" with "record"), and duplicating the "convert a key to a value" and "convert a value to a key" steps with "record" variants.
@inexorabletash
Copy link
Member

Note that SharedArrayBuffer is taken care of c/o #152 / #197

File/Blob could use more definition still.

@inexorabletash
Copy link
Member

Since this is mostly done, forked #203 for the specifics of a storage operation distinct beyond StructuredSerializeForStorage.

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

No branches or pull requests

3 participants