Chromium Code Reviews

Issue 7538016: Make window.{undefined,NaN,Infinity} read-only (Closed)

Created:
9 years, 4 months ago by Jakob Kummerow
Modified:
9 years, 4 months ago
Reviewers:
Kevin Millikin (Chromium)
CC:
v8-dev
Visibility:
Public.

Description

Make window.{undefined,NaN,Infinity} read-only as per ES5. BUG=89490 TEST=es5conform 15.1.*, 15.2.3.*; mjsunit/undeletable-functions Committed: http://code.google.com/p/v8/source/detail?r=8766

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Stats (+23 lines, -20 lines)
M src/runtime.cc View 2 chunks +4 lines, -5 lines 0 comments
M src/v8natives.js View 1 chunk +4 lines, -3 lines 0 comments
M test/cctest/test-decls.cc View 1 chunk +1 line, -1 line 0 comments
M test/es5conform/es5conform.status View 1 chunk +0 lines, -10 lines 0 comments
M test/mjsunit/undeletable-functions.js View 2 chunks +3 lines, -1 line 1 comment
M test/sputnik/sputnik.status View 1 chunk +11 lines, -0 lines 0 comments

Messages

Total messages: 2 (0 generated)
Jakob Kummerow
PTAL. And thanks for the help.
9 years, 4 months ago (2011-08-01 09:37:07 UTC) #1
Kevin Millikin (Chromium)
9 years, 4 months ago (2011-08-01 10:06:20 UTC) #2
LGTM.

http://codereview.chromium.org/7538016/diff/1/test/mjsunit/undeletable-functi...
File test/mjsunit/undeletable-functions.js (left):

http://codereview.chromium.org/7538016/diff/1/test/mjsunit/undeletable-functi...
test/mjsunit/undeletable-functions.js:192: assertEquals("foo", type[prop],
"overwritable: " + prop);
I don't understand why we expected "foo" before :)

It seems like someone could rewrite this test to use the ES5 reflective methods
(e.g. Object.getOwnPropertyDescriptor).  Presuming we have good test coverage of
that and that we don't otherwise have special handling of these properties, that
would be more direct.

Powered by Google App Engine