Make JSObjects with both indexed interceptors and indexed accessors work safe...

Make JSObjects with both indexed interceptors and indexed accessors work safely.

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

[William Hesse, 2009-06-10 13:39:57]

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

http://codereview.chromium.org/118499/diff/1/3#newcode5215
Line 5215: int entry = dictionary->FindNumberEntry(index);
Code is now an exact copy of the same code in SetElement.

[Mads Ager (chromium), 2009-06-10 13:52:44]

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

http://codereview.chromium.org/118499/diff/1/3#newcode5215
Line 5215: int entry = dictionary->FindNumberEntry(index);
As we discussed, this is a significant amount of code and we should refactor to
share it.

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

http://codereview.chromium.org/118499/diff/1/2#newcode2301
Line 2301: templ->SetIndexedPropertyHandler(0, 0,
CheckThisIndexedPropertyQuery);
It is unclear to me that we are testing the right thing when we only have an
indexed query interceptor.  Could you do this with a getter and a setter
interceptor instead and also test that they are both actually called (and have
them both return an empty handle).

http://codereview.chromium.org/118499/diff/1/2#newcode2310
Line 2310: for (int i = 0; i < 10; i++) {
Why is this in a loop?

[William Hesse, 2009-06-10 14:50:03]

All changes made.

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

http://codereview.chromium.org/118499/diff/1/3#newcode5215
Line 5215: int entry = dictionary->FindNumberEntry(index);
On 2009/06/10 13:52:44, Mads Ager wrote:
> As we discussed, this is a significant amount of code and we should refactor
to
> share it.

Refactored into SetElementWithoutInterceptor().

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

http://codereview.chromium.org/118499/diff/1/2#newcode2301
Line 2301: templ->SetIndexedPropertyHandler(0, 0,
CheckThisIndexedPropertyQuery);
On 2009/06/10 13:52:44, Mads Ager wrote:
> It is unclear to me that we are testing the right thing when we only have an
> indexed query interceptor.  Could you do this with a getter and a setter
> interceptor instead and also test that they are both actually called (and have
> them both return an empty handle).

Done.

http://codereview.chromium.org/118499/diff/1/2#newcode2310
Line 2310: for (int i = 0; i < 10; i++) {
On 2009/06/10 13:52:44, Mads Ager wrote:
> Why is this in a loop?

Because the test I used as a framework had a loop.  I thought they all did. 
They don't.  Gone now.

[Mads Ager (chromium), 2009-06-10 15:22:00]

LGTM once you remove the GetElementPostInterceptor method and its declaration.