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

Issue 1240433002: bindings: Makes almost all attributes accessor-type properties. (2nd try) (Closed)

Created:
5 years, 5 months ago by Yuki
Modified:
5 years, 5 months ago
Reviewers:
haraken, bashi
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, vivekg_samsung, sof, eae+blinkwatch, vivekg, dglazkov+blink, blink-reviews-bindings_chromium.org, Inactive, rwlbuis
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

bindings: Makes almost all attributes accessor-type properties. (2nd try) Introduces |attribute.is_data_type_property| as a template variable to specify the property type. is_data_type_property is true only for - attributes of Window and Location interface just because we'd like to move on step by step. We'll make Window's attributes accessor-type in a following CL. - constructors which must be implemented as methods rather than attributes. This is another task. is_data_type_property will be gone once we resolve the above two cases. Note that we need an undesired hack in {HTML,XML}Document.idl and {Compositor,Dedicated,Service,Shared}WorkerCacheStorage.idl because v8::Template::SetAccessorProperty doesn't support inheriting accessors on InstanceTemplate. The 1st try is: https://codereview.chromium.org/1193793003/ BUG=43394, 491006 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198852

Patch Set 1 : Patched from http://crrev.com/1193793003/ #

Patch Set 2 : Excludes Location in addition to Window. #

Total comments: 3

Patch Set 3 : Reverted Location-related test results. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+470 lines, -481 lines) Patch
M Source/bindings/scripts/v8_attributes.py View 1 4 chunks +8 lines, -40 lines 0 comments Download
M Source/bindings/scripts/v8_interface.py View 1 chunk +7 lines, -7 lines 0 comments Download
M Source/bindings/scripts/v8_utilities.py View 2 chunks +4 lines, -0 lines 0 comments Download
M Source/bindings/templates/attributes.cpp View 12 chunks +25 lines, -24 lines 0 comments Download
M Source/bindings/templates/interface.h View 1 chunk +6 lines, -6 lines 0 comments Download
M Source/bindings/templates/interface_base.cpp View 4 chunks +19 lines, -26 lines 0 comments Download
M Source/bindings/tests/results/core/V8SVGTestInterface.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestException.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterface.cpp View 18 chunks +73 lines, -70 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceAccessors.cpp View 4 chunks +9 lines, -20 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp View 1 chunk +7 lines, -7 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceEventInitConstructor.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceNode.cpp View 1 chunk +7 lines, -7 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceOwnProperties.cpp View 6 chunks +14 lines, -24 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceOwnPropertiesDerived.cpp View 6 chunks +14 lines, -24 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceWillBeGarbageCollected.cpp View 4 chunks +8 lines, -16 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestNode.cpp View 1 chunk +4 lines, -4 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestObject.cpp View 8 chunks +185 lines, -182 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestTypedefs.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/modules/V8TestInterface5.cpp View 5 chunks +14 lines, -13 lines 0 comments Download
M Source/core/dom/Document.idl View 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/dom/XMLDocument.idl View 1 chunk +7 lines, -0 lines 0 comments Download
M Source/core/html/HTMLDocument.idl View 1 chunk +8 lines, -0 lines 0 comments Download
A + Source/modules/cachestorage/CompositorWorkerCacheStorage.idl View 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/DedicatedWorkerCacheStorage.idl View 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/ServiceWorkerCacheStorage.idl View 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/SharedWorkerCacheStorage.idl View 1 chunk +7 lines, -1 line 0 comments Download
M Source/modules/cachestorage/WorkerCacheStorage.idl View 1 chunk +7 lines, -1 line 0 comments Download
M Source/modules/modules.gypi View 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Yuki
Could you guys review this CL? The difference from the previous CL is that we ...
5 years, 5 months ago (2015-07-14 08:00:48 UTC) #3
haraken
On 2015/07/14 08:00:48, Yuki wrote: > Could you guys review this CL? > > The ...
5 years, 5 months ago (2015-07-14 08:19:04 UTC) #4
bashi
LGTM https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts/v8_utilities.py File Source/bindings/scripts/v8_utilities.py (right): https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts/v8_utilities.py#newcode424 Source/bindings/scripts/v8_utilities.py:424: # TODO(yukishiino): Implement this function following the spec. ...
5 years, 5 months ago (2015-07-14 08:20:19 UTC) #5
Yuki
https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts/v8_utilities.py File Source/bindings/scripts/v8_utilities.py (right): https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts/v8_utilities.py#newcode424 Source/bindings/scripts/v8_utilities.py:424: # TODO(yukishiino): Implement this function following the spec. On ...
5 years, 5 months ago (2015-07-14 09:07:59 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1240433002/60001
5 years, 5 months ago (2015-07-14 09:21:38 UTC) #9
commit-bot: I haz the power
Committed patchset #3 (id:60001) as https://src.chromium.org/viewvc/blink?view=rev&revision=198852
5 years, 5 months ago (2015-07-14 10:44:42 UTC) #10
bashi
5 years, 5 months ago (2015-07-14 23:43:01 UTC) #11
Message was sent while issue was closed.
https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts...
File Source/bindings/scripts/v8_utilities.py (right):

https://codereview.chromium.org/1240433002/diff/40001/Source/bindings/scripts...
Source/bindings/scripts/v8_utilities.py:424: # TODO(yukishiino): Implement this
function following the spec.
On 2015/07/14 09:07:59, Yuki wrote:
> On 2015/07/14 08:20:19, bashi1 wrote:
> > BTW, what remains to say this function follows the spec?
> 
> The final form of this function in my mind is:
> 
> return (is_unforgeable(interface, member) or
>         is_global(interface))
> 
> I think we don't need to rely on on_prototype.

Got it. Thanks for the explanation.

Powered by Google App Engine
This is Rietveld 408576698