Add access checks to Object.preventExtensions + add regression test for 1027....

Add access checks to Object.preventExtensions + add regression test for 1027.

Object.preventExtensions can currently be used cross-domain. With this
change we follow firefox (IE9 has our current behaviour). In addition
this includes a regression test for 1027 and access tests for
Object.seal and Object.freeze.



Committed: http://code.google.com/p/v8/source/detail?r=6848

[antonm, 2011-02-17 15:44:12]

Almost LGTM, one question though

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
nit: dot at the end, please

http://codereview.chromium.org/6534019/diff/1/src/runtime.cc#newcode887
src/runtime.cc:887: if (obj->IsAccessCheckNeeded() &&
Do we want access checks only on the proxy?  Why not on the global object also? 
Is there something special about these queries?

I am asking as apparently the common approach is to do something like that:

Foo(obj)
 // security check for obj.
 if (obj->IsJSGlobalProx()) {
  return Foo(obj->GetPrototype())
 }
 // Real logic

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#newcode...
test/cctest/test-api.cc:5686: "Object.defineProperty("
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

http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:5690: ").blocked_prop");
might be slightly easier for people not that experienced in defineProperty if
split into two parts:

Object.defineProperty(other, 'blocked_prop', ....);
other.blocked_prop

http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:5699: // Seal and freeze uses other functions which
already includes access
function[s] hence include with not s, correct?

http://codereview.chromium.org/6534019/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:5700: // checks, but we check these anyway.
and maybe you can just drop this comment

[Rico, 2011-02-18 07:49:09]

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#newcode...
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#newcode...
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#newcode...
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#newcode...
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.

[antonm, 2011-02-18 10:31:10]

LGTM

http://codereview.chromium.org/6534019/diff/7004/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/6534019/diff/7004/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:5691: // Regression test ofr issue 1171.
nit: ofr => for