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

Issue 261243002: Add support for [Global] / [PrimaryGlobal] IDL extended attributes (Closed)

Created:
6 years, 7 months ago by Inactive
Modified:
6 years, 7 months ago
CC:
blink-reviews, jsbell+serviceworker_chromium.org, serviceworker-reviews, arv+blink, jsbell+bindings_chromium.org, tzik, sof, kouhei+bindings_chromium.org, kinuko, nhiroki, abarth-chromium, falken, marja+watch_chromium.org, blink-reviews-bindings_chromium.org, horo+watch_chromium.org, kinuko+watch, Nate Chapin, watchdog-blink-watchlist_google.com, alecflett+watch_chromium.org
Visibility:
Public.

Description

Add support for [Global] / [PrimaryGlobal] IDL extended attributes Add support for [Global] / [PrimaryGlobal] IDL extended attributes as per the latest Web IDL specification: http://heycam.github.io/webidl/#Global The [Global] and [PrimaryGlobal] extended attributes can be used to give a name to one or more global interfaces, which can then be referenced by the [Exposed] extended attribute (already supported). One slight difference with the specification is that the values for [Global] and [PrimaryGlobal] are '&'-separated instead of comma-separated. This is because having comma-separated here would make IDL parsing a lot harder. This is consistent with the [Exposed] IDL extended attribute. There is no web exposed behavior change with this CL. It just makes our IDL closer to the specification. R=haraken@chromium.org, nbarth@chromium.org BUG=369115 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=173595

Patch Set 1 #

Patch Set 2 : Add more comments #

Patch Set 3 : Remove non-ASCII character in comment #

Total comments: 28

Patch Set 4 : Take feedback into consideration #

Total comments: 13

Patch Set 5 : Fix nits #

Total comments: 2

Patch Set 6 : Fix nit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+77 lines, -34 lines) Patch
M Source/bindings/IDLExtendedAttributes.txt View 2 chunks +3 lines, -1 line 0 comments Download
M Source/bindings/generated_bindings.gyp View 3 chunks +0 lines, -4 lines 0 comments Download
M Source/bindings/scripts/generate_global_constructors.py View 1 2 3 4 5 4 chunks +59 lines, -21 lines 0 comments Download
M Source/bindings/scripts/utilities.py View 1 2 3 2 chunks +9 lines, -6 lines 0 comments Download
M Source/core/frame/Window.idl View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/workers/DedicatedWorkerGlobalScope.idl View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/workers/SharedWorkerGlobalScope.idl View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M Source/modules/serviceworkers/ServiceWorkerGlobalScope.idl View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
Inactive
https://codereview.chromium.org/261243002/diff/40001/Source/bindings/generated_bindings.gyp File Source/bindings/generated_bindings.gyp (left): https://codereview.chromium.org/261243002/diff/40001/Source/bindings/generated_bindings.gyp#oldcode108 Source/bindings/generated_bindings.gyp:108: '<(blink_output_dir)/WorkerGlobalScopeConstructors.idl', This is split between SharedWorkerGlobalScopeConstructors.idl and DedicatedWorkerGlobalScopeConstructors.idl now. ...
6 years, 7 months ago (2014-05-04 21:40:39 UTC) #1
haraken
LGTM, but nbarth@ might want to take another look.
6 years, 7 months ago (2014-05-05 03:10:43 UTC) #2
Inactive
On 2014/05/05 03:10:43, haraken wrote: > LGTM, but nbarth@ might want to take another look. ...
6 years, 7 months ago (2014-05-05 13:02:08 UTC) #3
Inactive
Nils, you around? :)
6 years, 7 months ago (2014-05-06 11:29:22 UTC) #4
haraken
On 2014/05/06 11:29:22, Chris Dumez wrote: > Nils, you around? :) Unfortunately Japan is national ...
6 years, 7 months ago (2014-05-06 11:30:37 UTC) #5
Inactive
On 2014/05/06 11:30:37, haraken wrote: > On 2014/05/06 11:29:22, Chris Dumez wrote: > > Nils, ...
6 years, 7 months ago (2014-05-06 11:32:26 UTC) #6
Nils Barth (inactive)
Thanks Chris, and appreciate your patience! 2 substantive points, then various style points. Main substance ...
6 years, 7 months ago (2014-05-07 01:55:56 UTC) #7
Inactive
On 2014/05/07 01:55:56, Nils Barth wrote: > Thanks Chris, and appreciate your patience! > > ...
6 years, 7 months ago (2014-05-07 02:20:04 UTC) #8
Nils Barth (inactive)
On 2014/05/07 02:20:04, Chris Dumez wrote: > Thanks for the review Nils. It has been ...
6 years, 7 months ago (2014-05-07 02:24:25 UTC) #9
Inactive
https://codereview.chromium.org/261243002/diff/40001/Source/bindings/scripts/generate_global_constructors.py File Source/bindings/scripts/generate_global_constructors.py (right): https://codereview.chromium.org/261243002/diff/40001/Source/bindings/scripts/generate_global_constructors.py#newcode54 Source/bindings/scripts/generate_global_constructors.py:54: # If the [Global] or [PrimaryGlobal] extended attribute is ...
6 years, 7 months ago (2014-05-07 15:45:11 UTC) #10
Nils Barth (inactive)
LGTM with nits - thanks for the revisions! https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py File Source/bindings/scripts/generate_global_constructors.py (right): https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py#newcode56 Source/bindings/scripts/generate_global_constructors.py:56: """ ...
6 years, 7 months ago (2014-05-08 00:37:03 UTC) #11
Inactive
https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py File Source/bindings/scripts/generate_global_constructors.py (right): https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py#newcode56 Source/bindings/scripts/generate_global_constructors.py:56: """ On 2014/05/08 00:37:03, Nils Barth wrote: > Could ...
6 years, 7 months ago (2014-05-08 01:09:28 UTC) #12
Nils Barth (inactive)
One nit. https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py File Source/bindings/scripts/generate_global_constructors.py (right): https://codereview.chromium.org/261243002/diff/60001/Source/bindings/scripts/generate_global_constructors.py#newcode80 Source/bindings/scripts/generate_global_constructors.py:80: constructors = [] On 2014/05/08 01:09:29, Chris ...
6 years, 7 months ago (2014-05-08 01:13:54 UTC) #13
Inactive
https://codereview.chromium.org/261243002/diff/80001/Source/bindings/scripts/generate_global_constructors.py File Source/bindings/scripts/generate_global_constructors.py (right): https://codereview.chromium.org/261243002/diff/80001/Source/bindings/scripts/generate_global_constructors.py#newcode183 Source/bindings/scripts/generate_global_constructors.py:183: if len(unknown_global_names) > 0: On 2014/05/08 01:13:54, Nils Barth ...
6 years, 7 months ago (2014-05-08 01:17:02 UTC) #14
Inactive
The CQ bit was checked by ch.dumez@samsung.com
6 years, 7 months ago (2014-05-08 01:17:11 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/261243002/100001
6 years, 7 months ago (2014-05-08 01:17:47 UTC) #16
Nils Barth (inactive)
On 2014/05/08 01:17:02, Chris Dumez wrote: > Done. Sorry I suck at using the Google ...
6 years, 7 months ago (2014-05-08 01:29:36 UTC) #17
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 7 months ago (2014-05-08 02:29:58 UTC) #18
commit-bot: I haz the power
6 years, 7 months ago (2014-05-08 03:07:46 UTC) #19
Message was sent while issue was closed.
Change committed as 173595

Powered by Google App Engine
This is Rietveld 408576698