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

Issue 1380503002: binding: Makes Window/Location's attributes accessor-type properties. (Closed)

Created:
5 years, 2 months ago by Yuki
Modified:
4 years, 7 months ago
Reviewers:
tkent, haraken
CC:
blink-reviews, blink-reviews-bindings_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

binding: Makes Window/Location's attributes accessor-type properties. Makes almost all the attributes (except for cross-origin accessible attributes) accessor-type properties. The target attributes in this CL are DOM attributes of Window, Location and workers not annotated as [DoNotCheckSecurity], which are cross-origin accessible. BUG=43394, 516274 Committed: https://crrev.com/9ac1750adf85027985c7af3468e0b972c2086235 Cr-Commit-Position: refs/heads/master@{#390893}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : Synced. #

Patch Set 5 : #

Patch Set 6 : Synced. #

Patch Set 7 : #

Patch Set 8 : #

Patch Set 9 : #

Patch Set 10 : Synced. #

Patch Set 11 : Synced. #

Patch Set 12 : Synced. #

Patch Set 13 : Updated layout tests. #

Patch Set 14 : Synced. #

Patch Set 15 : Synced. #

Patch Set 16 : Fixed layout tests. #

Patch Set 17 : Updated layout tests. #

Patch Set 18 : Synced. #

Patch Set 19 : #

Patch Set 20 : Synced. #

Patch Set 21 : #

Patch Set 22 : Fixed a critical bug. #

Patch Set 23 : Fixed [LenientThis], etc. #

Patch Set 24 : Updated the test for cross-origin accesses. #

Total comments: 4

Patch Set 25 : Addressed a review comment + fixed a comment. #

Patch Set 26 : Synced. #

Patch Set 27 : Updated a test result. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+784 lines, -503 lines) Patch
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-navigated-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-removed-and-gced-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-properties-after-frame-removed-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-navigated-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-and-gced-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/property-access-on-cached-window-after-frame-removed-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/replaceable.html View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/Window/replaceable-expected.txt View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/constructed-objects-prototypes-expected.txt View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/everything-to-string-expected.txt View 1 2 3 4 5 6 18 19 20 21 22 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/dom/lenient-this.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +37 lines, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/fast/js/global-constructors-expected.txt View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/dom/location-stringify.html View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/cross-origin-access-over-property-descriptor.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +21 lines, -18 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/cross-origin-access-over-property-descriptor-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 1 chunk +13 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/isolatedWorld/all-window-properties-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/security/isolatedWorld/all-window-prototypes-expected.txt View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/imported/web-platform-tests/IndexedDB/interfaces-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/imported/web-platform-tests/html/browsers/the-window-object/window-properties-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +93 lines, -93 lines 0 comments Download
M third_party/WebKit/LayoutTests/imported/web-platform-tests/webstorage/idlharness-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 5 chunks +249 lines, -134 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 5 chunks +257 lines, -139 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +0 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/idl_types.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/v8_attributes.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 11 chunks +39 lines, -22 lines 0 comments Download
M third_party/WebKit/Source/bindings/templates/attributes.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 4 chunks +10 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterface.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 8 chunks +29 lines, -47 lines 0 comments Download

Messages

Total messages: 28 (11 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1380503002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1380503002/180001
5 years, 1 month ago (2015-11-06 06:34:54 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/131374)
5 years, 1 month ago (2015-11-06 08:09:32 UTC) #4
Yuki
Finally, this CL has got ready for review, I think. Could you review this CL?
4 years, 7 months ago (2016-04-27 09:16:03 UTC) #7
haraken
Congrats to reaching this point! https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right): https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py#newcode217 third_party/WebKit/Source/bindings/scripts/v8_attributes.py:217: # boolean or DOMString ...
4 years, 7 months ago (2016-04-27 11:08:55 UTC) #8
Yuki
https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right): https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py#newcode217 third_party/WebKit/Source/bindings/scripts/v8_attributes.py:217: # boolean or DOMString at least. We're making the ...
4 years, 7 months ago (2016-04-27 15:09:56 UTC) #9
haraken
On 2016/04/27 15:09:56, Yuki wrote: > https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py > File third_party/WebKit/Source/bindings/scripts/v8_attributes.py (right): > > https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/scripts/v8_attributes.py#newcode217 > ...
4 years, 7 months ago (2016-04-27 16:15:01 UTC) #10
haraken
https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp File third_party/WebKit/Source/bindings/templates/attributes.cpp (right): https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp#newcode89 third_party/WebKit/Source/bindings/templates/attributes.cpp:89: v8SetReturnValueNull(info); Does this need to be checked as well?
4 years, 7 months ago (2016-04-27 16:15:13 UTC) #11
Yuki
https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp File third_party/WebKit/Source/bindings/templates/attributes.cpp (right): https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp#newcode89 third_party/WebKit/Source/bindings/templates/attributes.cpp:89: v8SetReturnValueNull(info); On 2016/04/27 16:15:12, haraken wrote: > > Does ...
4 years, 7 months ago (2016-04-28 07:53:39 UTC) #12
haraken
On 2016/04/28 07:53:39, Yuki wrote: > https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp > File third_party/WebKit/Source/bindings/templates/attributes.cpp (right): > > https://codereview.chromium.org/1380503002/diff/460001/third_party/WebKit/Source/bindings/templates/attributes.cpp#newcode89 > ...
4 years, 7 months ago (2016-04-28 07:57:20 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1380503002/520001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1380503002/520001
4 years, 7 months ago (2016-04-28 11:31:20 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/174576)
4 years, 7 months ago (2016-04-28 11:39:48 UTC) #18
Yuki
+tkent@, could you review this CL as an API owner? This CL changes webexposed/global-interface-listing-expected.txt .
4 years, 7 months ago (2016-04-28 11:54:51 UTC) #20
tkent
lgtm
4 years, 7 months ago (2016-04-28 13:22:18 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1380503002/520001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1380503002/520001
4 years, 7 months ago (2016-05-02 02:07:22 UTC) #23
commit-bot: I haz the power
Committed patchset #27 (id:520001)
4 years, 7 months ago (2016-05-02 05:04:32 UTC) #25
commit-bot: I haz the power
Patchset 27 (id:??) landed as https://crrev.com/9ac1750adf85027985c7af3468e0b972c2086235 Cr-Commit-Position: refs/heads/master@{#390893}
4 years, 7 months ago (2016-05-02 05:05:33 UTC) #27
Yuki
4 years, 7 months ago (2016-05-09 06:43:22 UTC) #28
Message was sent while issue was closed.
A revert of this CL (patchset #27 id:520001) has been created in
https://codereview.chromium.org/1958883003/ by yukishiino@chromium.org.

The reason for reverting is: We hit a (huge) performance regression.  See
http://crbug.com/608475 ..

Powered by Google App Engine
This is Rietveld 408576698