550275 - Implement the HTML5 structured clone algorithm

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

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

Attachment: Rough WIP, nonrecursive version

http://www.whatwg.org/specs/web-apps/current-work/multipage/urls.html#safe-passing-of-structured-data

This will allow us to clone JS objects without using JSON for postMessage, etc.

The algorithm is recursive and I've got a recursive version of it implemented but I like the nonrecursive version better.

Comment 1

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

Attachment: Patch, v1

This should do it, for now.

Comment 2

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

Attachment: Make workers use structured clone, v1

Comment 3

[Jeff Walden [:Waldo]]

window.postMessage should be using this as well, right?  Sibling patch to the worker-specific bits, methinks.

Comment 4

[Johnny Stenback (:jst)]

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

Looks good to me, but I do think mrbkap or someone in the JS team should look over the JS specific parts, especially the changes to the JS engine headers. I know Andreas was going through and doing a bunch of the opposite of what this patch does in the headers, so he'll probably have some thoughts on this too.

Comment 5

[Brendan Eich [:brendan]]

(In reply to comment #4)
> (From update of attachment 430789 [details] [diff] [review])
> Looks good to me, but I do think mrbkap or someone in the JS team should look
> over the JS specific parts, especially the changes to the JS engine headers. I
> know Andreas was going through and doing a bunch of the opposite of what this
> patch does in the headers, so he'll probably have some thoughts on this too.

Andreas was removing #include "js*.h" from nsContentUtils.h and other widely included .h files. That's bad and to be avoided, but narrow #includes in a .cpp or two are ok, or at least not as bad. Seems like we could provide first-class API and avoid these, though. Followup bug?

/be

Comment 6

[Andreas Gal :gal]

If anyone tries to include (again) non-public JS headers (anything but jsapi.h and jspubtd.h) in external header files, especially header files that get included all over the place (hi there nsContentUtils.h), I am going to get out the pitchforks. Removing a couple lines reduced my re-build time by 30 minutes (no more rebuilding SVG is you touch jsnum.h). Including stuff in .cpp files is ugly, but not that big of a deal (as far as build time is concerned). I will touch base with bent to see what proper external interface we might need here.

Comment 7

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

gal says this friend-y stuff is fine for now, I'll file a followup on exposing proper API for this.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

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

I talked this over with Ben and gave him some comments. He'll post a new patch.

Comment 9

[Andreas Gal :gal]

A random idea, feel free to disregard. Using proxies you could do lazy on-demand copying. You grab the root object, make a transparent proxy for it, send it over. If any parts of it are touched, it generates proxies for all of its object properties, and then fixes itself (turns itself into a regular object, fast, no trace of it being formerly a proxy). If the objects are touched, the same happens and propagates until everything has been pulled over.

Comment 10

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

Attachment: Patch, v1.1

This patch incorporates fixes suggested by mrbkap. Specifically, we're using auto-helpers in CloneStackFrame now, using JSVAL_IS_PRIMITIVE, JS_DefinePropertyById (instead of JS_SetPropertyById) and aborting reparenting when we see a JSObject that has a non-standard proto key.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 433118 [details] [diff] [review]
Patch, v1.1

I like it! I asked bent on IRC about a few small problems that he'll fix. r=mrbkap with those fixed.

Comment 12

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

http://hg.mozilla.org/mozilla-central/rev/5ae1a1d05ebb
http://hg.mozilla.org/mozilla-central/rev/2e9bb82a9eb3

Comment 13

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

Filed followups:

bug 553125
bug 553127
bug 553128