Map v8::Object to v8::internal::JSReceiver

Map v8::Object to v8::internal::JSReceiver

BUG=none
R=verwaest@chromium.org,rossberg@chromium.org,bmeurer@chromium.org,neis@chromium.org
LOG=y

Committed: https://crrev.com/6ce7f90aba752951ec8468550f8e40bbea157616
Cr-Commit-Position: refs/heads/master@{#32014}

[jochen (gone - plz use gerrit), 2015-10-27 15:19:59]

wip

I'll add JSReceiver versions of

JSObject::DefinePropertyOrElementIgnoreAttributes
JSObject::CreateDataProperty
JSObject::SetPrototype
JSObject::SetHiddenProperty
JSObject::GetHiddenProperty

what about SetAccessor? It's a bit like CreateDataProperty?

the following methods will be JSObject only:

ForceSet
Clone
HasRealNamedProperty
HasRealIndexedProperty
HasRealNamedCallbackProperty
HasNamedLoopupInterceptor
HasIndexedLookupInterceptor

[rossberg, 2015-10-27 16:09:59]

Description was changed from

==========
Map v8::Object to v8::internal::JSReceiver

BUG=none
R=verwaest@chromium.org,rossberg@chromium.org,bmeurer@chromium.org
LOG=y
==========

to

==========
Map v8::Object to v8::internal::JSReceiver

BUG=none
R=verwaest@chromium.org,rossberg@chromium.org,bmeurer@chromium.org,neis@chrom...
LOG=y
==========

[rossberg, 2015-10-27 16:09:59]

rossberg@chromium.org changed reviewers:
+ neis@chromium.org

[Toon Verwaest, 2015-11-02 09:41:26]

https://codereview.chromium.org/1413463006/diff/1/src/accessors.cc
File src/accessors.cc (right):

https://codereview.chromium.org/1413463006/diff/1/src/accessors.cc#newcode169
src/accessors.cc:169: // TODO(jochen): Update to JSReceiver.
These methods are guaranteed to be called directly on the known receiver type,
so there's not need to call out to JSReceiver:: here. The cast above is valid.

[rossberg, 2015-11-04 10:43:56]

https://codereview.chromium.org/1413463006/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1413463006/diff/1/src/api.cc#newcode4050
src/api.cc:4050: if (!self->IsJSObject()) return Nothing<bool>();
Doesn't Nothing indicate an exception? Wouldn't you need to prepare one here
then? (Similarly below) Alternatively, just return false, which would be
consistent with the Has*LookupInterceptor functions below.

https://codereview.chromium.org/1413463006/diff/1/src/api.h
File src/api.h (right):

https://codereview.chromium.org/1413463006/diff/1/src/api.h#newcode240
src/api.h:240: static inline Local<Object> ReceiverToLocal(
Is there a reason to name this specially? So far, we only do that for functions
whose argument type does not uniquely/canonically determine the result type.

https://codereview.chromium.org/1413463006/diff/1/src/api.h#newcode372
src/api.h:372: MAKE_TO_LOCAL(ReceiverToLocal, JSReceiver, Object)
Same here.

[jochen (gone - plz use gerrit), 2015-11-06 22:23:28]

ptal

https://codereview.chromium.org/1413463006/diff/20001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1413463006/diff/20001/src/api.cc#newcode3543
src/api.cc:3543: i::Handle<i::JSObject>::cast(self))) {
proxies can't have access checks, so that's safe

https://codereview.chromium.org/1413463006/diff/20001/src/api.cc#newcode3579
src/api.cc:3579: auto self =
i::Handle<i::JSObject>::cast(Utils::OpenHandle(this));
deprecated method, just don't call this on proxies

https://codereview.chromium.org/1413463006/diff/20001/src/api.cc#newcode4068
src/api.cc:4068: if (!self->IsJSObject()) return Just(false);
we probably want to have hasOwnProperty and getOwnPropertyDescriptor like APIs
that then also work with proxies

[rossberg, 2015-11-09 13:31:46]

https://codereview.chromium.org/1413463006/diff/1/src/api.h
File src/api.h (right):

https://codereview.chromium.org/1413463006/diff/1/src/api.h#newcode240
src/api.h:240: static inline Local<Object> ReceiverToLocal(
On 2015/11/04 10:43:56, rossberg wrote:
> Is there a reason to name this specially? So far, we only do that for
functions
> whose argument type does not uniquely/canonically determine the result type.

What about this one?

[jochen (gone - plz use gerrit), 2015-11-09 22:42:04]

https://codereview.chromium.org/1413463006/diff/1/src/api.h
File src/api.h (right):

https://codereview.chromium.org/1413463006/diff/1/src/api.h#newcode240
src/api.h:240: static inline Local<Object> ReceiverToLocal(
On 2015/11/09 at 13:31:46, rossberg wrote:
> On 2015/11/04 10:43:56, rossberg wrote:
> > Is there a reason to name this specially? So far, we only do that for
functions
> > whose argument type does not uniquely/canonically determine the result type.
> 
> What about this one?

that's the case here - JSReceiver could also be a v8::Function()

[rossberg, 2015-11-10 07:04:48]

https://codereview.chromium.org/1413463006/diff/1/src/api.h
File src/api.h (right):

https://codereview.chromium.org/1413463006/diff/1/src/api.h#newcode240
src/api.h:240: static inline Local<Object> ReceiverToLocal(
On 2015/11/09 22:42:04, jochen (slow - traveling) wrote:
> On 2015/11/09 at 13:31:46, rossberg wrote:
> > On 2015/11/04 10:43:56, rossberg wrote:
> > > Is there a reason to name this specially? So far, we only do that for
> functions
> > > whose argument type does not uniquely/canonically determine the result
type.
> > 
> > What about this one?
> 
> that's the case here - JSReceiver could also be a v8::Function()

I don't understand. Not all receivers are functions, so that mapping would not
make sense as a generic choice. v8::Object is the most specific API type that
can represent all receivers.

[jochen (gone - plz use gerrit), 2015-11-16 15:23:22]

Added ToLocal for JSReceiver/JSObject/JSProxy now

ptal

[rossberg, 2015-11-16 15:26:08]

Cute, LGTM

[Toon Verwaest, 2015-11-16 15:29:52]

lgtm

[jochen (gone - plz use gerrit), 2015-11-16 15:59:22]

The CQ bit was checked by jochen@chromium.org

[jochen (gone - plz use gerrit), 2015-11-16 15:59:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org, verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1413463006/#ps60001
(title: "updates")

[commit-bot: I haz the power, 2015-11-16 15:59:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1413463006/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1413463006/60001

[commit-bot: I haz the power, 2015-11-16 16:48:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-16 16:49:03]

Patchset 4 (id:??) landed as
https://crrev.com/6ce7f90aba752951ec8468550f8e40bbea157616
Cr-Commit-Position: refs/heads/master@{#32014}