613083 - IndexedDB: Switch serialization format from JSON to structured clone bytestream

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

We don't want to have to support JSON forever, let's just move to the structured clone bytestream format.

Comment 1

[Johnny Stenback (:jst)]

Agreed. Let's do this before we ship Firefox 4.

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Note that we need to bake in the structured clone version in every object store or db, somehow. That version number needs to be passed to the structured clone read functions.

Comment 3

[Shawn Wilsher :sdwilsh]

(In reply to comment #2)
> Note that we need to bake in the structured clone version in every object store
> or db, somehow. That version number needs to be passed to the structured clone
> read functions.
Ben, did you mean to removing the blocking flag with this comment?

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

No :(

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 498180 [details] [diff] [review]
Patch, v1

>@@ -408,19 +409,20 @@ IDBCursor::GetValue(JSContext* aCx,
>-    nsCOMPtr<nsIJSON> json(new nsJSON());
>-    rv = json->DecodeToJSVal(mValue, aCx, &mCachedValue);
>-    NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_DATA_CLONE_ERR);
>+    if (!mCloneBuffer.read(&mCachedValue, aCx)) {
>+      mCachedValue = JSVAL_VOID;
>+      return NS_ERROR_DOM_DATA_CLONE_ERR;
>+    }

Should you free the buffer-data after this to free up the memory as soon as possible. The buffer is never used again, right? 

r=me

Comment 7

[Jason Orendorff [:jorendorff]]

I'm a fan of fixing this bug -- but I'm on vacation until the new year.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #7)
> I'm a fan of fixing this bug -- but I'm on vacation until the new year.

Ok, thanks. I think I'll land it now since the changes to indexedDB are huge and the changes to jsapi are small (I only tweaked the JSAutoStructuredCloneBuffer class to accept more than one context). I'd still like your review, and we can file a followup if you want any changes.

Comment 9

[Ben Turner (not reading bugmail, use the needinfo flag!)]

http://hg.mozilla.org/mozilla-central/rev/1b1561128c5d

Comment 10

[Jason Orendorff [:jorendorff]]

Comment on attachment 498180 [details] [diff] [review]
Patch, v1

Mmm. r=me, but this API keeps getting uglier. Something's wrong. It would be nice to try and figure out what that is at some point.