[es6] Make sure we call add property when adding a new property

[es6] Make sure we call add property when adding a new property

When setting a property using `super.prop = val` we need to use
add property if we are adding a new property and not set property.

BUG=493566
LOG=N
R=verwaest@chromium.org, dslomov@chromium.org

Committed: https://crrev.com/5f72593d166e1d604d6061573bd220da5ce64db4
Cr-Commit-Position: refs/heads/master@{#28971}

[arv (Not doing code reviews), 2015-05-29 14:38:17]

arv@chromium.org changed reviewers:
+ verwaest@chromium.org

[arv (Not doing code reviews), 2015-05-29 14:38:27]

PTAL

[arv (Not doing code reviews), 2015-06-01 13:59:16]

Ping

[arv (Not doing code reviews), 2015-06-01 14:17:59]

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

https://codereview.chromium.org/1161073002/diff/1/src/objects.cc#newcode3336
src/objects.cc:3336: return AddDataProperty(&own_lookup, value, NONE,
language_mode,
AddDataProperty has a DCHECK that the receiver is not a proxy.

Can found be false afte a proxy trap is invoked?

[caitp (gmail), 2015-06-01 14:36:38]

caitpotter88@gmail.com changed reviewers:
+ caitpotter88@gmail.com

[caitp (gmail), 2015-06-01 14:36:38]

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

https://codereview.chromium.org/1161073002/diff/1/src/objects.cc#newcode3336
src/objects.cc:3336: return AddDataProperty(&own_lookup, value, NONE,
language_mode,
On 2015/06/01 14:17:58, arv wrote:
> AddDataProperty has a DCHECK that the receiver is not a proxy.
> 
> Can found be false afte a proxy trap is invoked?

In the current implementation, found can be false if the property key is a
Symbol (the proxy trap is never invoked in this case).

[Toon Verwaest, 2015-06-01 14:37:03]

lgtm

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

https://codereview.chromium.org/1161073002/diff/1/src/objects.cc#newcode3336
src/objects.cc:3336: return AddDataProperty(&own_lookup, value, NONE,
language_mode,
On 2015/06/01 14:17:58, arv wrote:
> AddDataProperty has a DCHECK that the receiver is not a proxy.
> 
> Can found be false afte a proxy trap is invoked?

Not if the receiver is the proxy, as is the case here.

[Toon Verwaest, 2015-06-01 14:39:08]

Actually, I think this is not right. You have to do the DATA thing after ruling
out the interceptor / access check case.

[Toon Verwaest, 2015-06-01 14:41:30]

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

https://codereview.chromium.org/1161073002/diff/1/src/objects.cc#newcode3336
src/objects.cc:3336: return AddDataProperty(&own_lookup, value, NONE,
language_mode,
On 2015/06/01 14:37:03, Toon Verwaest wrote:
> On 2015/06/01 14:17:58, arv wrote:
> > AddDataProperty has a DCHECK that the receiver is not a proxy.
> > 
> > Can found be false afte a proxy trap is invoked?
> 
> Not if the receiver is the proxy, as is the case here.

I think you'd want to make this switch nested in a for-loop like is the case for
other Set with iterator. After ruling out the interceptor / access check, you
just continue with whatever is behind the interceptor / access check. Behind the
access check could be an interceptor. Behind the interceptor could be data, or
not-found.

[arv (Not doing code reviews), 2015-06-01 14:42:13]

not lgtm then

[arv (Not doing code reviews), 2015-06-01 14:42:32]

On 2015/06/01 14:42:13, arv wrote:
> not lgtm then

That didn't work ;-)

[Toon Verwaest, 2015-06-01 14:57:16]

right. not lgtm

[arv (Not doing code reviews), 2015-06-01 16:00:26]

Use a loop and add proxy tests

[arv (Not doing code reviews), 2015-06-01 16:01:58]

PTAL

I added a loop so that it keeps going until NOT_FOUND. I also added some tests
for the JSProxy path.

[arv (Not doing code reviews), 2015-06-08 18:29:23]

arv@chromium.org changed reviewers:
+ dslomov@chromium.org

[arv (Not doing code reviews), 2015-06-08 18:29:43]

Dmitry, welcome back. Can you take a look?

[arv (Not doing code reviews), 2015-06-08 18:29:43]

Dmitry, welcome back. Can you take a look?

[Dmitry Lomov (no reviews), 2015-06-10 08:13:13]

On 2015/06/08 18:29:43, arv wrote:
> Dmitry, welcome back. Can you take a look?

lgtm but Toon is back too, would be nice if he can have a look as well

[arv (Not doing code reviews), 2015-06-11 20:04:42]

Ping. Toon, can you take a look?

[Toon Verwaest, 2015-06-11 20:07:16]

lgtm... but I actually already fixed it and landed it as part of another CL,
since I was cleaning up all SetProperty related code.

[Toon Verwaest, 2015-06-11 20:07:34]

(not the test though)

[arv (Not doing code reviews), 2015-06-11 20:46:41]

On 2015/06/11 20:07:34, Toon Verwaest wrote:
> (not the test though)

I'll commit the test then.

Thanks

[arv (Not doing code reviews), 2015-06-11 20:58:11]

The CQ bit was checked by arv@chromium.org

[arv (Not doing code reviews), 2015-06-11 20:58:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
dslomov@chromium.org, verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1161073002/#ps40001
(title: "Only test left")

[commit-bot: I haz the power, 2015-06-11 20:58:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1161073002/40001

[commit-bot: I haz the power, 2015-06-11 21:24:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-11 21:24:53]

Patchset 3 (id:??) landed as
https://crrev.com/5f72593d166e1d604d6061573bd220da5ce64db4
Cr-Commit-Position: refs/heads/master@{#28971}