Update bindings gn files for binding modularization.

Update bindings gn files for binding modularization.

bindings gyp files support but gn files are old and doesn't support binding modularization. e.g.
- create WindowCoreConstructors.idl and WindowModulesConstructors.idl instead of WindowConstructors.idl,
- ditto (SharedWorkderGlobalScope, DedicatedWorkerGlobalScope, ServiceWorkerGlobalScope).

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

[tasak, 2014-10-07 11:13:17]

tasak@google.com changed reviewers:
+ brettw@chromium.org, haraken@chromium.org

[tasak, 2014-10-07 11:13:18]

Would you review this CL?

I translated current bindings gyp into gn.

[brettw, 2014-10-07 16:44:08]

https://codereview.chromium.org/633913002/diff/1/Source/bindings/modules/BUIL...
File Source/bindings/modules/BUILD.gn (right):

https://codereview.chromium.org/633913002/diff/1/Source/bindings/modules/BUIL...
Source/bindings/modules/BUILD.gn:115: action("modules_global_objects") {
These script invocations are duplicated in the two files. Usually if a script is
called more than once, it's best to write a template that knows how to invoke
that script and to provide a nice interface for it.

See Source/bindings/scripts/scripts.gni for some examples (this is probably
where you'd put the template for these two scripts as well). You can see this
separates the complexity of rebasing paths and all the args and so the callsite
can be just a couple of lines that is much easier to follow.

Let me know if you need more pointers.

[tasak, 2014-10-08 03:29:50]

Thank you for reviewing.

https://codereview.chromium.org/633913002/diff/1/Source/bindings/modules/BUIL...
File Source/bindings/modules/BUILD.gn (right):

https://codereview.chromium.org/633913002/diff/1/Source/bindings/modules/BUIL...
Source/bindings/modules/BUILD.gn:115: action("modules_global_objects") {
On 2014/10/07 16:44:08, brettw wrote:
> These script invocations are duplicated in the two files. Usually if a script
is
> called more than once, it's best to write a template that knows how to invoke
> that script and to provide a nice interface for it.
> 
> See Source/bindings/scripts/scripts.gni for some examples (this is probably
> where you'd put the template for these two scripts as well). You can see this
> separates the complexity of rebasing paths and all the args and so the
callsite
> can be just a couple of lines that is much easier to follow.
> 
> Let me know if you need more pointers.

I see.
I added 2 new templates: compute_global_objects and
generate_global_constructors.

[brettw, 2014-10-08 18:28:33]

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/modules/...
File Source/bindings/modules/BUILD.gn (right):

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/modules/...
Source/bindings/modules/BUILD.gn:121: sources_generated = [
"$bindings_core_output_dir/GlobalObjectsCore.pickle" ]
This takes an input from another target. We should have that target in the deps
of this one (in practice, Ninja should figure this out since this will be an
input here, and an output from another step, but I think it's always good to
make the dep explicit here).

This probably means adding an optional "deps" parameter to your template, and
listing the core_global_objects as a dep here.

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/scripts.gni (right):

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/scripts.gni:280: #   output_idl_file = a list of IDL
files to write to.
you need an "s" at the end of the var name. I think this should also explicitly
call out that these must correspond to the "interfaces" values (same length,
same order).

Actually, this is pretty fragile because the value you pass from the
output_idl_files is defines in some .gni file, and the interfaces list is
defined in the invocation to this template. Requiring these to stay in sync is
hard.

Can we autogenerate the .idl names in this template? All the ones I've seen have
the form "$blink_output_dir/${interface}Constructors.idl". Doing this would
solve the fragility and also clean up the call sites.

[tasak, 2014-10-14 11:51:34]

Thank you for reviewing.

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/modules/...
File Source/bindings/modules/BUILD.gn (right):

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/modules/...
Source/bindings/modules/BUILD.gn:121: sources_generated = [
"$bindings_core_output_dir/GlobalObjectsCore.pickle" ]
On 2014/10/08 18:28:33, brettw wrote:
> This takes an input from another target. We should have that target in the
deps
> of this one (in practice, Ninja should figure this out since this will be an
> input here, and an output from another step, but I think it's always good to
> make the dep explicit here).
> 
> This probably means adding an optional "deps" parameter to your template, and
> listing the core_global_objects as a dep here.

Done.

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/scripts/...
File Source/bindings/scripts/scripts.gni (right):

https://codereview.chromium.org/633913002/diff/20001/Source/bindings/scripts/...
Source/bindings/scripts/scripts.gni:280: #   output_idl_file = a list of IDL
files to write to.
On 2014/10/08 18:28:33, brettw wrote:
> you need an "s" at the end of the var name. I think this should also
explicitly
> call out that these must correspond to the "interfaces" values (same length,
> same order).
> 
> Actually, this is pretty fragile because the value you pass from the
> output_idl_files is defines in some .gni file, and the interfaces list is
> defined in the invocation to this template. Requiring these to stay in sync is
> hard.
> 
> Can we autogenerate the .idl names in this template? All the ones I've seen
have
> the form "$blink_output_dir/${interface}Constructors.idl". Doing this would
> solve the fragility and also clean up the call sites.

Done.
Autogenerated the .idl names and headers.

[brettw, 2014-10-14 17:04:30]

LGTM with a few minor fixes. Thanks!

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/core/cor...
File Source/bindings/core/core.gni (right):

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/core/cor...
Source/bindings/core/core.gni:21: core_global_constructors_generated_idl_files
+= [ core_idl_file ]
Can you do:
core_global_constructors_generated_idl_files += [
  "$blink_core_output_dir/${interface}CoreConstructors.idl"
]
instead? Your temporary will actually outlive the scope of this loop which will
pollute the global namespace.

Also: indent only 2 spaces.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
File Source/bindings/modules/modules.gni (right):

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:12: "ServiceWorkerGlobalScope",
All of these should be indented only 2 spaces.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:24: modules_idl_file =
"$blink_modules_output_dir/${interface}CoreConstructors.idl"
Avoid temporary as above.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:30: modules_idl_file =
"$blink_modules_output_dir/${interface}ModulesConstructors.idl"
Ditto

[tasak, 2014-10-15 06:23:19]

Thank you for reviewing.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/core/cor...
File Source/bindings/core/core.gni (right):

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/core/cor...
Source/bindings/core/core.gni:21: core_global_constructors_generated_idl_files
+= [ core_idl_file ]
On 2014/10/14 17:04:30, brettw wrote:
> Can you do:
> core_global_constructors_generated_idl_files += [
>   "$blink_core_output_dir/${interface}CoreConstructors.idl"
> ]
> instead? Your temporary will actually outlive the scope of this loop which
will
> pollute the global namespace.
> 
> Also: indent only 2 spaces.

Done.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
File Source/bindings/modules/modules.gni (right):

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:12: "ServiceWorkerGlobalScope",
On 2014/10/14 17:04:30, brettw wrote:
> All of these should be indented only 2 spaces.

Done.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:24: modules_idl_file =
"$blink_modules_output_dir/${interface}CoreConstructors.idl"
On 2014/10/14 17:04:30, brettw wrote:
> Avoid temporary as above.

Done.

https://codereview.chromium.org/633913002/diff/40001/Source/bindings/modules/...
Source/bindings/modules/modules.gni:30: modules_idl_file =
"$blink_modules_output_dir/${interface}ModulesConstructors.idl"
On 2014/10/14 17:04:30, brettw wrote:
> Ditto

Done.

[tasak, 2014-10-15 06:23:26]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-10-15 06:24:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/633913002/60001

[commit-bot: I haz the power, 2014-10-15 06:31:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-15 06:31:12]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)

[haraken, 2014-10-16 02:15:33]

rubberstamp LGTM

[tasak, 2014-10-16 02:16:01]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-10-16 02:16:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/633913002/60001

[commit-bot: I haz the power, 2014-10-16 04:02:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-16 04:02:25]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/31974)

[tasak, 2014-10-16 05:55:28]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-10-16 05:55:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/633913002/60001

[commit-bot: I haz the power, 2014-10-16 06:32:11]

Committed patchset #4 (id:60001) as 183786