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

Issue 186673002: Support deprecation + use counters for constructor attributes. (Closed)

Created:
6 years, 9 months ago by sof
Modified:
6 years, 9 months ago
CC:
blink-reviews, kojih, arv+blink, jsbell+bindings_chromium.org, abarth-chromium, falken, marja+watch_chromium.org, adamk+blink_chromium.org, horo+watch_chromium.org, kinuko+watch, Nate Chapin, watchdog-blink-watchlist_google.com, Inactive
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Support deprecation + use counters for constructor attributes. If a constructor attribute is annotated with either [MeasureAs=..] or [DeprecateAs=..], then emit a needed callback wrapper that performs the required reporting before looking up the constructor. Use it to correctly handle use counters for the "webkitURL" properties that both Window and WorkerGlobalScope currently provide; use counters also added for these here. (The above change also happens to fix use counter reporting for the window.WebKitShadowRoot constructor.) R=nbarth@chromium.org,haraken@chromium.org BUG=348985 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168576

Patch Set 1 #

Total comments: 2

Patch Set 2 : IDL style fix #

Patch Set 3 : Always generate constructor getter callbacks #

Total comments: 7

Patch Set 4 : Revert to initial selective callback generation scheme + adjust generated callback naming #

Patch Set 5 : Rebased (UseCounter.h conflict) #

Total comments: 6

Patch Set 6 : Renamings #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -4 lines) Patch
M Source/bindings/scripts/v8_attributes.py View 1 2 3 4 5 2 chunks +5 lines, -0 lines 0 comments Download
M Source/bindings/templates/attributes.cpp View 1 2 3 4 5 1 chunk +19 lines, -0 lines 0 comments Download
M Source/bindings/templates/interface.cpp View 1 2 3 4 5 1 chunk +4 lines, -1 line 0 comments Download
M Source/bindings/templates/interface_base.cpp View 1 2 3 4 5 2 chunks +7 lines, -1 line 0 comments Download
M Source/bindings/tests/idls/TestObject.idl View 1 2 chunks +2 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/V8TestObject.cpp View 1 2 3 4 5 3 chunks +18 lines, -0 lines 0 comments Download
M Source/core/frame/UseCounter.h View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M Source/core/frame/Window.idl View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/workers/WorkerGlobalScope.idl View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 23 (0 generated)
sof
Please take a look.
6 years, 9 months ago (2014-03-04 14:42:51 UTC) #1
haraken
How about generating the constructor getter callback always? For other DOM attributes/methods, we always generate ...
6 years, 9 months ago (2014-03-04 15:24:11 UTC) #2
sof
On 2014/03/04 15:24:11, haraken wrote: > How about generating the constructor getter callback always? > ...
6 years, 9 months ago (2014-03-04 15:32:13 UTC) #3
sof
https://codereview.chromium.org/186673002/diff/1/Source/bindings/tests/idls/TestObject.idl File Source/bindings/tests/idls/TestObject.idl (right): https://codereview.chromium.org/186673002/diff/1/Source/bindings/tests/idls/TestObject.idl#newcode61 Source/bindings/tests/idls/TestObject.idl:61: [MeasureAs=TestFeature]attribute TestSubObjConstructor TestSubObjMeasured; On 2014/03/04 15:24:11, haraken wrote: > ...
6 years, 9 months ago (2014-03-04 15:32:22 UTC) #4
haraken
On 2014/03/04 15:32:13, sof wrote: > On 2014/03/04 15:24:11, haraken wrote: > > How about ...
6 years, 9 months ago (2014-03-04 15:35:40 UTC) #5
sof
On 2014/03/04 15:35:40, haraken wrote: > On 2014/03/04 15:32:13, sof wrote: > > On 2014/03/04 ...
6 years, 9 months ago (2014-03-04 15:53:36 UTC) #6
sof
On 2014/03/04 15:53:36, sof wrote: > On 2014/03/04 15:35:40, haraken wrote: > > On 2014/03/04 ...
6 years, 9 months ago (2014-03-04 16:12:27 UTC) #7
haraken
> > > I think that would be ok; any objections? > > > > ...
6 years, 9 months ago (2014-03-05 00:53:39 UTC) #8
sof
On 2014/03/05 00:53:39, haraken wrote: > > > > I think that would be ok; ...
6 years, 9 months ago (2014-03-05 06:25:01 UTC) #9
haraken
> > > > I think we just need to make sure that it won't ...
6 years, 9 months ago (2014-03-05 06:54:25 UTC) #10
sof
On 2014/03/05 06:54:25, haraken wrote: > > > > > I think we just need ...
6 years, 9 months ago (2014-03-05 06:58:13 UTC) #11
Nils Barth (inactive)
On 2014/03/05 00:53:39, haraken wrote: > > > > I think that would be ok; ...
6 years, 9 months ago (2014-03-05 07:26:40 UTC) #12
Nils Barth (inactive)
Few style points, but otherwise CG and IDL changes LGTM. (Still need to decide whether ...
6 years, 9 months ago (2014-03-05 07:27:51 UTC) #13
Nils Barth (inactive)
On 2014/03/05 07:27:51, Nils Barth wrote: > Few style points, but otherwise CG and IDL ...
6 years, 9 months ago (2014-03-05 07:34:01 UTC) #14
sof
On 2014/03/05 06:54:25, haraken wrote: > > > > > I think we just need ...
6 years, 9 months ago (2014-03-05 11:04:25 UTC) #15
Nils Barth (inactive)
On 2014/03/05 11:04:25, sof wrote: > With manually enabling icf (icf=safe) and the gold linker, ...
6 years, 9 months ago (2014-03-05 11:56:54 UTC) #16
sof
Thanks much for the detailed review & comments. Dropped back to the selective generation of ...
6 years, 9 months ago (2014-03-05 12:45:48 UTC) #17
haraken
LGTM Though I'm a bit surprised that generating the constructor getter callbacks all the time ...
6 years, 9 months ago (2014-03-05 14:34:56 UTC) #18
sof
https://codereview.chromium.org/186673002/diff/70001/Source/bindings/scripts/v8_attributes.py File Source/bindings/scripts/v8_attributes.py (right): https://codereview.chromium.org/186673002/diff/70001/Source/bindings/scripts/v8_attributes.py#newcode403 Source/bindings/scripts/v8_attributes.py:403: contents['needs_constructor_callback'] = contents['measure_as'] or contents['deprecate_as'] On 2014/03/05 14:34:57, haraken ...
6 years, 9 months ago (2014-03-05 14:56:45 UTC) #19
sof
The CQ bit was checked by sigbjornf@opera.com
6 years, 9 months ago (2014-03-05 14:56:50 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/186673002/90001
6 years, 9 months ago (2014-03-05 14:57:19 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sigbjornf@opera.com/186673002/90001
6 years, 9 months ago (2014-03-05 20:59:10 UTC) #22
commit-bot: I haz the power
6 years, 9 months ago (2014-03-06 05:51:26 UTC) #23
Message was sent while issue was closed.
Change committed as 168576

Powered by Google App Engine
This is Rietveld 408576698