Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(611)

Issue 953123003: IDL: Put generated union type containers in separate files (Closed)

Created:
5 years, 10 months ago by bashi
Modified:
4 years, 3 months ago
Reviewers:
haraken, Jens Widell, tasak
CC:
blink-reviews, dshwang, arv+blink, vivekg_samsung, blink-reviews-html_chromium.org, Justin Novosad, vivekg, dglazkov+blink, Rik, blink-reviews-bindings_chromium.org, aandrey+blink_chromium.org
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

IDL: Put generated union type containers in separate files Currently, we put all union type containers in a single file per component (i.e. UnionTypesCore.h and UnionTypesModules.h) and use forward-declarations to avoid unnecessary recompilation. However, this has been the root cause of following issues: - It makes a union type impossible to have a dictionary type. A union type depends on the class's definition. - We need actual definitions when we want to link core/modules/web separately on Windows. To solve these issues, this CL puts union type containers in separate files. The UnionTypes{Core,Modules}.{cpp,h} still exist; they "include" all union containers and are listed as GYP/GN output files in GYP/GN rules. BUG=240176

Patch Set 1 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2727 lines, -2654 lines) Patch
M Source/bindings/scripts/code_generator_v8.py View 1 chunk +84 lines, -21 lines 0 comments Download
M Source/bindings/scripts/v8_union.py View 5 chunks +11 lines, -40 lines 0 comments Download
M Source/bindings/templates/templates.gypi View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/bindings/templates/union.h View 2 chunks +1 line, -76 lines 0 comments Download
M Source/bindings/templates/union.cpp View 1 chunk +2 lines, -190 lines 0 comments Download
A + Source/bindings/templates/union_container.h View 2 chunks +23 lines, -29 lines 0 comments Download
A + Source/bindings/templates/union_container.cpp View 7 chunks +46 lines, -51 lines 0 comments Download
A Source/bindings/tests/results/core/ArrayBufferOrArrayBufferViewOrDictionary.h View 1 chunk +76 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/ArrayBufferOrArrayBufferViewOrDictionary.cpp View 1 chunk +128 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/BooleanOrStringOrUnrestrictedDouble.h View 1 chunk +74 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/BooleanOrStringOrUnrestrictedDouble.cpp View 1 chunk +126 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/DoubleOrString.h View 1 chunk +77 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/DoubleOrString.cpp View 1 chunk +99 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/NodeOrNodeList.h View 1 chunk +75 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/NodeOrNodeList.cpp View 1 chunk +106 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrArrayBufferOrArrayBufferView.h View 1 chunk +76 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrArrayBufferOrArrayBufferView.cpp View 1 chunk +127 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrDouble.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrDouble.cpp View 1 chunk +99 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrStringSequence.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/StringOrStringSequence.cpp View 1 chunk +99 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestEnumOrDouble.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestEnumOrDouble.cpp View 1 chunk +109 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterface2OrUint8Array.h View 1 chunk +69 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterface2OrUint8Array.cpp View 1 chunk +100 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceGarbageCollectedOrString.h View 1 chunk +70 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceGarbageCollectedOrString.cpp View 1 chunk +104 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceOrLong.h View 1 chunk +72 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceOrLong.cpp View 1 chunk +105 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceOrTestInterfaceEmpty.h View 1 chunk +73 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceOrTestInterfaceEmpty.cpp View 1 chunk +100 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceWillBeGarbageCollectedOrTestDictionary.h View 1 chunk +72 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/TestInterfaceWillBeGarbageCollectedOrTestDictionary.cpp View 1 chunk +107 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/core/UnionTypesCore.h View 2 chunks +15 lines, -730 lines 0 comments Download
M Source/bindings/tests/results/core/UnionTypesCore.cpp View 1 chunk +15 lines, -1355 lines 0 comments Download
A Source/bindings/tests/results/core/UnrestrictedDoubleOrString.h View 1 chunk +67 lines, -0 lines 0 comments Download
A Source/bindings/tests/results/core/UnrestrictedDoubleOrString.cpp View 1 chunk +99 lines, -0 lines 0 comments Download
A + Source/bindings/tests/results/modules/BooleanOrString.h View 2 chunks +4 lines, -6 lines 0 comments Download
A + Source/bindings/tests/results/modules/BooleanOrString.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/bindings/tests/results/modules/UnionTypesModules.h View 2 chunks +2 lines, -58 lines 0 comments Download
M Source/bindings/tests/results/modules/UnionTypesModules.cpp View 1 chunk +2 lines, -93 lines 0 comments Download
M Source/core/html/HTMLCanvasElement.h View 2 chunks +1 line, -1 line 0 comments Download
M Source/core/html/HTMLCanvasElement.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/canvas/CanvasRenderingContext2D.h View 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/html/canvas/CanvasRenderingContext2D.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/html/canvas/CanvasRenderingContext2DTest.cpp View 1 chunk +1 line, -0 lines 0 comments Download
M Source/modules/fetch/Request.h View 2 chunks +1 line, -1 line 0 comments Download
M Source/modules/fetch/Request.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 5 (2 generated)
bashi
PTAL? Sorry for asking a large review, but most part of the change is generated ...
5 years, 10 months ago (2015-02-25 04:17:43 UTC) #3
Jens Widell
I worry a bit about the generation of files that gyp/gn (and thus ninja) knows ...
5 years, 10 months ago (2015-02-25 09:27:56 UTC) #4
bashi
5 years, 10 months ago (2015-02-26 01:35:51 UTC) #5
On 2015/02/25 09:27:56, Jens Widell wrote:
> I worry a bit about the generation of files that gyp/gn (and thus ninja) knows
> nothing about. It will probably work well enough in practice, but it is
> technically wrong, and if for instance one of the unmentioned generated files
go
> missing, ninja won't know that it should exist or what to do to generate it.
> Also, cleaning ("ninja -t clean") won't work properly.
> 
> (I don't know that this would be the first instance of such an error. In fact,
> I'd be somewhat surprised if it were.)

Your concern makes sense. I'll ask GYP/GN guys a workaround.

Powered by Google App Engine
This is Rietveld 408576698