[api] Throw in defineProperty() when necessary.

[api] Throw in defineProperty() when necessary.

According to http://heycam.github.io/webidl/#defineownproperty,
we must throw when the object has a setter callback and we call
defineProperty with a non data descriptor.


BUG=chromium:645542

[Franzi, 2016-09-13 16:49:41]

The CQ bit was checked by franzih@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-13 16:49:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 17:18:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-13 17:18:50]

Dry run: This issue passed the CQ dry run.

[Franzi, 2016-09-13 17:43:18]

franzih@chromium.org changed reviewers:
+ adamk@chromium.org, jochen@chromium.org

[Franzi, 2016-09-13 17:43:19]

Hi, 

Here's a fix for a Chromium issue related to defineProperty(). Can you please
have a look? 

Thanks, 

Franzi

[adamk, 2016-09-13 19:22:02]

This seems reasonable enough, but from my reading of the attached bug and some
experiments it doesn't seem like any browser currently implements these
semantics. Intuitively I don't imagine this is likely to be a major compat
issue, but it's something to keep in mind. I'll defer to Jochen for the API code
itself.

[jochen (gone - plz use gerrit), 2016-09-14 08:43:01]

https://codereview.chromium.org/2331223004/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2331223004/diff/60001/src/objects.cc#newcode1688
src/objects.cc:1688: // Throw if setter callback is defined and define property
is called with a
how about only doing this if we don't have a definer?

https://codereview.chromium.org/2331223004/diff/60001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/ForOf.golden (left):

https://codereview.chromium.org/2331223004/diff/60001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/ForOf.golden:89: B(Wide),
B(LdaSmi), U16(129),
why did this change?

[Franzi, 2016-09-14 16:19:51]

Thinking about this some more, this change will break the Node.js interactive
shell (REPL) every time defineProperty() with an accessor descriptor is called.
Because REPL (or rather the vm module) defines NamedSetterCallbacks on the
global object. 

I'll look into how hard it is to change the specification, especially since
Firefox and Safari currently expose the same behavior as Chrome.

https://codereview.chromium.org/2331223004/diff/60001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/2331223004/diff/60001/src/objects.cc#newcode1688
src/objects.cc:1688: // Throw if setter callback is defined and define property
is called with a
On 2016/09/14 at 08:43:01, jochen wrote:
> how about only doing this if we don't have a definer?

If I understand the IDL specification correctly, we're supposed to throw before
we call 
OrdinaryDefineOwnProperty(), in which we check for a definer. Or is that just an
implementation detail
and the definer should intercept before anything else happens?

https://codereview.chromium.org/2331223004/diff/60001/test/cctest/interpreter...
File test/cctest/interpreter/bytecode_expectations/ForOf.golden (left):

https://codereview.chromium.org/2331223004/diff/60001/test/cctest/interpreter...
test/cctest/interpreter/bytecode_expectations/ForOf.golden:89: B(Wide),
B(LdaSmi), U16(129),
On 2016/09/14 at 08:43:01, jochen wrote:
> why did this change?

Because we added a new error message.

[adamk, 2016-09-15 00:18:47]

I like the idea of pushing back on the spec here, and is a good contact for
that.

[adamk, 2016-09-15 00:19:10]

On 2016/09/15 00:18:47, adamk wrote:
> I like the idea of pushing back on the spec here, and is a good contact for
> that.

That was supposed to read "...and bzbarsky is a good contact for that".

[Franzi, 2017-02-26 08:16:12]

Description was changed from

==========
[api] Throw in defineProperty() when necessary.

According to http://heycam.github.io/webidl/#defineownproperty,
we must throw when the object has a setter callback and we call
defineProperty with a non data descriptor.


BUG=chromium:645542
==========

to

==========
[api] Throw in defineProperty() when necessary.

According to http://heycam.github.io/webidl/#defineownproperty,
we must throw when the object has a setter callback and we call
defineProperty with a non data descriptor.


BUG=chromium:645542
==========