I changed this a bit and moved the access check to the
JSObject::PreventExtensions call (where we also already had support for
JSGlobalProxy - but no access checks).
Please have another look.
http://codereview.chromium.org/6534019/diff/1/src/runtime.cc
File src/runtime.cc (right):
http://codereview.chromium.org/6534019/diff/1/src/runtime.cc#newcode886
src/runtime.cc:886: // If access checks fail simply return false
On 2011/02/17 15:44:12, antonm wrote:
nit: dot at the end, please
Done.
http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):
http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode5686
test/cctest/test-api.cc:5686: "Object.defineProperty("
On 2011/02/17 15:44:12, antonm wrote:
feel free to ignore: I would group lines into something like
"Object.defineProperty(\n"
" other, 'blocked_prop', { configurable: false}).blocked_prop")
because for me C's " really stand in the way, esp. for
"'blocked_prop'"
But if you find your version more readable, that's perfectly ok
Done.
http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode5690
test/cctest/test-api.cc:5690: ").blocked_prop");
On 2011/02/17 15:44:12, antonm wrote:
might be slightly easier for people not that experienced in
defineProperty if
split into two parts:
Object.defineProperty(other, 'blocked_prop', ....);
other.blocked_prop
Done.
http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode5699
test/cctest/test-api.cc:5699: // Seal and freeze uses other functions
which already includes access
On 2011/02/17 15:44:12, antonm wrote:
function[s] hence include with not s, correct?
Changed comment
http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode5700
test/cctest/test-api.cc:5700: // checks, but we check these anyway.
On 2011/02/17 15:44:12, antonm wrote:
and maybe you can just drop this comment
Done.
http://codereview.chromium.org/6534019/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
