Object.observe: notify when element addition causes array growth

Object.observe: notify when element addition causes array growth

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

[rossberg, 2012-11-08 16:50:19]

Looks good so far, but I think you need to look for other code paths that might
set array elements, especially when doing it through Object.defineProperty.

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc#newcode...
src/objects.cc:10322: if (!preexists && self->IsJSArray())
Maybe make that an "else if (self->IsJSArray())"

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc#newcode...
src/objects.cc:10349: Handle<Object> newValue = Object::GetElement(self, index);
Not your change, but while you're at it, can you also fix this misspelling of
'newValue' I overlooked?

[adamk, 2012-11-08 17:08:17]

Added test cases with defineProperty (both accessor and data properties). Also
reversed ordering to match ES5's [[DefineOwnProperty]] on Array:

http://ecma-international.org/ecma-262/5.1/#sec-15.4.5.1

I figured out where the length update was coming from: v8natives.js has
DefineArrayProperty() which handles updating length.

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc#newcode...
src/objects.cc:10322: if (!preexists && self->IsJSArray())
On 2012/11/08 16:50:20, rossberg wrote:
> Maybe make that an "else if (self->IsJSArray())"

Done.

https://chromiumcodereview.appspot.com/11369135/diff/1/src/objects.cc#newcode...
src/objects.cc:10349: Handle<Object> newValue = Object::GetElement(self, index);
On 2012/11/08 16:50:20, rossberg wrote:
> Not your change, but while you're at it, can you also fix this misspelling of
> 'newValue' I overlooked?

Fixed.

[rafaelw, 2012-11-09 10:06:27]

lgtm

[rossberg, 2012-11-09 10:51:42]

LGTM, landing...