Bug 67666 – Binding CodeGenerators don't support Conditional= on constants

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Aaron Colwell]

It appears that at least CodeGeneratorV8.pm does not honor the Conditional= directive on constants and functions specified in the IDL. I encountered this while trying to convert my MediaSource API methods from using an "#if defined(ENABLE_MEDIA_SOURCE)..." block to [Conditional=MEDIA_SOURCE] on each of the items in the block. I've attached a patch for the changes. 

When I apply this patch I get a bunch of compiler errors (see attached) that appear to indicate that the code generator completely ignored the Condition annotation on the functions and constants.

I'd expect the code generator to not emit code for functions and constants if the Conditional value is 0 or not set like it does for attributes.

[Aaron Colwell]

Created attachment 106472 [details]
A patch that generates the errors

[Aaron Colwell]

Created attachment 106473 [details]
Errors generated by the patch.

[Erik Arvidsson]

I can duplicate the issue with conditional consts but we have code that uses conditional functions so at least that should work:

http://www.google.com/codesearch#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/xml/XMLHttpRequest.idl&exact_package=chromium&q=(%5Cs)*%5C%5BConditional=%20file:%5C.idl&type=cs&l=77

[Erik Arvidsson]

Created attachment 116296 [details]
Patch

[Adam Barth]

Comment on attachment 116296 [details]
Patch

Nice patch.  We should share more code like this.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:172:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:1921:  jsTestObjCONDITIONAL_CONST is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/bindings/scripts/test/JS/JSTestObj.h:266:  jsTestObjCONDITIONAL_CONST is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:1472:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 4 in 12 files


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

[WebKit Review Bot]

Comment on attachment 116296 [details]
Patch

Clearing flags on attachment: 116296

Committed r101102: <http://trac.webkit.org/changeset/101102>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Patrick R. Gansterer]

*** Bug 67450 has been marked as a duplicate of this bug. ***