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#newcode5686 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#newcode5690 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#newcode5699 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#newcode5700 test/cctest/test-api.cc:5700: // checks, but we check these anyway. and maybe you can just drop this comment http://codereview.chromium.org/6534019/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
