bindings: Expand typedefs before generating union files.

bindings: Expand typedefs before generating union files.

Prior to this commit, the IDL compiler was failing to resolve typedefs in
unions before generating new union code. In practice, this meant an IDL
construct such as:

    typedef boolean Foo;
    void func((Foo or long) arg);

Generated FooOrLong.cpp and FooOrLong.h, and "Foo" was considered an IDL
interface instead of a simple bool. Since Foo.cpp and Foo.h do not exist,
and neither does the Foo class, this caused build failures.

We now resolve the typedefs earlier, and in the example above we would now
generate BooleanOrLong.cpp and BooleanOrLong.h, which works as expected.

Fixing this requires a few separate, intertwined changes:
* IdlUnionType.resolve_typedefs() was broken: the |typedefs| argument it
  receives is a dictionary whose keys are typedef names, not
  IdlTypeBase-derived objects, so |typedefs.get()| was always failing and
  returning the original, unresolved member objects.
* CodeGeneratorUnionType actually needs to call resolve_typedefs(). It was
  the only generator class not doing so. The mechanism needs to be different
  from the other classes though, as IdlUnionType is not a type defined in
  idl_definitions.py as is the case for the types handled by the other
  generators. Instead of using TypedefResolver, we simply call the relevant
  parts of it manually.
* The code manually adding union header includes to interfaces and partial
  interfaces has been dropped: there isn't enough information to resolve the
  typedefs at that point, which meant interface .cpp files would include
  FooOrLong.h, while interface .h files include BooleanOrLong.h. The test
  expectations IDL files show a reduction in unneeded includes, and the
  build seems to work fine.

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

Review-Url: https://codereview.chromium.org/2749253002
Cr-Commit-Position: refs/heads/master@{#457377}
Committed: https://chromium.googlesource.com/chromium/src/+/200763220d087ef3ccf735855398ec193db29658

[Raphael Kubo da Costa (rakuco), 2017-03-15 10:55:57]

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-15 10:56:05]

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

[Raphael Kubo da Costa (rakuco), 2017-03-15 10:59:11]

Description was changed from

==========
bindings: Expand typedefs before generating union files.

Prior to this commit, the IDL compiler was failing to resolve typedefs in
unions before generating new union code. In practice, this meant an IDL
construct such as:

    typedef boolean Foo;
    void func((Foo or long) arg);

Generated FooOrLong.cpp and FooOrLong.h, and "Foo" was considered an IDL
interface instead of a simple bool. Since Foo.cpp and Foo.h do not exist,
and neither does the Foo class, this caused build failures.

We now resolve the typedefs earlier, and in the example above we would now
generate BooleanOrLong.cpp and BooleanOrLong.h, which works as expected.

Fixing this requires a few separate, intertwined changes:
* IdlUnionType.resolve_typedefs() was broken: the |typedefs| argument it
  receives is a dictionary whose keys are typedef names, not
  IdlTypeBase-derived objects, so |typedefs.get()| was always failing and
  returning the original, unresolved member objects.
* CodeGeneratorUnionType actually needs to call resolve_typedefs(). It was
  the only generator class not doing so. The mechanism needs to be different
  from the other classes though, as IdlUnionType is not a type defined in
  idl_definitions.py as is the case for the types handled by the other
  generators. Instead of using TypedefResolver, we simply call the relevant
  parts of it manually.
* The code manually adding union header includes to interfaces and partial
  interfaces has been dropped: there isn't enough information to resolve the
  typedefs at that point, which meant interface .cpp files would include
  FooOrLong.h, while interface .h files include BooleanOrLong.h. The test
  expectations IDL files show a reduction in unneeded includes, and the
  build seems to work fine.

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

to

==========
bindings: Expand typedefs before generating union files.

Prior to this commit, the IDL compiler was failing to resolve typedefs in
unions before generating new union code. In practice, this meant an IDL
construct such as:

    typedef boolean Foo;
    void func((Foo or long) arg);

Generated FooOrLong.cpp and FooOrLong.h, and "Foo" was considered an IDL
interface instead of a simple bool. Since Foo.cpp and Foo.h do not exist,
and neither does the Foo class, this caused build failures.

We now resolve the typedefs earlier, and in the example above we would now
generate BooleanOrLong.cpp and BooleanOrLong.h, which works as expected.

Fixing this requires a few separate, intertwined changes:
* IdlUnionType.resolve_typedefs() was broken: the |typedefs| argument it
  receives is a dictionary whose keys are typedef names, not
  IdlTypeBase-derived objects, so |typedefs.get()| was always failing and
  returning the original, unresolved member objects.
* CodeGeneratorUnionType actually needs to call resolve_typedefs(). It was
  the only generator class not doing so. The mechanism needs to be different
  from the other classes though, as IdlUnionType is not a type defined in
  idl_definitions.py as is the case for the types handled by the other
  generators. Instead of using TypedefResolver, we simply call the relevant
  parts of it manually.
* The code manually adding union header includes to interfaces and partial
  interfaces has been dropped: there isn't enough information to resolve the
  typedefs at that point, which meant interface .cpp files would include
  FooOrLong.h, while interface .h files include BooleanOrLong.h. The test
  expectations IDL files show a reduction in unneeded includes, and the
  build seems to work fine.

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

[Raphael Kubo da Costa (rakuco), 2017-03-15 11:16:12]

PTAL.

The only parts I'm not 100% confident about are the changes to
compute_interfaces_info_individual.py and compute_interfaces_info_overall.py.
I'm under the impression the code I've removed was obsoleted by
https://codereview.chromium.org/1975993003, as TypedefResolver takes care of
including the union headers in interface headers (plus I rm -rf'ed my
out/Debug/gen/blink, rebuilt all bindings and everything still compiled fine).

[commit-bot: I haz the power, 2017-03-15 11:36:25]

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

[commit-bot: I haz the power, 2017-03-15 11:36:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Yuki, 2017-03-15 13:52:38]

I'm not an expert in this area though, looks good to me in general.

Defer to bashi@ who is the expert.

[Raphael Kubo da Costa (rakuco), 2017-03-15 17:32:27]

(just fyi, the chromeos browser test failure seems to be unrelated to this CL,
https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...
also looks sick at the moment)

[bashi, 2017-03-15 23:27:11]

Great fix! lgtm. Sorry for the late review (I'm ooo every Wed in JST).

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

https://codereview.chromium.org/2749253002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_v8.py:294:
self.typedefs = {}
Could you explain why do you need to set |typedef| as an instance member? Can we
move this into _generate_container_code()?

[Raphael Kubo da Costa (rakuco), 2017-03-16 08:04:48]

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

https://codereview.chromium.org/2749253002/diff/1/third_party/WebKit/Source/b...
third_party/WebKit/Source/bindings/scripts/code_generator_v8.py:294:
self.typedefs = {}
On 2017/03/15 23:27:11, bashi wrote:
> Could you explain why do you need to set |typedef| as an instance member? Can
we
> move this into _generate_container_code()?

Just to avoid having to iterate through self.info_provider.typedefs on every
_generate_container_code() invocation (which depends on the number of unions we
have) instead of just once when the class is created.

[Raphael Kubo da Costa (rakuco), 2017-03-16 08:20:41]

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

[commit-bot: I haz the power, 2017-03-16 08:20:56]

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-16 08:56:45]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489652441548240, "parent_rev":
"d51024e6850f785d2bb364af8d63f742126f7a91", "commit_rev":
"200763220d087ef3ccf735855398ec193db29658"}

[commit-bot: I haz the power, 2017-03-16 08:57:34]

Description was changed from

==========
bindings: Expand typedefs before generating union files.

Prior to this commit, the IDL compiler was failing to resolve typedefs in
unions before generating new union code. In practice, this meant an IDL
construct such as:

    typedef boolean Foo;
    void func((Foo or long) arg);

Generated FooOrLong.cpp and FooOrLong.h, and "Foo" was considered an IDL
interface instead of a simple bool. Since Foo.cpp and Foo.h do not exist,
and neither does the Foo class, this caused build failures.

We now resolve the typedefs earlier, and in the example above we would now
generate BooleanOrLong.cpp and BooleanOrLong.h, which works as expected.

Fixing this requires a few separate, intertwined changes:
* IdlUnionType.resolve_typedefs() was broken: the |typedefs| argument it
  receives is a dictionary whose keys are typedef names, not
  IdlTypeBase-derived objects, so |typedefs.get()| was always failing and
  returning the original, unresolved member objects.
* CodeGeneratorUnionType actually needs to call resolve_typedefs(). It was
  the only generator class not doing so. The mechanism needs to be different
  from the other classes though, as IdlUnionType is not a type defined in
  idl_definitions.py as is the case for the types handled by the other
  generators. Instead of using TypedefResolver, we simply call the relevant
  parts of it manually.
* The code manually adding union header includes to interfaces and partial
  interfaces has been dropped: there isn't enough information to resolve the
  typedefs at that point, which meant interface .cpp files would include
  FooOrLong.h, while interface .h files include BooleanOrLong.h. The test
  expectations IDL files show a reduction in unneeded includes, and the
  build seems to work fine.

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

to

==========
bindings: Expand typedefs before generating union files.

Prior to this commit, the IDL compiler was failing to resolve typedefs in
unions before generating new union code. In practice, this meant an IDL
construct such as:

    typedef boolean Foo;
    void func((Foo or long) arg);

Generated FooOrLong.cpp and FooOrLong.h, and "Foo" was considered an IDL
interface instead of a simple bool. Since Foo.cpp and Foo.h do not exist,
and neither does the Foo class, this caused build failures.

We now resolve the typedefs earlier, and in the example above we would now
generate BooleanOrLong.cpp and BooleanOrLong.h, which works as expected.

Fixing this requires a few separate, intertwined changes:
* IdlUnionType.resolve_typedefs() was broken: the |typedefs| argument it
  receives is a dictionary whose keys are typedef names, not
  IdlTypeBase-derived objects, so |typedefs.get()| was always failing and
  returning the original, unresolved member objects.
* CodeGeneratorUnionType actually needs to call resolve_typedefs(). It was
  the only generator class not doing so. The mechanism needs to be different
  from the other classes though, as IdlUnionType is not a type defined in
  idl_definitions.py as is the case for the types handled by the other
  generators. Instead of using TypedefResolver, we simply call the relevant
  parts of it manually.
* The code manually adding union header includes to interfaces and partial
  interfaces has been dropped: there isn't enough information to resolve the
  typedefs at that point, which meant interface .cpp files would include
  FooOrLong.h, while interface .h files include BooleanOrLong.h. The test
  expectations IDL files show a reduction in unneeded includes, and the
  build seems to work fine.

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

Review-Url: https://codereview.chromium.org/2749253002
Cr-Commit-Position: refs/heads/master@{#457377}
Committed:
https://chromium.googlesource.com/chromium/src/+/200763220d087ef3ccf735855398...
==========

[commit-bot: I haz the power, 2017-03-16 08:57:36]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/200763220d087ef3ccf735855398...

[haraken, 2017-03-16 10:14:19]

LGTM