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

Issue 1193793003: bindings: Makes almost all attributes accessor-type properties. (Closed)

Created:
5 years, 6 months ago by Yuki
Modified:
5 years, 5 months ago
Reviewers:
haraken, bashi
CC:
blink-reviews, blink-reviews-bindings_chromium.org, vivekg_samsung, arv+blink, vivekg
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

bindings: Makes almost all attributes accessor-type properties. 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 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 because v8::Template::SetAccessorProperty doesn't support inheriting accessors on InstanceTemplate. BUG=43394, 491006 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198687

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : Synced. #

Patch Set 5 : Synced. #

Patch Set 6 : #

Patch Set 7 : Synced. #

Patch Set 8 : Applies v8::AccessControl::Default to accessor-type properties because the parameter is meaningless… #

Patch Set 9 : Updated an expected error message in a test: ParameterizedWebFrameTest::CrossDomainAccessErrorsUseC… #

Patch Set 10 : Added hacks for WorkerCacheStorage.idl:[Unforgeable] caches #

Patch Set 11 : Synced. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+473 lines, -484 lines) Patch
M LayoutTests/http/tests/dom/location-stringify.html View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/http/tests/security/xss-DENIED-assign-location-href-javascript-expected.txt View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/scripts/v8_attributes.py View 1 2 4 chunks +8 lines, -40 lines 0 comments Download
M Source/bindings/scripts/v8_interface.py View 1 2 3 4 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 1 2 3 4 5 6 7 12 chunks +25 lines, -24 lines 0 comments Download
M Source/bindings/templates/interface.h View 1 2 3 4 1 chunk +6 lines, -6 lines 0 comments Download
M Source/bindings/templates/interface_base.cpp View 1 2 3 4 5 4 chunks +19 lines, -26 lines 0 comments Download
M Source/bindings/tests/results/core/V8SVGTestInterface.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestException.cpp View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterface.cpp View 1 2 3 4 5 6 7 18 chunks +73 lines, -70 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceAccessors.cpp View 1 2 3 4 5 6 7 4 chunks +9 lines, -20 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp View 1 2 3 4 5 6 7 1 chunk +7 lines, -7 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceEventInitConstructor.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceGarbageCollected.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceNode.cpp View 1 2 3 4 5 6 7 1 chunk +7 lines, -7 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceOwnProperties.cpp View 1 2 3 4 5 6 7 6 chunks +14 lines, -24 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceOwnPropertiesDerived.cpp View 1 2 3 4 5 6 7 6 chunks +14 lines, -24 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestInterfaceWillBeGarbageCollected.cpp View 1 2 3 4 5 6 7 4 chunks +8 lines, -16 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestNode.cpp View 1 2 3 4 5 6 7 1 chunk +4 lines, -4 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestObject.cpp View 1 2 3 4 5 6 7 8 chunks +185 lines, -182 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestTypedefs.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/tests/results/modules/V8TestInterface5.cpp View 1 2 3 4 5 6 7 5 chunks +14 lines, -13 lines 0 comments Download
M Source/core/dom/Document.idl View 1 2 3 4 5 6 7 8 9 10 1 chunk +6 lines, -0 lines 0 comments Download
M Source/core/dom/XMLDocument.idl View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M Source/core/html/HTMLDocument.idl View 1 2 3 4 5 6 7 8 9 10 1 chunk +8 lines, -0 lines 0 comments Download
A + Source/modules/cachestorage/CompositorWorkerCacheStorage.idl View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/DedicatedWorkerCacheStorage.idl View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/ServiceWorkerCacheStorage.idl View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -1 line 0 comments Download
A + Source/modules/cachestorage/SharedWorkerCacheStorage.idl View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -1 line 0 comments Download
M Source/modules/cachestorage/WorkerCacheStorage.idl View 1 2 3 4 5 6 7 8 9 1 chunk +7 lines, -1 line 0 comments Download
M Source/modules/modules.gypi View 1 2 3 4 5 6 7 8 9 10 1 chunk +4 lines, -0 lines 0 comments Download
M Source/web/tests/WebFrameTest.cpp View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 12 (3 generated)
Yuki
Could you guys review this CL?
5 years, 5 months ago (2015-07-07 09:23:09 UTC) #2
haraken
LGTM Can we remove [ExposeJSAccessors] in a follow-up CL?
5 years, 5 months ago (2015-07-07 12:54:25 UTC) #3
bashi
LGTM
5 years, 5 months ago (2015-07-07 23:30:00 UTC) #4
Yuki
On 2015/07/07 12:54:25, haraken wrote: > LGTM > > Can we remove [ExposeJSAccessors] in a ...
5 years, 5 months ago (2015-07-08 08:19:02 UTC) #5
Yuki
On 2015/07/08 08:19:02, Yuki wrote: > On 2015/07/07 12:54:25, haraken wrote: > > LGTM > ...
5 years, 5 months ago (2015-07-10 12:13:25 UTC) #6
haraken
On 2015/07/10 12:13:25, Yuki wrote: > On 2015/07/08 08:19:02, Yuki wrote: > > On 2015/07/07 ...
5 years, 5 months ago (2015-07-10 13:07:59 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1193793003/200001
5 years, 5 months ago (2015-07-10 13:27:10 UTC) #10
commit-bot: I haz the power
Committed patchset #11 (id:200001) as https://src.chromium.org/viewvc/blink?view=rev&revision=198687
5 years, 5 months ago (2015-07-10 13:31:20 UTC) #11
Dirk Pranke
5 years, 5 months ago (2015-07-10 17:09:13 UTC) #12
Message was sent while issue was closed.
A revert of this CL (patchset #11 id:200001) has been created in
https://codereview.chromium.org/1226203012/ by dpranke@chromium.org.

The reason for reverting is: This causes failures in content_browsertests, e.g.:

http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests/builds/45398

I verified that this was the issue via 'git bisect' on linux; it should be
trivial to reproduce.

Sorry!.

Powered by Google App Engine
This is Rietveld 408576698