Added test for CONSTANT_FUNCTION and extra tests for CALLBACKS in case
these are
partially redefined (i.e., we only change the get or set).
http://codereview.chromium.org/555149/diff/5002/6005
File src/runtime.cc (right):
http://codereview.chromium.org/555149/diff/5002/6005#newcode601
src/runtime.cc:601: // and API defined callback.
On 2010/02/01 17:36:14, Mads Ager wrote:
and -> an
Done.
http://codereview.chromium.org/555149/diff/5002/6005#newcode2907
src/runtime.cc:2907: if (result.type() == FIELD || result.type() ==
NORMAL )
On 2010/02/01 17:36:14, Mads Ager wrote:
How about CONSTANT_FUNCTION?
I have added a check to object-define-property.js where I test for
CONSTANT_FUNCTION - this works fine without deleting it first.
Is CALLBACK handled correctly if there already is a CALLBACK property?
As discussed offline I have added some new test to check that the it
actually behaves correctly if there is already a CALLBACK property (with
both or just one of {get, set}) and we do a redefine with a descriptor
that has only a get or set. This works as expected (i.e., the get or set
that is undefined in the descriptor keeps it's existing value). The
reason is that the DefineAccessor only redefines the getter or setter
field (this method is actually called twice if we call defineProperty
with a descriptor that has both a getter and a setter, once for each).
http://codereview.chromium.org/555149/diff/5002/6005#newcode2944
src/runtime.cc:2944: return Runtime::ForceSetObjectProperty(js_object,
name, obj_value, attr);
On 2010/02/01 17:36:14, Mads Ager wrote:
Did you need the ForceSet here?
Done.
http://codereview.chromium.org/555149/diff/5002/6008
File test/cctest/test-api.cc (right):
http://codereview.chromium.org/555149/diff/5002/6008#newcode2308
test/cctest/test-api.cc:2308: Local<Script> script_desc =
Script::Compile(v8_str("var prop ="
On 2010/02/01 17:36:14, Mads Ager wrote:
You could give yourself a bit more horizontal space by doing:
Local<Script> script_desc
= Script::Compile(v8_str("..."
"..."));
Done.
http://codereview.chromium.org/555149/diff/5002/6008#newcode2316
test/cctest/test-api.cc:2316: Local<Script> script_define =
On 2010/02/01 17:36:14, Mads Ager wrote:
Move '=' to next line and do a four-space indent. Repeated below.
Done.
http://codereview.chromium.org/555149/diff/5002/6008#newcode2317
test/cctest/test-api.cc:2317: Script::Compile(v8_str("var desc = {get:
function(){return 42; },"
On 2010/02/01 17:36:14, Mads Ager wrote:
Space before "get:". Repeated below.
Done.
http://codereview.chromium.org/555149
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
