Bug 40835 – Adds support for throwing behavior of SerializedScriptValue to be specified in the IDL.

• Status: UNCONFIRMED
• Component: New Bugs

[Marcus Bulach]

Adds support for throwing behavior of SerializedScriptValue to be specified in the IDL.

[Marcus Bulach]

Created attachment 59113 [details]
Patch

[WebKit Review Bot]

Attachment 59113 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:177:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:179:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:180:  _g_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:181:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:188:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:191:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:204:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:206:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 8 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

jorlow: I took over your patch (thanks!), fixed a couple of nits, and WebKitTools/Scripts/run-bindings-tests seems to be happy: "All tests passed!".

would you mind taking another look? also, please, let me know if there's any other test I should run, or anybody else that should be cc:d on this bug.

thanks!

[Early Warning System Bot]

Attachment 59113 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3267314

[Jeremy Orlow]

Can you please write some layout tests to verify that we are doing the right thing now (and verify we weren't before)?

checking PopStateEvent.init throws and that IDBObjectStore.put doesn't (and a FIXME to verify that it does throw an onError) is probably enough

I took a glance and it looks good to me, but since I wrote a bunch of the original code, someone else should review this.

[Jeremy Orlow]

Comment on attachment 59113 [details]
Patch

Removing from review queue until there are layout tests.

[Marcus Bulach]

Created attachment 59274 [details]
Patch

[WebKit Review Bot]

Attachment 59274 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:181:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:207:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 3 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

(In reply to comment #6)
> (From update of attachment 59113 [details])
> Removing from review queue until there are layout tests.

thanks Jeremy! added layout tests for both PopStateEvent and IDBObjectStore, another look please? feel free to add anyone else who should review this patch.

[Early Warning System Bot]

Attachment 59274 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3334542

[WebKit Review Bot]

Attachment 59274 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3281563

[Marcus Bulach]

Comment on attachment 59274 [details]
Patch

ops, wait a bit, need to fix the JS generator.. will upload a new patch shortly.

[Marcus Bulach]

Created attachment 59362 [details]
Patch

[Marcus Bulach]

fixed the JS generator, and added a TestPODObj.idl to exercise the binding generator for interfaces with [PODType=] attribute.. another look please?

(In reply to comment #13)
> Created an attachment (id=59362) [details]
> Patch

[WebKit Review Bot]

Attachment 59362 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
our identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:116:  prop_id is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:122:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  webkit_dom_test_pod_obj_get_property is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  prop_id is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:139:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:155:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:157:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:182:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:181:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:207:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 30 in 38 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Attachment 59362 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/3341605

[Marcus Bulach]

Comment on attachment 59362 [details]
Patch

not there yet... :( let me fix the new issue.

[WebKit Review Bot]

Attachment 59362 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3282546

[Marcus Bulach]

Created attachment 59405 [details]
Patch

[Marcus Bulach]

ok, I managed to compile Qt and run the tests locally, let's hope it makes the bots just as happy... if everything (except the style of autogenerated code) is green, would you mind another look please? thanks!

(In reply to comment #17)
> (From update of attachment 59362 [details])
> not there yet... :( let me fix the new issue.

[WebKit Review Bot]

Attachment 59405 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
our identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:116:  prop_id is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:122:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  webkit_dom_test_pod_obj_get_property is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  prop_id is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:139:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:155:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:157:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:182:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:181:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:207:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 30 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Jeremy Orlow]

Comment on attachment 59405 [details]
Patch

WebCore/bindings/js/SerializedScriptValue.cpp:987
 +      PassRefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));
RefPtr not PassRefPtr

WebCore/bindings/js/SerializedScriptValue.cpp:994
 +      return serializedValue;
.release() here.

WebCore/bindings/js/SerializedScriptValue.cpp:981
 +      PassRefPtr<SerializedScriptValue> serializedValue = SerializedScriptValue::create(exec, value, &ec);
Don't you need to set the dom exception here as well?
Also don't use PassRefPtr within a function.

WebCore/bindings/scripts/CodeGeneratorJS.pm:1719
 +                                      push(@implContent, "    podImp = " . JSValueToNative($attribute->signature, "value", 0) . ";\n");
Use booleans.

WebCore/bindings/scripts/CodeGeneratorJS.pm:1721
 +                                      push(@implContent, "    podImp.set$implSetterFunctionName(" . JSValueToNative($attribute->signature, "value", 0) . ");\n");
ditto (and elsewhere)

LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:1
 +  description("Test IndexedDB's openCursor.");
Update the description.

LayoutTests/storage/indexeddb/resources/shared.js:52
 +      testFailed("Success function called unexpectedly: (" + event.code + ") " + event.message);
There will be no code/message.

LayoutTests/fast/loader/stateobjects/initpopstateevent-expected.txt:5
 +  PASS serialized script value threw an exception due to a cycle.
Put a message in the beginning saying it'll print "COMPLETE" when done and then add code to do that.

[Marcus Bulach]

Created attachment 59888 [details]
Patch

[WebKit Review Bot]

Attachment 59888 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  webkit_dom_test_pod_obj_get_property is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:133:  prop_id is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:139:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:155:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:157:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:182:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:181:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:198:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:207:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:208:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 33 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

thanks jeremy! all comments addressed, plus one clarification below. another look please?

(In reply to comment #22)
> (From update of attachment 59405 [details])
> WebCore/bindings/js/SerializedScriptValue.cpp:987
>  +      PassRefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));
> RefPtr not PassRefPtr

Done.

> 
> WebCore/bindings/js/SerializedScriptValue.cpp:994
>  +      return serializedValue;
> .release() here.

Done.

> 
> WebCore/bindings/js/SerializedScriptValue.cpp:981
>  +      PassRefPtr<SerializedScriptValue> serializedValue = SerializedScriptValue::create(exec, value, &ec);
> Don't you need to set the dom exception here as well?
Not really, this is just calling the factory below (which sets dom exception).

> Also don't use PassRefPtr within a function.
Done.

> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:1719
>  +                                      push(@implContent, "    podImp = " . JSValueToNative($attribute->signature, "value", 0) . ";\n");
> Use booleans.
Done.

> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:1721
>  +                                      push(@implContent, "    podImp.set$implSetterFunctionName(" . JSValueToNative($attribute->signature, "value", 0) . ");\n");
> ditto (and elsewhere)
Done.


> 
> LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:1
>  +  description("Test IndexedDB's openCursor.");
> Update the description.
ouch, done.

> 
> LayoutTests/storage/indexeddb/resources/shared.js:52
>  +      testFailed("Success function called unexpectedly: (" + event.code + ") " + event.message);
> There will be no code/message.
removed.

> 
> LayoutTests/fast/loader/stateobjects/initpopstateevent-expected.txt:5
>  +  PASS serialized script value threw an exception due to a cycle.
> Put a message in the beginning saying it'll print "COMPLETE" when done and then add code to do that.
done.

[Jeremy Orlow]

Comment on attachment 59888 [details]
Patch

LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:8
 +      verifyErrorEvent(event);
Assert that we're raising a serialization error?  Or...make a fixme to do this, since it probably doesn't work right today.  :-)

LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:18
 +      var a = {};
Maybe make a function in the shared script for this?  createUnserializableValue()  ?

LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:23
 +          testFailed("objStore.add threw an unexpected exception: " + err);
And then notify done + return?

WebCore/bindings/js/SerializedScriptValue.cpp:972
 +      PassRefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));
This should be a RefPtr...later return serializedValue.release();

WebCore/bindings/js/SerializedScriptValue.cpp:990
 +      } else {
No {}'s on the second half

WebCore/bindings/scripts/test/TestPODObj.idl:13
 +   * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
Just use a 2 clause license.

WebCore/bindings/scripts/test/TestPODObj.idl:2
 +   * Copyright (C) 2009 Google Inc. All rights reserved.
2010

WebCore/bindings/scripts/test/TestPODObj.idl:34
 +                   attribute [ConvertNullToNullString] DOMString stringAttr;
Not sure this indenting is necessary.

WebCore/bindings/scripts/test/TestObj.idl:65
 +          // Test various forms of SerializedScriptValue
period at end.

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:798
 +      RefPtr<SerializedScriptValue> serializedArg = SerializedScriptValue::create(exec, exec->argument(0), &ec);
Hmm...do we need to generate bail out code for if ec is !0?

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:828
 +      JSC::JSValue result = imp->serializedValueReturnRaises(ec) ? imp->serializedValueReturnRaises(ec)->deserialize(exec, castedThis->globalObject()) : jsNull();
Hu?  This seems very not right.  :-)
At least make sure the bindings generator kills itself rather than emit stuff like this.

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:842
 +      JSC::JSValue result = imp->serializedValueReturnNoRaise() ? imp->serializedValueReturnNoRaise()->deserialize(exec, castedThis->globalObject()) : jsNull();
ditto

[Marcus Bulach]

Created attachment 60033 [details]
Patch

[WebKit Review Bot]

Attachment 60033 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
bject/WebKitDOMTestPODObj.cpp:145:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:161:  webkit_dom_test_pod_obj_constructed is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:168:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:170:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:196:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:211:  Extra space between return and WEBKIT_DOM_TEST_POD_OBJ  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:212:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:204:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:210:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:220:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:221:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 36 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

(In reply to comment #26)
> (From update of attachment 59888 [details])
> LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:8
>  +      verifyErrorEvent(event);
> Assert that we're raising a serialization error?  Or...make a fixme to do this, since it probably doesn't work right today.  :-)
yeah, there was a FIXME further down where I'm short-circuiting this :)
but good point, added another one here.


> 
> LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:18
>  +      var a = {};
> Maybe make a function in the shared script for this?  createUnserializableValue()  ?
done.

> 
> LayoutTests/storage/indexeddb/script-tests/idb-add-serialized-script-value.js:23
>  +          testFailed("objStore.add threw an unexpected exception: " + err);
> And then notify done + return?
done.

> 
> WebCore/bindings/js/SerializedScriptValue.cpp:972
>  +      PassRefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));
> This should be a RefPtr...later return serializedValue.release();
done.

> 
> WebCore/bindings/js/SerializedScriptValue.cpp:990
>  +      } else {
> No {}'s on the second half
done.

> 
> WebCore/bindings/scripts/test/TestPODObj.idl:13
>  +   * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> Just use a 2 clause license.
sigh, I'll keep this in mind when copying from somewhere else next time.. thanks!

> 
> WebCore/bindings/scripts/test/TestPODObj.idl:2
>  +   * Copyright (C) 2009 Google Inc. All rights reserved.
> 2010
copy the whole license from somewhere else.

> 
> WebCore/bindings/scripts/test/TestPODObj.idl:34
>  +                   attribute [ConvertNullToNullString] DOMString stringAttr;
> Not sure this indenting is necessary.
removed.

> 
> WebCore/bindings/scripts/test/TestObj.idl:65
>  +          // Test various forms of SerializedScriptValue
> period at end.
done.

> 
> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:798
>  +      RefPtr<SerializedScriptValue> serializedArg = SerializedScriptValue::create(exec, exec->argument(0), &ec);
> Hmm...do we need to generate bail out code for if ec is !0?
no idea.. :( won't the setDOMException() below take care of it?

> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:828
>  +      JSC::JSValue result = imp->serializedValueReturnRaises(ec) ? imp->serializedValueReturnRaises(ec)->deserialize(exec, castedThis->globalObject()) : jsNull();
> Hu?  This seems very not right.  :-)
> At least make sure the bindings generator kills itself rather than emit stuff like this.

fixed. getting imp->serializedValueReturn[Raises|NoRaise]() as a temp, and then testing it (plus ec, if available), and returning jsNull() instead.
note that this code would call the function twice, but wasn't intrinsically wrong AFAICT, since the function that returns the serializedValue should be const anyway..

oh, the joys of perl. :) would please double check the generator bits as well? thanks!

> 
> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:842
>  +      JSC::JSValue result = imp->serializedValueReturnNoRaise() ? imp->serializedValueReturnNoRaise()->deserialize(exec, castedThis->globalObject()) : jsNull();
> ditto

fixed as above..

[Jeremy Orlow]

(In reply to comment #29)
> > WebCore/bindings/scripts/test/JS/JSTestObj.cpp:798
> >  +      RefPtr<SerializedScriptValue> serializedArg = SerializedScriptValue::create(exec, exec->argument(0), &ec);
> > Hmm...do we need to generate bail out code for if ec is !0?
> no idea.. :( won't the setDOMException() below take care of it?

Well, we don't want to run the webCore method unless we haven't hit an error at this point.  See what Chromium does.  (Not sure if this is what you did or not...won't have time to look until tonight.)

[Marcus Bulach]

Created attachment 60111 [details]
Patch

[WebKit Review Bot]

Attachment 60111 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
bject/WebKitDOMTestPODObj.cpp:145:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:161:  webkit_dom_test_pod_obj_constructed is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:168:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:170:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:196:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:211:  Extra space between return and WEBKIT_DOM_TEST_POD_OBJ  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:212:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:204:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:210:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:220:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:221:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 36 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

(In reply to comment #30)
> (In reply to comment #29)
> > > WebCore/bindings/scripts/test/JS/JSTestObj.cpp:798
> > >  +      RefPtr<SerializedScriptValue> serializedArg = SerializedScriptValue::create(exec, exec->argument(0), &ec);
> > > Hmm...do we need to generate bail out code for if ec is !0?
> > no idea.. :( won't the setDOMException() below take care of it?
> 
> Well, we don't want to run the webCore method unless we haven't hit an error at this point.  See what Chromium does.  (Not sure if this is what you did or not...won't have time to look until tonight.)

got it. the generated code now sets the exception code and returns immediately on exception, without calling the webcore method in the latest patch.

[Jeremy Orlow]

Comment on attachment 60111 [details]
Patch

WebCore/bindings/js/SerializedScriptValue.h:179
 +          static PassRefPtr<SerializedScriptValue> createWithoutThrowing(JSC::ExecState* exec, JSC::JSValue value);
don't need exec or value here...or below

WebCore/bindings/js/SerializedScriptValue.h:183
 +          static PassRefPtr<SerializedScriptValue> create(JSC::ExecState* exec, JSC::JSValue value, ExceptionCode* ec);
don't need ec

WebCore/bindings/scripts/CodeGeneratorJS.pm:2234
 +          push(@implContent, "\n" . NativeSerializedScriptValueToJSValue($function->signature, $functionString, @{$function->raisesExceptions})) if $function->signature->type eq "SerializedScriptValue";
what about any?  be sure to test for this if it's true that you should have had "any"?

WebCore/bindings/scripts/CodeGeneratorJS.pm:2473
 +      if ($type eq "SerializedScriptValue" or $type eq "any") {
Do you actually need this?  Can't you assume it?

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:832
 +      SerializedScriptValue* serializedScriptValue = imp->serializedValueReturnRaises(ec);
Wouldn't this need to be a PassRefPtr?

Does anything even use this code yet?  If not, then maybe we shouldn't test for it and/or worry about it.

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:833
 +      JSC::JSValue result = serializedScriptValue && !ec ? serializedScriptValue->deserialized() : jsNull();
if ec is set, we should return undefined.  Otherwise null.

[Marcus Bulach]

Created attachment 60241 [details]
Patch

[WebKit Review Bot]

Attachment 60241 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
bject/WebKitDOMTestPODObj.cpp:145:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:161:  webkit_dom_test_pod_obj_constructed is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:168:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:170:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:196:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:211:  Extra space between return and WEBKIT_DOM_TEST_POD_OBJ  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:212:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:192:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:204:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:210:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:220:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:221:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 36 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Marcus Bulach]

thanks jeremy! another look please?

(In reply to comment #34)
> (From update of attachment 60111 [details])
> WebCore/bindings/js/SerializedScriptValue.h:179
>  +          static PassRefPtr<SerializedScriptValue> createWithoutThrowing(JSC::ExecState* exec, JSC::JSValue value);
> don't need exec or value here...or below
done

> 
> WebCore/bindings/js/SerializedScriptValue.h:183
>  +          static PassRefPtr<SerializedScriptValue> create(JSC::ExecState* exec, JSC::JSValue value, ExceptionCode* ec);
> don't need ec
done.

> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:2234
>  +          push(@implContent, "\n" . NativeSerializedScriptValueToJSValue($function->signature, $functionString, @{$function->raisesExceptions})) if $function->signature->type eq "SerializedScriptValue";
> what about any?  be sure to test for this if it's true that you should have had "any"?
right, testing for both SerializedScriptValue and any.

> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:2473
>  +      if ($type eq "SerializedScriptValue" or $type eq "any") {
> Do you actually need this?  Can't you assume it?
good point.. changed so that the caller tests it.

> 
> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:832
>  +      SerializedScriptValue* serializedScriptValue = imp->serializedValueReturnRaises(ec);
> Wouldn't this need to be a PassRefPtr?
done.

> 
> Does anything even use this code yet?  If not, then maybe we shouldn't test for it and/or worry about it.
well, I think it's better to have it now rather than worry about it later? as in, as far as i can tell it's a valid case and we want to make sure the generator will "do the right thing"?

> 
> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:833
>  +      JSC::JSValue result = serializedScriptValue && !ec ? serializedScriptValue->deserialized() : jsNull();
> if ec is set, we should return undefined.  Otherwise null.
done.

[Jeremy Orlow]

(In reply to comment #37)
> > Does anything even use this code yet?  If not, then maybe we shouldn't test for it and/or worry about it.
> well, I think it's better to have it now rather than worry about it later? as in, as far as i can tell it's a valid case and we want to make sure the generator will "do the right thing"?

For better or for worse, what the generator supports is generally built up on demand.  This means that often when you add something to an IDL you have to add support into the generators.

Will look at the patch tomorrow.  Sorry for these long cycles, but it's midnight here and I'm pretty tired.

[Jeremy Orlow]

Comment on attachment 60241 [details]
Patch

WebCore/bindings/scripts/test/JS/JSTestObj.cpp:833
 +      JSC::JSValue result =  ec ? jsUndefined() :  serializedScriptValue ? serializedScriptValue->deserialized() : jsNull();
Personally, I think this is cleaner:

if (ec) {
    setDOMException
    return jsUndefined;
}
return serializedScriptVlaue->deserilized()

WebCore/bindings/scripts/CodeGeneratorJS.pm:2475
 +      $ret = $ret . " serializedScriptValue ? serializedScriptValue->deserialized() : jsNull();\n";
you get double spaces between a few items here...not that big of a deal tho



r=me

[Marcus Bulach]

Created attachment 62810 [details]
Patch

[Marcus Bulach]

sorry about the delay, this one went under my radar. both comments addressed, another look please?

(In reply to comment #39)
> (From update of attachment 60241 [details])
> WebCore/bindings/scripts/test/JS/JSTestObj.cpp:833
>  +      JSC::JSValue result =  ec ? jsUndefined() :  serializedScriptValue ? serializedScriptValue->deserialized() : jsNull();
> Personally, I think this is cleaner:
> 
> if (ec) {
>     setDOMException
>     return jsUndefined;
> }
> return serializedScriptVlaue->deserilized()
> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:2475
>  +      $ret = $ret . " serializedScriptValue ? serializedScriptValue->deserialized() : jsNull();\n";
> you get double spaces between a few items here...not that big of a deal tho
> 
> 
> 
> r=me

[WebKit Review Bot]

Attachment 62810 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Last 3072 characters of output:
bject/WebKitDOMTestPODObj.cpp:145:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:161:  webkit_dom_test_pod_obj_constructed is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:168:  webkit_dom_test_pod_obj_class_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:170:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:196:  webkit_dom_test_pod_obj_init is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:211:  Extra space between return and WEBKIT_DOM_TEST_POD_OBJ  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.cpp:212:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:39:  parent_instance is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:43:  parent_class is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObj.h:47:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:194:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:206:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:212:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:222:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:223:  Extra space between WebKitDOMSerializedScriptValue* and res  [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:21:  #ifndef header guard has wrong style, please use: WebKitDOMTestPODObjPrivate_h  [build/header_guard] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestPODObjPrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 36 in 40 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 62810 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=62810&action=review

Looks reasonable.  Some cleanup mentioned below.  The only serious problem is what looks like a memory leak.

> WebCore/bindings/js/SerializedScriptValue.cpp:977
> +    RefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));

Isn't this a memory leak?  We need to call adoptRef, right?

> WebCore/bindings/js/SerializedScriptValue.cpp:991
> +    RefPtr<SerializedScriptValue> serializedValue = new SerializedScriptValue(SerializedScriptValueData::serialize(exec, value));

Same here.  adoptRef?

> WebCore/bindings/scripts/CodeGeneratorJS.pm:2491
> +    $ret = $ret . "    PassRefPtr<SerializedScriptValue> serializedScriptValue = $value;\n";

We don't usually store PassRefPtrs on the stack.  We usually us RefPtr and then release.

> WebCore/bindings/scripts/test/V8/V8TestObj.cpp:610
> +    if (UNLIKELY(ec))
> +        goto fail;

That's pretty nasty code.  Also, the indent is wrong here.

> WebCore/bindings/scripts/test/V8/V8TestObj.cpp:637
> +    {
> +    RefPtr<SerializedScriptValue> result = imp->serializedValueReturnRaises(ec);
> +    if (UNLIKELY(ec))
> +        goto fail;
> +    return result.release()->deserialize();
> +    }

Another bad indent.  Why do we need these { }  brackets?

> WebCore/bindings/v8/SerializedScriptValue.cpp:1031
> +        didThrow = true;

Why are we using didThrow instead of the ec, like we usually do?