Search prototypes for accessor setters if interceptor returns empty value.

Search prototypes for accessor setters if interceptor returns empty value.

Extract the part of SetPropertyForResult that searches the prototype chain
for accessor setters into a separate function SetPropertyInPrototypes.
Call this function in SetPropertyPostInterceptor.

This should fix both optimized and unoptimized cases because
the cache stub for storing with interceptor calls the runtime system.

BUG=v8:1636
TEST=cctest/test-api.cc/EmptyInterceptorDoesNotShadowAccessors

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

[ulan, 2011-09-26 07:57:45]

Please take a look.

[Rico, 2011-09-26 08:45:10]

Style comments first:
please Start comments with capital letter and end with period.

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

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1307: const AccessorInfo& info) {
indention

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1313: const AccessorInfo& info) {
indention and put value on seperate line

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1319: const AccessorInfo& info) {
indention

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1331: // intercept names that start with 'interceptor_'
Capitalize I at start of comment

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1331: // intercept names that start with 'interceptor_'
period at the end of comment

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1336: for (i = 0; name_str[i] && prefix[i]; ++i)
Please add {} around body when not on single line

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1345: // intercept accesses that set certain integer
values
Capital I at start of comment, period at the end

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1362: v8::NamedPropertyGetter getter,
Indention

[ulan, 2011-09-26 09:15:18]

Addressed style comments.

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

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1307: const AccessorInfo& info) {
On 2011/09/26 08:45:10, Rico wrote:
> indention

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1313: const AccessorInfo& info) {
On 2011/09/26 08:45:10, Rico wrote:
> indention and put value on seperate line

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1319: const AccessorInfo& info) {
On 2011/09/26 08:45:10, Rico wrote:
> indention

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1331: // intercept names that start with 'interceptor_'
On 2011/09/26 08:45:10, Rico wrote:
> Capitalize I at start of comment

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1331: // intercept names that start with 'interceptor_'
On 2011/09/26 08:45:10, Rico wrote:
> Capitalize I at start of comment

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1336: for (i = 0; name_str[i] && prefix[i]; ++i)
On 2011/09/26 08:45:10, Rico wrote:
> Please add {} around body when not on single line

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1345: // intercept accesses that set certain integer
values
On 2011/09/26 08:45:10, Rico wrote:
> Capital I at start of comment, period at the end

Done.

http://codereview.chromium.org/7991007/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:1362: v8::NamedPropertyGetter getter,
On 2011/09/26 08:45:10, Rico wrote:
> Indention

Done.

[Rico, 2011-09-26 10:37:36]

LGTM

http://codereview.chromium.org/7991007/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode2004
src/objects.cc:2004: name,
indention

http://codereview.chromium.org/7991007/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2008
src/objects.h:2008: 
Could we add a comment here stating that this may return found=true even if this
is giving back a failure, I at least had to convince myself that in case of GC
during the callwould always be safe. Also, this would be nice to know for other
callsites in the future (i.e., that even when found, they should check the
result if they do not immediately return it)

http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2009
src/objects.h:2009: MUST_USE_RESULT MaybeObject* SetPropertyInPrototypes(
could we use a more descriptive name for this?
Like, SetPropertyWithCallbackSetterInPrototypes

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

http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:1340: return self->GetHiddenValue(v8_str(name_str+i));
space around +

http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:1437: // All i < 10000 go to interceptor.
go to interceptor -> go to the interceptor
same below and same in next two functions

[Rico, 2011-09-26 10:40:42]

http://codereview.chromium.org/7991007/diff/2002/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/2002/src/objects.cc#newcode2023
src/objects.cc:2023: bool found = false;
Move comment about using handlified references up to here, state that
SetPropertyWithHandlerIfDefiningSetter can cause GC

[ulan, 2011-09-26 12:56:07]

Addressed feedback. Merged latest changes (which probably I should have done
after the review).

http://codereview.chromium.org/7991007/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode2004
src/objects.cc:2004: name,
On 2011/09/26 10:37:36, Rico wrote:
> indention

Done.

http://codereview.chromium.org/7991007/diff/1/src/objects.h
File src/objects.h (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2008
src/objects.h:2008: 
On 2011/09/26 10:37:36, Rico wrote:
> Could we add a comment here stating that this may return found=true even if
this
> is giving back a failure, I at least had to convince myself that in case of GC
> during the callwould always be safe. Also, this would be nice to know for
other
> callsites in the future (i.e., that even when found, they should check the
> result if they do not immediately return it)

Done.

http://codereview.chromium.org/7991007/diff/1/src/objects.h#newcode2009
src/objects.h:2009: MUST_USE_RESULT MaybeObject* SetPropertyInPrototypes(
On 2011/09/26 10:37:36, Rico wrote:
> could we use a more descriptive name for this?
> Like, SetPropertyWithCallbackSetterInPrototypes

Done.

http://codereview.chromium.org/7991007/diff/2002/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/2002/src/objects.cc#newcode2023
src/objects.cc:2023: bool found = false;
On 2011/09/26 10:40:42, Rico wrote:
> Move comment about using handlified references up to here, state that
> SetPropertyWithHandlerIfDefiningSetter can cause GC

Done.

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

http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:1340: return self->GetHiddenValue(v8_str(name_str+i));
On 2011/09/26 10:37:36, Rico wrote:
> space around +

Done.

http://codereview.chromium.org/7991007/diff/2002/test/cctest/test-api.cc#newc...
test/cctest/test-api.cc:1437: // All i < 10000 go to interceptor.
On 2011/09/26 10:37:36, Rico wrote:
> go to interceptor -> go to the interceptor
> same below and same in next two functions

Done.

[Vyacheslav Egorov (Chromium), 2011-09-26 13:52:06]

drive by

http://codereview.chromium.org/7991007/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode1666
src/objects.cc:1666: &found, strict_mode);
We usually use one-arguments-per-line formatting for calls that do not fit onto
one line.

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode1667
src/objects.cc:1667: if (found) return result_object;
Some compilers will complain that found can be uninitialized here. Please add
initializer to the variable.

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode2587
src/objects.cc:2587: result_object = SetPropertyInPrototypes(name, value,
attributes,
ditto

[ulan, 2011-09-26 14:17:57]

Addressed feedback, landing.

http://codereview.chromium.org/7991007/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode1666
src/objects.cc:1666: &found, strict_mode);
On 2011/09/26 13:52:07, Vyacheslav Egorov wrote:
> We usually use one-arguments-per-line formatting for calls that do not fit
onto
> one line. 

Done.

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode1667
src/objects.cc:1667: if (found) return result_object;
On 2011/09/26 13:52:07, Vyacheslav Egorov wrote:
> Some compilers will complain that found can be uninitialized here. Please add
> initializer to the variable.

Done.

http://codereview.chromium.org/7991007/diff/1/src/objects.cc#newcode2587
src/objects.cc:2587: result_object = SetPropertyInPrototypes(name, value,
attributes,
On 2011/09/26 13:52:07, Vyacheslav Egorov wrote:
> ditto

Done.