Add support in generate scripts to handle Conditional

Add support in generate scripts to handle Conditional

WEB_AUDIO and SVG_FONTS conditions in tagName files were 
handled by generate scripts and this CL adds the support. 
This fix was also needed to build without SVG_FONTS enabled.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=176249

[maheshkk, 2014-06-12 15:50:35]

PTAL

[pdr., 2014-06-12 17:34:45]

+cc nbarth

[Nils Barth (inactive), 2014-06-13 02:14:24]

LGTM with nits.

BTW, since looking at conditionals in generated files,
you're probably interested in a bindings cleanup CL that this
here CL sparked:
Turn conditionals back on in aggregate bindings
https://codereview.chromium.org/333803003/

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
File Source/build/scripts/make_element_factory.py (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
Source/build/scripts/make_element_factory.py:50: 'Conditional': None,
nit: Capitalization: this should be lowercase, right?

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
File Source/build/scripts/make_element_type_helpers.py (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
Source/build/scripts/make_element_type_helpers.py:36: 'Conditional': None,
Ditto: lowercase, no?

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/templat...
File Source/build/scripts/templates/ElementFactory.cpp.tmpl (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/templat...
Source/build/scripts/templates/ElementFactory.cpp.tmpl:73: static const
Create{{namespace}}FunctionMapData data[] = {
One subtlety: this definition *fails to compile* on MSVC
(and is illegal per C++ standard) if there are no tags;
this is more likely now with conditionals.

It's probably worth adding a comment here that it will *fail* if empty.
(I assume this doesn't actually happen here and this template
isn't v. widely used.)

This issue comes up in the bindings generation code, since we have
automatically generated array declarations that may be empty and thus
must be omitted.

==Details==
Test needed as size 0 arrays definitions are not allowed per standard
(so objects have distinct addresses), which is enforced by MSVC.
8.5.1 Aggregates [dcl.init.aggr]
An array of unknown size initialized with a brace-enclosed
initializer-list containing n initializer-clauses, where n shall be
greater than zero, is defined as having n elements (8.3.4).

[maheshkk, 2014-06-13 20:03:16]

Thanks for your review!

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
File Source/build/scripts/make_element_factory.py (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
Source/build/scripts/make_element_factory.py:50: 'Conditional': None,
On 2014/06/13 02:14:24, Nils Barth wrote:
> nit: Capitalization: this should be lowercase, right?

I think it is capital C. Like below, these attributes are all "Conditional", and
as you know "defaults" needs to match attributes names. let me know if  you
think we should change all of them to "conditional" instead.

modules/EventTargetModulesFactory.in
25:modules/webaudio/AudioContext Conditional=WEB_AUDIO
26:modules/webaudio/AudioNode Conditional=WEB_AUDIO

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
File Source/build/scripts/make_element_type_helpers.py (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
Source/build/scripts/make_element_type_helpers.py:36: 'Conditional': None,
On 2014/06/13 02:14:24, Nils Barth wrote:
> Ditto: lowercase, no?

Same here.

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/templat...
File Source/build/scripts/templates/ElementFactory.cpp.tmpl (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/templat...
Source/build/scripts/templates/ElementFactory.cpp.tmpl:73: static const
Create{{namespace}}FunctionMapData data[] = {
On 2014/06/13 02:14:24, Nils Barth wrote:
> One subtlety: this definition *fails to compile* on MSVC
> (and is illegal per C++ standard) if there are no tags;
> this is more likely now with conditionals.
> 
> It's probably worth adding a comment here that it will *fail* if empty.
> (I assume this doesn't actually happen here and this template
> isn't v. widely used.)
> 
> This issue comes up in the bindings generation code, since we have
> automatically generated array declarations that may be empty and thus
> must be omitted.
> 
> ==Details==
> Test needed as size 0 arrays definitions are not allowed per standard
> (so objects have distinct addresses), which is enforced by MSVC.
> 8.5.1 Aggregates [dcl.init.aggr]
> An array of unknown size initialized with a brace-enclosed
> initializer-list containing n initializer-clauses, where n shall be
> greater than zero, is defined as having n elements (8.3.4).

That's a good point. I have added comment for now and FIXME to handle the
condition when there are no tags present.

[maheshkk, 2014-06-13 20:09:31]

Nils, I also found that WEB_AUDIO condition in
modules/EventTargetModulesFactory.in where not generating resulting files
correctly. make_name.py script did not handle Conditional and this patch adds
that as well. 

gen/blink/modules/EventTargetModulesNames.h
gen/blink/modules/EventTargetModulesNames.cpp


PTAL.

[Nils Barth (inactive), 2014-06-16 06:27:52]

LGTM with nits: alphabetical order and comment.

Thanks for additional catch!

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
File Source/build/scripts/make_element_factory.py (right):

https://codereview.chromium.org/330093002/diff/1/Source/build/scripts/make_el...
Source/build/scripts/make_element_factory.py:50: 'Conditional': None,
On 2014/06/13 20:03:16, maheshkk wrote:
> On 2014/06/13 02:14:24, Nils Barth wrote:
> > nit: Capitalization: this should be lowercase, right?
> 
> I think it is capital C. Like below, these attributes are all "Conditional",
and
> as you know "defaults" needs to match attributes names. let me know if  you
> think we should change all of them to "conditional" instead.

Thanks for explaining.
Looking at the .in files, upper/lower is inconsistent,
and having upper to match with extended attributes is fine,
thanks!

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
File Source/build/scripts/make_element_factory.py (right):

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
Source/build/scripts/make_element_factory.py:50: 'Conditional': None,
However, could you put this *first*, for alphabetical order?
(Uppercase before lowercase.)

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
File Source/build/scripts/make_element_type_helpers.py (right):

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
Source/build/scripts/make_element_type_helpers.py:36: 'Conditional': None,
alpha
(could you alphabetize the earlier interfaceName and no* too?)

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
Source/build/scripts/make_element_type_helpers.py:49: 'enable_conditional':
name_utilities.enable_conditional_if_endif,
alpha

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
File Source/build/scripts/make_names.py (right):

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
Source/build/scripts/make_names.py:64: 'enable_conditional':
name_utilities.enable_conditional_if_endif,
alpha

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
File Source/build/scripts/make_qualified_names.py (right):

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/mak...
Source/build/scripts/make_qualified_names.py:60: 'enable_conditional':
name_utilities.enable_conditional_if_endif,
alpha (fix existing)

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/tem...
File Source/build/scripts/templates/ElementFactory.cpp.tmpl (right):

https://codereview.chromium.org/330093002/diff/40001/Source/build/scripts/tem...
Source/build/scripts/templates/ElementFactory.cpp.tmpl:73: // FIXME: Compilation
on MSVC will fail if data[] is empty. Zero size arrays
I wouldn't make this a FIXME;
it's fine to just have a note, since if this is ever triggered,
compilation will fail exactly here.

Also, this is not MSVC-specific: it's C++ standard.

Thus I'd write this as:
// Empty array initializer lists are illegal [dcl.init.aggr] and will not
// compile in MSVC. If tags list is empty, add check to skip this.

[Nils Barth (inactive), 2014-06-16 06:28:43]

pdr, this CL LGTM; could you check and ok it (for core) if looks ok?

[pdr., 2014-06-16 20:18:54]

On 2014/06/16 06:28:43, Nils Barth wrote:
> pdr, this CL LGTM; could you check and ok it (for core) if looks ok?

This looks good to me. It's a shame so much work is being used for SVG fonts
which we are planning on removing very soon. Because this is useful for more
than just svg fonts, LGTM.

[maheshkk, 2014-06-16 20:22:57]

The CQ bit was checked by mahesh.kk@samsung.com

[commit-bot: I haz the power, 2014-06-16 20:25:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mahesh.kk@samsung.com/330093002/60001

[commit-bot: I haz the power, 2014-06-16 22:34:42]

Change committed as 176249