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

Issue 315003004: IDL build: Split global object computation into core and modules (Closed)

Created:
6 years, 6 months ago by Nils Barth (inactive)
Modified:
6 years, 6 months ago
Reviewers:
haraken
CC:
blink-reviews, blink-reviews-bindings_chromium.org, arv+blink, abarth-chromium
Visibility:
Public.

Description

IDL build: Split global object computation into core and modules This completes the split! Note that this deletes the top-level: * bindings/generated.gypi * bindings/idl.gypi ...and eliminates the variable bindings_output_dir, b/c we do not output anything to there any longer! One layering violation is: constructors for interfaces in core need to know about global objects defined in modules, namely ServiceWorker, because some of these are exposed to ServiceWorker. This can be fixed by moving ServiceWorkerGlobalScopeCoreConstructors.idl to modules, which I'll do in a followup. R=haraken BUG=358074 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175554

Patch Set 1 #

Patch Set 2 : Cleanup #

Total comments: 3

Patch Set 3 : Move modules_global_objects to core/generated to eliminate circular dependency of GYP files #

Unified diffs Side-by-side diffs Delta from patch set Stats (+108 lines, -130 lines) Patch
M Source/bindings/bindings.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M Source/bindings/core/core.gypi View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/core/generated.gyp View 1 2 4 chunks +72 lines, -3 lines 0 comments Download
D Source/bindings/generated.gyp View 1 chunk +0 lines, -77 lines 0 comments Download
D Source/bindings/idl.gypi View 1 chunk +0 lines, -25 lines 0 comments Download
M Source/bindings/modules/generated.gyp View 1 2 3 chunks +3 lines, -4 lines 0 comments Download
M Source/bindings/modules/modules.gypi View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/scripts/compute_global_objects.py View 5 chunks +23 lines, -9 lines 0 comments Download
M Source/bindings/scripts/compute_interfaces_info_overall.py View 3 chunks +2 lines, -9 lines 0 comments Download
M Source/bindings/scripts/utilities.py View 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
Nils Barth (inactive)
Last split!
6 years, 6 months ago (2014-06-05 02:20:24 UTC) #1
haraken
LGTM https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp File Source/bindings/modules/generated.gyp (right): https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp#newcode163 Source/bindings/modules/generated.gyp:163: '<(bindings_core_output_dir)/GlobalObjectsCore.pickle', Do we need GlobalObjectsCore.pickle?
6 years, 6 months ago (2014-06-05 03:05:09 UTC) #2
Nils Barth (inactive)
https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp File Source/bindings/modules/generated.gyp (right): https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp#newcode163 Source/bindings/modules/generated.gyp:163: '<(bindings_core_output_dir)/GlobalObjectsCore.pickle', On 2014/06/05 03:05:10, haraken wrote: > Do we ...
6 years, 6 months ago (2014-06-05 03:10:53 UTC) #3
Nils Barth (inactive)
The CQ bit was checked by nbarth@chromium.org
6 years, 6 months ago (2014-06-05 03:10:57 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nbarth@chromium.org/315003004/20001
6 years, 6 months ago (2014-06-05 03:11:41 UTC) #5
Nils Barth (inactive)
https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp File Source/bindings/modules/generated.gyp (right): https://codereview.chromium.org/315003004/diff/20001/Source/bindings/modules/generated.gyp#newcode163 Source/bindings/modules/generated.gyp:163: '<(bindings_core_output_dir)/GlobalObjectsCore.pickle', On 2014/06/05 03:05:10, haraken wrote: > Do we ...
6 years, 6 months ago (2014-06-05 03:23:58 UTC) #6
Nils Barth (inactive)
The CQ bit was checked by nbarth@chromium.org
6 years, 6 months ago (2014-06-05 03:30:19 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nbarth@chromium.org/315003004/40001
6 years, 6 months ago (2014-06-05 03:30:28 UTC) #8
commit-bot: I haz the power
6 years, 6 months ago (2014-06-05 11:19:43 UTC) #9
Message was sent while issue was closed.
Change committed as 175554

Powered by Google App Engine
This is Rietveld 408576698