[Webkit-unassigned] [Bug 63481] Bring V8's SerializedScriptValue implementation up to date

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=63481





--- Comment #19 from Luke Zarko <lukezarko+bugzilla at gmail.com>  2011-07-13 17:41:51 PST ---
(In reply to comment #18)
> (From update of attachment 100699 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=100699&action=review
> 
> ok comments on the tests now.
> 
> > LayoutTests/fast/dom/Window/script-tests/postmessage-clone-deep-array.js:1
> > +document.getElementById("description").innerHTML = "Tests that we abort cloning deep(ish) arrays.";
> 
> Why does this get a different error than really deep array?

The description was incorrect. deep-array passes, but there's a weird exception that happens when the harness tries to print the value to say that it passed (because the array is too deep for the default printer). JSC and V8 differ by a period in the error text, hence the different expectations. really-deep-array is intended to trigger the overflow behavior and is correctly described.

JSC doesn't currently use DATA_CLONE_ERR (code 25), hence the difference in expectation text for really-deep-array.

> 
> > LayoutTests/fast/dom/Window/script-tests/postmessage-clone.js:98
> > +        doPassFail(v[2] === undefined, "ix 2 undefined"); // undefined
> 
> ix = index, right? (pls xpnd)

Done.

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:1
> > +window.jsTestIsAsync = true;
> 
> Why is this a chromium test?

This behavior is not currently in the standard but is there to reflect inadvertent behavior from the previous version of the code that users have come to expect. That being said, I fully expect that this test will apply to the standard once it's completed; at that point we can promote it to a WebKit-global test.

> 
> If any port implementing webgl and array message passing should pass this, then it shouldn't be in a chromium directory.
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:2
> > +window.cheat = false;
> 
> When is cheat == true? I think this is debugging code that should be removed (along with the if below).

Correct; done.

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:7
> > +function classCompare(tname, g, s) {
> 
> s/tname/testName/
> 
> What do g and s stand for?

Done (got/sent).

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:8
> > +  var classStr = Object.prototype.toString;
> 
> avoid abbreviations: classString
> 
> Also ideally you'd stick with stand WebKit 4 space indents.

Done.

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:9
> > +  var gclass = classStr.call(g);
> 
> gClass (but hopefully with a better substitue for "g").

Done (gotClass).

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:98
> > +function createTypedArrayOverBuffer(tatype, taeltsize, length, substart, sublength) {
> 
> s/tatype/typedArrayType/
> s/taeltsize/typedArray??Size/

Done (Element).

> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:140
> > +  {return [t[0] + "_0b", createTypedArrayOverBuffer(t[1], t[2], 0), typedArrayCompare];}));
> 
> I wish the "b" of 0b was expanded so I didn't have to try to figure out what it means.

Done (_buffer); others expanded too.

> 
> > LayoutTests/platform/chromium/fast/dom/Window/script-tests/postmessage-clone-ex.js:1
> > +document.getElementById("description").innerHTML = "Tests that we clone object hierarchies";
> 
> Why is this chromium only and why does it appear to copy so much from the other similar test?

My bad. It was a leftover from when I was figuring out how to write platform tests. Nothing referenced it and it has been garbage collected.

(In reply to comment #18)
> (From update of attachment 100699 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=100699&action=review
> 
> ok comments on the tests now.
> 
> > LayoutTests/fast/dom/Window/script-tests/postmessage-clone-deep-array.js:1
> > +document.getElementById("description").innerHTML = "Tests that we abort cloning deep(ish) arrays.";
> 
> Why does this get a different error than really deep array?
> 
> > LayoutTests/fast/dom/Window/script-tests/postmessage-clone.js:98
> > +        doPassFail(v[2] === undefined, "ix 2 undefined"); // undefined
> 
> ix = index, right? (pls xpnd)
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:1
> > +window.jsTestIsAsync = true;
> 
> Why is this a chromium test?
> 
> If any port implementing webgl and array message passing should pass this, then it shouldn't be in a chromium directory.
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:2
> > +window.cheat = false;
> 
> When is cheat == true? I think this is debugging code that should be removed (along with the if below).
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:7
> > +function classCompare(tname, g, s) {
> 
> s/tname/testName/
> 
> What do g and s stand for?
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:8
> > +  var classStr = Object.prototype.toString;
> 
> avoid abbreviations: classString
> 
> Also ideally you'd stick with stand WebKit 4 space indents.
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:9
> > +  var gclass = classStr.call(g);
> 
> gClass (but hopefully with a better substitue for "g").
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:98
> > +function createTypedArrayOverBuffer(tatype, taeltsize, length, substart, sublength) {
> 
> s/tatype/typedArrayType/
> s/taeltsize/typedArray??Size/
> 
> > LayoutTests/platform/chromium/fast/canvas/webgl/script-tests/array-message-passing.js:140
> > +  {return [t[0] + "_0b", createTypedArrayOverBuffer(t[1], t[2], 0), typedArrayCompare];}));
> 
> I wish the "b" of 0b was expanded so I didn't have to try to figure out what it means.
> 
> > LayoutTests/platform/chromium/fast/dom/Window/script-tests/postmessage-clone-ex.js:1
> > +document.getElementById("description").innerHTML = "Tests that we clone object hierarchies";
> 
> Why is this chromium only and why does it appear to copy so much from the other similar test?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.