Bug 25612 – Add support for feature conditionals in the generated JSC bindings code

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Simon Hausmann]

Currently features in IDL files are configured through preprocessor macros. The patches attached to this bug report add support for moving these #ifdefs into the generated code, making the generated code independent from the features are code generation time.

[Simon Hausmann]

Created attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

This patch adds support for the conditional attribute for IDL attributes, which places feature #ifdefs into the generated code, replacing #ifdefs in the IDL files.

[Simon Hausmann]

Created attachment 30094 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code

This patch allows moving #ifdefs in the tagnames.in files into the generated tag factory code.

[Simon Hausmann]

Comment on attachment 30094 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code

Clearing review, this patch doesn't work entirely anymore. Will update soon.

[Maciej Stachowiak]

It might be good for Sam to look at this. Sounds like a good idea.

[Eric Seidel (no email)]

Won't we still end up re-generating the code every time you change FEATURE_DEFINES?  We'll certainly do a full recompile (at least in xcode/gcc land), so this savings seems minimal.  It's not necessarily a bad idea though.

[Simon Hausmann]

(In reply to comment #5)
> Won't we still end up re-generating the code every time you change
> FEATURE_DEFINES?  We'll certainly do a full recompile (at least in xcode/gcc
> land), so this savings seems minimal.  It's not necessarily a bad idea though.

Yes, that is currently the case. However my long term goal is to have generated files that can be used with different configurations without the need to re-generate them. The generation can take a long time on older platforms :).

We're _almost_ there with the existing #ifdef's that exist for example for the SVG files. AFAICS it's just a few IDL files that need tweaking :)

[Maciej Stachowiak]

Comment on attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

Looks good.

[Simon Hausmann]

Comment on attachment 30093 [details]
Patch for moving feature #ifdefs into generated JSC bindings code

Thanks for the review, landed this one in r44116. Keeping the bug open though because of the pending tags patch.

[Simon Hausmann]

Created attachment 30631 [details]
Patch to add support for feature #ifdefs in the genertaed tag factory code (take 2)

[Laszlo Gombos]

(In reply to comment #8)
> (From update of attachment 30093 [details] [review])
> Thanks for the review, landed this one in r44116. Keeping the bug open though
> because of the pending tags patch.
> 

JSDOMWindow.cpp fails to build for me, when VIDEO is off after changeset 44116.

generated/release/JSDOMWindow.cpp:91:32: error: JSHTMLAudioElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:125:32: error: JSHTMLMediaElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:149:32: error: JSHTMLVideoElement.h: No such file or directory
generated/release/JSDOMWindow.cpp:151:26: error: JSMediaError.h: No such file or directory

#include's might need guards as well.

[Simon Hausmann]

(In reply to comment #10)
> (In reply to comment #8)
> > (From update of attachment 30093 [details] [review] [review])
> > Thanks for the review, landed this one in r44116. Keeping the bug open though
> > because of the pending tags patch.
> > 
> 
> JSDOMWindow.cpp fails to build for me, when VIDEO is off after changeset 44116.
> 
> generated/release/JSDOMWindow.cpp:91:32: error: JSHTMLAudioElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:125:32: error: JSHTMLMediaElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:149:32: error: JSHTMLVideoElement.h: No such
> file or directory
> generated/release/JSDOMWindow.cpp:151:26: error: JSMediaError.h: No such file
> or directory
> 
> #include's might need guards as well.
> 

Well spotted, fixed in r44126.

[Simon Hausmann]

Landed in r44128