Generate separate files for union type containers

Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/89853fd3c7589e801151a70687beb391195cee9a
Cr-Commit-Position: refs/heads/master@{#393130}

[bashi, 2016-05-09 10:25:25]

Description was changed from

==========
WIP: Generate separate files for union type containers

BUG=
==========

to

==========
WIP: Generate separate files for union type containers

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[bashi, 2016-05-09 10:35:18]

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

[bashi, 2016-05-09 10:35:20]

Not yet ready for review; I just want to show what I'm going to do.

This is a huge CL but many changes are generated-results and mechanical changes.

- Remove aggregated files (UnionTypes{Core,Modules}.h and
UnionTypes{Core,Modules}.cpp).
- core/modules should include separate union type container files.
- Explicitly list generated union type container files.

WDYT?

https://codereview.chromium.org/1961883002/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/core/v8/generated.gni (right):

https://codereview.chromium.org/1961883002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/core/v8/generated.gni:37:
bindings_core_generated_union_type_files = [
This is the problem of this approach. We should figure out a way to update this
list automatically.

[haraken, 2016-05-09 10:47:48]

On 2016/05/09 10:35:20, bashi1 wrote:
> Not yet ready for review; I just want to show what I'm going to do.
> 
> This is a huge CL but many changes are generated-results and mechanical
changes.
> 
> - Remove aggregated files (UnionTypes{Core,Modules}.h and
> UnionTypes{Core,Modules}.cpp).
> - core/modules should include separate union type container files.
> - Explicitly list generated union type container files.
> 
> WDYT?

The approach looks good to me.

> 
>
https://codereview.chromium.org/1961883002/diff/1/third_party/WebKit/Source/b...
> File third_party/WebKit/Source/bindings/core/v8/generated.gni (right):
> 
>
https://codereview.chromium.org/1961883002/diff/1/third_party/WebKit/Source/b...
> third_party/WebKit/Source/bindings/core/v8/generated.gni:37:
> bindings_core_generated_union_type_files = [
> This is the problem of this approach. We should figure out a way to update
this
> list automatically.

Won't we hit a compile error if we forget to add a new file to .gni? Then I'm
fine with the approach.

[bashi, 2016-05-10 08:10:52]

Description was changed from

==========
WIP: Generate separate files for union type containers

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424

CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[bashi, 2016-05-10 08:16:10]

Now ready for review, PTAL? Sorry for asking a huge review but I couldn't
separate this into small CLs...

Please review IDLUnionTypes.md, *.py and template files (other changes are
machine-generated or trivial). There are some quick hacks in script changes but
I'd like to land this first then do some refactoring.

[haraken, 2016-05-10 09:19:59]

LGTM

yukishiino@: Would you take another look at the IDL compiler part?

https://codereview.chromium.org/1961883002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/IDLUnionTypes.md (right):

https://codereview.chromium.org/1961883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/IDLUnionTypes.md:7: For each union type, the
code generator creates a C++ class which is used like an "impl" class of a
normal interface type. The name of a generated classe is a [type
name](https://heycam.github.io/webidl/#dfn-type-name) of the union type. For
example, the code generator will create `StringOrFloat` class for `(DOMString or
float)`.

generated class

[Yuki, 2016-05-10 12:29:46]

I'm not an expert in this area, and it's not easy for me to follow each line of
changes...  I trust bashi@ and LGTM.

Having said that, what rule do you apply to the header name (AorBorC.h)?  It
seems not the alphabetical order.  How can developers predict the header names?

[bashi, 2016-05-11 23:09:29]

> Having said that, what rule do you apply to the header name (AorBorC.h)?  It
> seems not the alphabetical order.  How can developers predict the header
names?

We use "type name" for the header files. The type name of a union type is
formed by taking the type names of each member type, in order, and joining them
with the string "Or"[1].

[1] https://heycam.github.io/webidl/#idl-union

Developers should be able to find the type name for their union types.

https://codereview.chromium.org/1961883002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/IDLUnionTypes.md (right):

https://codereview.chromium.org/1961883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/IDLUnionTypes.md:7: For each union type, the
code generator creates a C++ class which is used like an "impl" class of a
normal interface type. The name of a generated classe is a [type
name](https://heycam.github.io/webidl/#dfn-type-name) of the union type. For
example, the code generator will create `StringOrFloat` class for `(DOMString or
float)`.
On 2016/05/10 09:19:59, haraken wrote:
> 
> generated class

Done.

[bashi, 2016-05-11 23:10:31]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-05-11 23:10:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, yukishiino@chromium.org
Link to the patchset: https://codereview.chromium.org/1961883002/#ps60001
(title: " ")

[commit-bot: I haz the power, 2016-05-11 23:11:25]

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

[commit-bot: I haz the power, 2016-05-12 01:16:20]

Description was changed from

==========
Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424

CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424

CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

[commit-bot: I haz the power, 2016-05-12 01:16:22]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-12 01:17:43]

Description was changed from

==========
Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424

CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
==========

to

==========
Generate separate files for union type containers

Generating union type containers into single files (per component)
is the root cause of cyclic dependencies when we use union types
with IDL dictionaries. We should put generated containers into
separate files.

Unfortunately this CL brings a burden on developers.
See IDLUnionTypes.md for more details.

BUG=524424

CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel

Committed: https://crrev.com/89853fd3c7589e801151a70687beb391195cee9a
Cr-Commit-Position: refs/heads/master@{#393130}
==========

[commit-bot: I haz the power, 2016-05-12 01:17:46]

Patchset 4 (id:??) landed as
https://crrev.com/89853fd3c7589e801151a70687beb391195cee9a
Cr-Commit-Position: refs/heads/master@{#393130}

[haraken, 2016-05-12 02:16:03]

Is there any remaining work to support the nested unions & dictionaries in
WebRTC?

[bashi, 2016-05-12 02:22:05]

On 2016/05/12 02:16:03, haraken wrote:
> Is there any remaining work to support the nested unions & dictionaries in
> WebRTC?

IIUC there is no remaining work. Just want to wait for several hours to make
sure there is no build failures on bots.

[Sébastien Marchand, 2016-05-12 14:07:25]

sebmarchand@chromium.org changed reviewers:
+ sebmarchand@chromium.org

[Sébastien Marchand, 2016-05-12 14:07:26]

This CL broke the Win PGO builder
(https://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20PGO%20Bui...):

No such file or directory:
'..\\..\\..\\..\\..\\..\\out\\Release/gen/blink/bindings/modules/v8/CanvasRenderingContext2DOrWebGLRenderingContextOrWebGL2RenderingContextOrImageBitmapRenderingContext.cpp'

any idea of what's going on ?

[bashi, 2016-05-12 23:16:19]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1974143002/ by bashi@chromium.org.

The reason for reverting is: Broke win build.

[ncarter (slow), 2016-05-13 18:59:30]

nick@chromium.org changed reviewers:
+ nick@chromium.org

[ncarter (slow), 2016-05-13 18:59:32]

https://codereview.chromium.org/1961883002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/bindings/scripts/utilities.py (right):

https://codereview.chromium.org/1961883002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/bindings/scripts/utilities.py:292: def
write_file(new_text, destination_filename, only_if_changed):
Just doing the following seems to fix the problem:

    destination_filename = os.path.normpath(os.path.join(os.getcwd(),
                                            destination_filename))

(note that os.path.abspath won't work -- it seems to fail when the
concatentation of cwd + arg is over the MAX_PATH limit.)