Issue 712697  has been merged into this issue.

Short-term workaround: The code generator throws an exception when the generated filename is too long. It should say that "Please add an alias to shorten_union_name() for TooooLongUnionName"

Possible longer-term solutions:
- Use C++ template IDLUnion<T1, T2, ...> and stop generating union type containers. I'm not sure this is doable though.
- Use typedef (ImageBitmapSource in this case) for generated files if exists.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8aa2f436c16b14c49320c78dca9ae64388ef3cd

commit d8aa2f436c16b14c49320c78dca9ae64388ef3cd
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Thu Apr 20 08:30:20 2017

IDL code generator: Throw an exception when a union name is too long

This is a hacky workaround to make sure that the code generator doesn't
generate too long file names for union type containers. This doesn't fix
the problem but asks developers to add an alias for long names.

BUG=711464

Change-Id: Ibfb7d3dc0c128b38f1b60d555f060223d915f792
Reviewed-on: https://chromium-review.googlesource.com/482839
Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#465949}
[modify] https://crrev.com/d8aa2f436c16b14c49320c78dca9ae64388ef3cd/third_party/WebKit/Source/bindings/scripts/utilities.py

bashi@ thanks for doing this. :)
For the long term solution, is there a reason for us not to always use the typedef name?

Couple of reasons.
- There might be more than one typedefs for the same union type.
- A union type may have the same members but the semantics may be different between IDL interfaces. For example,
// Foo.idl
typedef (boolean or double) FooSpecificName;
// Bar.idl
interface Bar {
  void method((boolean or double) arg);
}

Generating Bar::method(FooSpecificName arg) would sound strange.

Suggestions/ideas are highly welcomed :)

One silly idea is to just smash the basename of the IDL file and an arbitrary suffix together, e.g. V8ImageBitmapFactoriesUnions.cpp

(but I guess this would mean that the IDL compiler needs to know which IDL the union definition came from)

Thanks dcheng@!

Yeah IDL compiler needs to know where unions come from. I think we can add such metadata but unclear things still remain like what we should do when two IDL files have the same union definition.

Wouldn't the IDL basename be sufficient to disambiguate this? Or we have multiple IDL files with the same name?

Multiple IDL files can use the same union type.

// Foo.idl
interface Foo {
  void method((boolean or double) arg);
}

// Bar.idl
dictionary Bar {
  (boolean or double) member;
}

I'm not sure which base name should be used for (boolean or double).

One possibility (though it might be awkward to implement) might be to use the typedef name if it is only referred to through the typedef name in IDL.

I have to imagine that the ones overflowing aren't the simple ones, like (boolean or double), but are the ones with many possibilities, like ImageBitmapSource, and it seems unlikely to me that another union be exactly the same by coincidence.


Another option might be to make the union type implemented by a template, and then probably offer a type alias to the typedef names in .idl. It might be tricky to make the resulting type quite as nice as the current generated ones, though. And it's almost certainly overkill. But it would look something like this: https://gist.github.com/jeremyroman/73500b3fa91030451b9f5164035ed534

#10: I'm suggesting that the union type be defined in a file called V8FooUnions.cpp for the union from Foo.idl, and V8BarUnions.cpp for the union from Bar.idl.

#11

Thanks Jeremy for suggestions! About a year ago I tried to use template but gave up due to lack of my template skills. Your code looks promising and now we have ToV8() and NativeValueTraits thanks to yukishiino@ and racuko@ so template approach may be an option. That said I agree that this is overkill. We may want to take your first option.

#12

I got your idea. Probably we may want to avoid duplicates if possible :)

Issue 728584 has been merged into this issue.

 Issue 611545  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb

commit c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb
Author: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Fri Jun 02 14:00:22 2017

bindings: Use a smaller union name length limit (80 instead of 120).

While we do not reach a proper solution to the underlying problem of
generating union file names that are too long, make the accepted length
threshold 80 instead of 120 and convert all names longer than the new
threshold.

This should (at least for now) prevent errors such as the one reported in
https://bugs.chromium.org/p/chromium/issues/detail?id=725996#c10, where the
combination of the builder's name, union file name and target path in the
build directory went over Windows' length limit of 260 characters.

Bug: 711464,  725996 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I69d0afa9baaef1f6c5bd2103d965ac1ad8253488
Reviewed-on: https://chromium-review.googlesource.com/522685
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Raphael Kubo da Costa (rakuco) <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#476632}
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/bindings/modules/v8/generated.gni
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/bindings/scripts/utilities.py
[rename] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/bindings/tests/results/core/NestedUnionType.cpp
[rename] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/bindings/tests/results/core/NestedUnionType.h
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/bindings/tests/results/core/V8TestTypedefs.h
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/modules/canvas2d/BaseRenderingContext2D.h
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/modules/offscreencanvas2d/OffscreenCanvasRenderingContext2D.cpp
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContext.cpp
[modify] https://crrev.com/c0cdd99d33f708afb6ba0a7694482b4bdffbbaeb/third_party/WebKit/Source/modules/webgl/WebGLRenderingContext.cpp

 Issue 700907  has been merged into this issue.

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Reassign to peria@.