bindings: Add support for nested union types

bindings: Add support for nested union types

From a code perspective, this is more like "add support for retrieving a
union's flattened member types set", which is exposed via the
IdlUnionType.flattened_member_types property.

The algorithm is pretty simple and described in the spec:
https://heycam.github.io/webidl/#idl-union

v8_union can then iterate through its flattened member types when emitting
the C++ code for converting unions to and from JavaScript objects.

BUG=700225
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2746713003
Cr-Commit-Position: refs/heads/master@{#456363}
Committed: https://chromium.googlesource.com/chromium/src/+/a15fea4b2a0062f98fa2c7b446dfa4035e7231c9

[Raphael Kubo da Costa (rakuco), 2017-03-12 14:56:23]

PTAL.

[Raphael Kubo da Costa (rakuco), 2017-03-12 14:56:43]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-12 14:56:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-12 16:32:58]

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

[commit-bot: I haz the power, 2017-03-12 16:32:58]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-03-12 22:14:17]

LGTM but I want to have bashi@ take a look.

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/idl_types.py (right):

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/idl_types.py:307: #
IdlType('ByteString') and IdlType('ByteString') are two different objects.

Nit: It took me a long time to understand what "IdlType('ByteString') and
IdlType('ByteString') are two different objects" means. Can we rephrase it a
bit?

[bashi, 2017-03-13 00:17:14]

Thanks for fixing the issue. I was thinking the same solution and this seems a
right way to address the problem for now. However, I'm not sure we want to keep
generating union container in this way, as the generated classes looks weird.
Probably we want to revisit crbug.com/611545.

Anyway this CL itself generally looks good.

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
File third_party/WebKit/Source/bindings/scripts/idl_types.py (right):

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/idl_types.py:301: def
flattened_member_types(self):
Other algorithms related to union types are defined in v8_types.py (e.g.
includes_nullable_type_union). It would be better to put them together.

It looks this is the right place to put flattend_member_types() though. We may
want to move those algorithms into this class (You don't have to do in this CL).

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/idl_types.py:304:
https://heycam.github.io/webidl/#idl-union
https://heycam.github.io/webidl/#dfn-flattened-union-member-types ?

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
File
third_party/WebKit/Source/bindings/tests/results/core/NodeOrLongSequenceOrEventOrXMLHttpRequestOrStringOrStringByteStringOrNodeListRecord.h
(right):

https://codereview.chromium.org/2746713003/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/tests/results/core/NodeOrLongSequenceOrEventOrXMLHttpRequestOrStringOrStringByteStringOrNodeListRecord.h:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
IIUC this kind of too long file name is problematic on Windows
(crbug.com/611437). We have utilities.shorten_union_name() to work around the
problem.

I think we should check the length of a given union type and throw an exception
if it's too long (100 characters?) but doesn't have an alias.

[Yuki, 2017-03-13 08:40:34]

LGTM in general, and defer a detailed review to bashi@.

[Raphael Kubo da Costa (rakuco), 2017-03-13 11:38:24]

Patch v2 is up. I've fixed the link to the member flattening algorithm in the
spec, and rephrased the IdlType('ByteString') bit.

As for the rest:
- I noticed v8_types had some union-specific code, I'll see if I can move it to
idl_types in another CL.
- I don't have access to crbug.com/611437 :( I remember having problems with
paths that were too long on Windows in the past myself, so I'm guessing the bug
is about that. It's not clear to me if shorten_union_name() itself should
validate the union name or if it should be done elsewhere. I'd be happy to do
that in another CL too.

[Raphael Kubo da Costa (rakuco), 2017-03-13 11:38:29]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[Raphael Kubo da Costa (rakuco), 2017-03-13 11:38:30]

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/2746713003/#ps20001
(title: "Improve documentation")

[commit-bot: I haz the power, 2017-03-13 11:38:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yuki, 2017-03-13 12:23:31]

On 2017/03/13 11:38:24, Raphael Kubo da Costa (rakuco) wrote:
> Patch v2 is up. I've fixed the link to the member flattening algorithm in the
> spec, and rephrased the IdlType('ByteString') bit.
> 
> As for the rest:
> - I noticed v8_types had some union-specific code, I'll see if I can move it
to
> idl_types in another CL.
> - I don't have access to crbug.com/611437 :( I remember having problems with
> paths that were too long on Windows in the past myself, so I'm guessing the
bug
> is about that. It's not clear to me if shorten_union_name() itself should
> validate the union name or if it should be done elsewhere. I'd be happy to do
> that in another CL too.

Your guess is correct, it's too long path on Windows.
Thank you for working on so many issues here and there.  :)

[commit-bot: I haz the power, 2017-03-13 13:31:17]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489405109966990,
"parent_rev": "f8d3488e588458268f165f6e21e2e86bff0b9b6d", "commit_rev":
"a15fea4b2a0062f98fa2c7b446dfa4035e7231c9"}

[commit-bot: I haz the power, 2017-03-13 13:31:58]

Description was changed from

==========
bindings: Add support for nested union types

From a code perspective, this is more like "add support for retrieving a
union's flattened member types set", which is exposed via the
IdlUnionType.flattened_member_types property.

The algorithm is pretty simple and described in the spec:
https://heycam.github.io/webidl/#idl-union

v8_union can then iterate through its flattened member types when emitting
the C++ code for converting unions to and from JavaScript objects.

BUG=700225
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

to

==========
bindings: Add support for nested union types

From a code perspective, this is more like "add support for retrieving a
union's flattened member types set", which is exposed via the
IdlUnionType.flattened_member_types property.

The algorithm is pretty simple and described in the spec:
https://heycam.github.io/webidl/#idl-union

v8_union can then iterate through its flattened member types when emitting
the C++ code for converting unions to and from JavaScript objects.

BUG=700225
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2746713003
Cr-Commit-Position: refs/heads/master@{#456363}
Committed:
https://chromium.googlesource.com/chromium/src/+/a15fea4b2a0062f98fa2c7b446df...
==========

[commit-bot: I haz the power, 2017-03-13 13:31:59]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a15fea4b2a0062f98fa2c7b446df...

[bashi, 2017-03-13 23:19:05]

On 2017/03/13 11:38:24, Raphael Kubo da Costa (rakuco) wrote:
> Patch v2 is up. I've fixed the link to the member flattening algorithm in the
> spec, and rephrased the IdlType('ByteString') bit.
> 
> As for the rest:
> - I noticed v8_types had some union-specific code, I'll see if I can move it
to
> idl_types in another CL.
> - I don't have access to crbug.com/611437 :( I remember having problems with
> paths that were too long on Windows in the past myself, so I'm guessing the
bug
> is about that. It's not clear to me if shorten_union_name() itself should
> validate the union name or if it should be done elsewhere. I'd be happy to do
> that in another CL too.

I think we need to make sure that we have aliases for long union names as soon
as possible. Otherwise developers may submit CLs which break win builds by
accident.