Object.observe: Move notification of JSArray length changes to JSArray::SetElementsLength

Object.observe: Move notification of JSArray length changes to JSArray::SetElementsLength

The previous implementation in Accessors::ArraySetLength failed when array length was set through StoreIC_ArrayLength. But that stub and the accessor both delegate to JSArray::SetElementsLength, so moving the code there allows notifications to be sent in both cases.

Committed: https://code.google.com/p/v8/source/detail?r=12962

[rossberg, 2012-11-14 10:19:26]

LGTM, with nit.

https://codereview.chromium.org/11275292/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11275292/diff/1/src/objects.cc#newcode9378
src/objects.cc:9378: MaybeObject* JSArray::SetElementsLength(Object* len) {
Add a comment to objects.h that this can cause GC now.

[rossberg, 2012-11-14 12:13:13]

On 2012/11/14 10:19:26, rossberg wrote:
> LGTM, with nit.

Actually, please add a test case.

[adamk, 2012-11-14 15:56:25]

https://codereview.chromium.org/11275292/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/11275292/diff/1/src/objects.cc#newcode9378
src/objects.cc:9378: MaybeObject* JSArray::SetElementsLength(Object* len) {
On 2012/11/14 10:19:26, rossberg wrote:
> Add a comment to objects.h that this can cause GC now.

Done.

[adamk, 2012-11-14 16:10:18]

On 2012/11/14 12:13:13, rossberg wrote:
> On 2012/11/14 10:19:26, rossberg wrote:
> > LGTM, with nit.
> 
> Actually, please add a test case.

Can you point me in the direction of adding a test case that will reliably fail?
The reasons why my existing test case fails are mysterious to me (e.g., why does
it only happen when using an InternalArray?). Basically, how can I be assured
that we've generated code for setting .length and that we're running the stub
the second time through?

[rossberg, 2012-11-14 16:12:54]

On 2012/11/14 16:10:18, adamk wrote:
> On 2012/11/14 12:13:13, rossberg wrote:
> > On 2012/11/14 10:19:26, rossberg wrote:
> > > LGTM, with nit.
> > 
> > Actually, please add a test case.
> 
> Can you point me in the direction of adding a test case that will reliably
fail?
> The reasons why my existing test case fails are mysterious to me (e.g., why
does
> it only happen when using an InternalArray?). Basically, how can I be assured
> that we've generated code for setting .length and that we're running the stub
> the second time through?

Rafael's original example demonstrating the bug is good enough, I think, and
deterministic.

[adamk, 2012-11-14 16:29:25]

Added test case.

[rossberg, 2012-11-14 16:48:19]

LGTM, landing...