Confusion on changing data property callback attributes

Changing the attributes of a data property implemented with
ExecutableAccessorInfo turns the property into a field. Better
to keep it as a callback, and correctly deal with the changed
property attributes.

R=ulan@chromium.org

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

[mvstanton, 2014-05-07 13:19:14]

Hi Michael (and Toon if you want to have a look),
Here is the fix. As discussed, I would like to move this under 
JSObject::SetLocalPropertyIgnoreAttributes, rather than keeping the special case
in the runtime function. But I'm inclined to do that as a follow-up.
Thanks,
--Michael

[Michael Starzinger, 2014-05-07 18:46:03]

This is very brittle and adds way to many special cases in the code path that it
is actually trying to fix. We need to find a cleaner solution for this. Let's
discuss this offline.

[mvstanton, 2014-05-09 09:47:19]

Thx Michael, here is a version which integrates setting callbacks into
SetLocalPropertyIgnoreAttributes. Thx for the look.
--Michael

[mvstanton, 2014-05-09 13:59:16]

Adding Ulan, who is working in a similar area.

[mvstanton, 2014-05-23 09:42:58]

one kine ping...

[ulan, 2014-05-23 10:14:47]

Looks good. I didn't understand one part with is_observed:

https://codereview.chromium.org/262053011/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/262053011/diff/80001/src/objects.cc#newcode4438
src/objects.cc:4438: } else if (is_observed) {
Shouldn't we check is_observed independent of (attributes !=
lookup.GetAttributes()) ?

[mvstanton, 2014-05-27 13:31:24]

Hi Ulan, thanks for the look. Comments addressed,
--Michael

https://codereview.chromium.org/262053011/diff/80001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/262053011/diff/80001/src/objects.cc#newcode4438
src/objects.cc:4438: } else if (is_observed) {
On 2014/05/23 10:14:48, ulan wrote:
> Shouldn't we check is_observed independent of (attributes !=
> lookup.GetAttributes()) ?

Good catch. And I renamed the variable from dont_enqueue_records to
executed_set_prototype when is less awkward. Basically setting the prototype is
the one exception for this "observed" infrastructure.

[ulan, 2014-05-28 09:02:30]

LGTM, thanks for fixing!

[mvstanton, 2014-05-28 09:58:38]

Committed patchset #6 manually as r21558 (presubmit successful).