Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(123)

Issues Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 1185843010: bindings: Moves custom getters/setters to the prototype chain. (Closed)

Created:
4 years, 10 months ago by Yuki
Modified:
4 years, 10 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: Moves custom getters/setters to the prototype chain. Moves custom attributes to the prototype chain, and makes them accessor- type properties. BUG=43394 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197453

Patch Set 1 #

Patch Set 2 : Updated the layout test results. #

Patch Set 3 : Synced. #

Total comments: 2

Patch Set 4 : Addressed a review comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -58 lines) Patch
M LayoutTests/virtual/stable/webexposed/global-interface-listing-dedicated-worker-expected.txt View 1 3 chunks +4 lines, -0 lines 0 comments Download
M LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 6 chunks +8 lines, -0 lines 0 comments Download
M LayoutTests/virtual/stable/webexposed/global-interface-listing-shared-worker-expected.txt View 1 3 chunks +4 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-dedicated-worker-expected.txt View 1 3 chunks +4 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-expected.txt View 1 6 chunks +8 lines, -0 lines 0 comments Download
M LayoutTests/webexposed/global-interface-listing-shared-worker-expected.txt View 1 3 chunks +4 lines, -0 lines 0 comments Download
M Source/bindings/core/v8/custom/V8CustomEventCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/core/v8/custom/V8ErrorEventCustom.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M Source/bindings/core/v8/custom/V8HTMLOptionsCollectionCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/core/v8/custom/V8MessageEventCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/core/v8/custom/V8PopStateEventCustom.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/bindings/core/v8/custom/V8XMLHttpRequestCustom.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/bindings/scripts/v8_attributes.py View 1 2 3 1 chunk +2 lines, -5 lines 0 comments Download
M Source/bindings/scripts/v8_utilities.py View 1 2 1 chunk +0 lines, -5 lines 0 comments Download
M Source/bindings/templates/interface.h View 1 chunk +8 lines, -0 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestObject.h View 1 chunk +11 lines, -11 lines 0 comments Download
M Source/bindings/tests/results/core/V8TestObject.cpp View 1 2 7 chunks +36 lines, -29 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
Yuki
Could you guys review this CL?
4 years, 10 months ago (2015-06-18 13:11:08 UTC) #2
haraken
LGTM! Thanks vivekg -- I realized we now have a very small number of custom ...
4 years, 10 months ago (2015-06-18 15:56:23 UTC) #3
bashi
LGTM https://codereview.chromium.org/1185843010/diff/40001/Source/bindings/scripts/v8_attributes.py File Source/bindings/scripts/v8_attributes.py (right): https://codereview.chromium.org/1185843010/diff/40001/Source/bindings/scripts/v8_attributes.py#newcode523 Source/bindings/scripts/v8_attributes.py:523: # chains. Is this comment still valid?
4 years, 10 months ago (2015-06-18 23:50:54 UTC) #4
Yuki
https://codereview.chromium.org/1185843010/diff/40001/Source/bindings/scripts/v8_attributes.py File Source/bindings/scripts/v8_attributes.py (right): https://codereview.chromium.org/1185843010/diff/40001/Source/bindings/scripts/v8_attributes.py#newcode523 Source/bindings/scripts/v8_attributes.py:523: # chains. On 2015/06/18 23:50:53, bashi1 wrote: > Is ...
4 years, 10 months ago (2015-06-19 06:34:48 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1185843010/60001
4 years, 10 months ago (2015-06-19 06:35:06 UTC) #8
Yuki
On 2015/06/18 15:56:23, haraken wrote: > LGTM! > > Thanks vivekg -- I realized we ...
4 years, 10 months ago (2015-06-19 07:03:29 UTC) #9
commit-bot: I haz the power
4 years, 10 months ago (2015-06-19 07:59:01 UTC) #10
Message was sent while issue was closed. 
Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197453

Powered by Google App Engine
This is Rietveld 408576698