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

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
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 Delta from patch set Stats (+23 lines, -20 lines) Patch
M src/runtime.cc View 2 chunks +4 lines, -5 lines 0 comments Download
M src/v8natives.js View 1 chunk +4 lines, -3 lines 0 comments Download
M test/cctest/test-decls.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/es5conform/es5conform.status View 1 chunk +0 lines, -10 lines 0 comments Download
M test/mjsunit/undeletable-functions.js View 2 chunks +3 lines, -1 line 1 comment Download
M test/sputnik/sputnik.status View 1 chunk +11 lines, -0 lines 0 comments Download

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
This is Rietveld 408576698