Object.observe: Handle oldValue for elements with accessors properly.

Object.observe: Handle oldValue for elements with accessors properly.

Extended ElementAccessor interface to allow querying PropertyType and
AccessorPair. Also added respective functionality to JSObject.

R=mstarzinger@chromium.org
BUG=

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

[Michael Starzinger, 2012-11-14 17:30:39]

LGTM with two comments.

https://codereview.chromium.org/11358234/diff/1/src/elements.cc
File src/elements.cc (right):

https://codereview.chromium.org/11358234/diff/1/src/elements.cc#newcode1551
src/elements.cc:1551: return AccessorPair::cast(backing_store->ValueAt(entry));
This could potentially also be a Foreign callback or a AccessorInfo callback. I
am unsure if that ever happens in any of our embedders, so we could just add a
CHECK (on top of the implicit assertion) here that the value we get out actually
is a AccessorPair. WDYT?

https://codereview.chromium.org/11358234/diff/1/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/11358234/diff/1/src/objects.h#newcode1844
src/objects.h:1844: enum LocalElementKind {
As discussed offline, we should try to get rid of this enum completely. But
until then I am fine with the new name.

[Sven Panne, 2012-11-15 06:36:32]

https://codereview.chromium.org/11358234/diff/1/src/elements.cc
File src/elements.cc (right):

https://codereview.chromium.org/11358234/diff/1/src/elements.cc#newcode1551
src/elements.cc:1551: return AccessorPair::cast(backing_store->ValueAt(entry));
On 2012/11/14 17:30:39, Michael Starzinger wrote:
> This could potentially also be a Foreign callback or a AccessorInfo callback.
I
> am unsure if that ever happens in any of our embedders, so we could just add a
> CHECK (on top of the implicit assertion) here that the value we get out
actually
> is a AccessorPair. WDYT?

The Foreign case is completely under our control and (very probably) doesn't
matter here, but the unhandled AccessorInfo case really makes me nervous: If I
understand things correctly, this would change our external API with unknown
consequences. I would very much prefer avoiding this. If there is really, really
no way around this, the restriction (which one exactly?) should be documented in
a very visible way in our external header.

[rossberg, 2012-11-15 11:25:42]

https://codereview.chromium.org/11358234/diff/1/src/elements.cc
File src/elements.cc (right):

https://codereview.chromium.org/11358234/diff/1/src/elements.cc#newcode1551
src/elements.cc:1551: return AccessorPair::cast(backing_store->ValueAt(entry));
On 2012/11/15 06:36:32, Sven Panne wrote:
> On 2012/11/14 17:30:39, Michael Starzinger wrote:
> > This could potentially also be a Foreign callback or a AccessorInfo
callback.
> I
> > am unsure if that ever happens in any of our embedders, so we could just add
a
> > CHECK (on top of the implicit assertion) here that the value we get out
> actually
> > is a AccessorPair. WDYT?
> 
> The Foreign case is completely under our control and (very probably) doesn't
> matter here, but the unhandled AccessorInfo case really makes me nervous: If I
> understand things correctly, this would change our external API with unknown
> consequences. I would very much prefer avoiding this. If there is really,
really
> no way around this, the restriction (which one exactly?) should be documented
in
> a very visible way in our external header.

That was an oversight. Added IsAccessorPair condition, returning NULL otherwise
(analogous to JSObject::GetLocalPropertyAccessorPair).