Summary: | Add JSON parameter support to JS/V8 binding generator scripts | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kinuko Yasuda <kinuko> | ||||||||
Component: | WebCore Misc. | Assignee: | Kinuko Yasuda <kinuko> | ||||||||
Status: | RESOLVED INVALID | ||||||||||
Severity: | Normal | CC: | abarth, annevk, arv, dglazkov, dmikurube, dumi, ericu, japhet, levin, oliver, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Kinuko Yasuda
2010-09-04 16:25:13 PDT
Created attachment 66593 [details]
Patch
Proposal patch
Comment on attachment 66593 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=66593&action=prettypatch > WebCore/bindings/scripts/CodeGeneratorJS.pm:235 > + my $type = shift; > + return ("CREATE" => "boolean", "EXCLUSIVE" => "boolean") if $type eq "Flags"; This logic shouldn't be in the code generator. You'll need to provide this information as input to the code generator. > WebCore/bindings/scripts/CodeGeneratorJS.pm:240 > + # For testing. > + return ("intAttr" => "long", > + "stringAttr" => "DOMString", > + "testObjAttr" => "TestObj") if $type eq "TestJSONObj"; Why do we have special code to support testing? Doesn't that defeat the purpose of testing? Created attachment 66799 [details]
Patch
How about this? Changed the code to use codeGenerator->ParseInterface instead of having a copy of interface definitions in the generator code.
Created attachment 67223 [details]
Patch
rebased.
Attachment 67223 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
Last 3072 characters of output:
Use 0 instead of NULL. [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:199: webkit_dom_test_json_obj_set_property is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:199: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:206: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:227: webkit_dom_test_json_obj_get_property is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:227: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:234: Non-label code inside switch statements should be indented. [whitespace/indent] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:266: webkit_dom_test_json_obj_constructed is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:273: webkit_dom_test_json_obj_class_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:275: Declaration has space between type name and * in GObjectClass *gobjectClass [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:326: webkit_dom_test_json_obj_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:341: Extra space between return and WEBKIT_DOM_TEST_JSON_OBJ [whitespace/declaration] [3]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:342: Use 0 instead of NULL. [readability/null] [5]
WebCore/bindings/scripts/test/V8/V8TestJSONObj.h:26: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4]
WebCore/bindings/scripts/test/CPP/WebDOMTestJSONObj.cpp:30: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:27: Alphabetical sorting problem. [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:39: parent_instance is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:43: parent_class is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:47: Extra space before ( in function call [whitespace/parens] [4]
Total errors found: 37 in 21 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #5) > Attachment 67223 [details] did not pass style-queue: > > Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 > Last 3072 characters of output: > Use 0 instead of NULL. [readability/null] [5] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:199: webkit_dom_test_json_obj_set_property is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:199: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:206: Non-label code inside switch statements should be indented. [whitespace/indent] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:227: webkit_dom_test_json_obj_get_property is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:227: prop_id is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:234: Non-label code inside switch statements should be indented. [whitespace/indent] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:266: webkit_dom_test_json_obj_constructed is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:273: webkit_dom_test_json_obj_class_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:275: Declaration has space between type name and * in GObjectClass *gobjectClass [whitespace/declaration] [3] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:326: webkit_dom_test_json_obj_init is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:341: Extra space between return and WEBKIT_DOM_TEST_JSON_OBJ [whitespace/declaration] [3] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.cpp:342: Use 0 instead of NULL. [readability/null] [5] > WebCore/bindings/scripts/test/V8/V8TestJSONObj.h:26: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4] > WebCore/bindings/scripts/test/CPP/WebDOMTestJSONObj.cpp:30: wtf includes should be <wtf/file.h> instead of "wtf/file.h". [build/include] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:27: Alphabetical sorting problem. [build/include_order] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:39: parent_instance is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:43: parent_class is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] > WebCore/bindings/scripts/test/GObject/WebKitDOMTestJSONObj.h:47: Extra space before ( in function call [whitespace/parens] [4] > Total errors found: 37 in 21 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. All of them are for generated code. (I don't know how to suppress them...) > All of them are for generated code. (I don't know how to suppress them...)
Yeah, we need to teach check-webkit-style about those files.
(Adding some more people who know binding code well) Hmm this patch worked for me but calling $codeGenerator->ParseInterface() in the code generation really slows down the process. Is the spec really using json rather than the internal structured clone algorithm? Comment on attachment 67223 [details]
Patch
Removing from the review queue
(In reply to comment #9) > Is the spec really using json rather than the internal structured clone algorithm? (In reply to comment #9) > Is the spec really using json rather than the internal structured clone algorithm? The spec reads using json parameter rather than the internal structured clone algorithm. (As far as I believe, @ericu would be able to tell the truth) I have the feeling that probably adding a CustomConversion attribute to parameter attributes would make more sense. Then the developer would need to write a small custom code that only handles the parameter rather than writing a custom code for the entire method. (This was suggested by antonm@chromium.org) I'm going to poke around the idea and see if such a generalization makes more sense in other cases. (In reply to comment #11) > (In reply to comment #9) > > Is the spec really using json rather than the internal structured clone algorithm? > > The spec reads using json parameter rather than the internal structured clone algorithm. > (As far as I believe, @ericu would be able to tell the truth) btw, I don't understand the comment because I don't a text representation for structured clone only an algorithm that explains how to do it. (If there was a text representation, then that may make more sense.) V8 integration is no longer a thing. |