Yeah I think so. I'm actually kinda surprised that the getters are implemented in this way at this point. I'd update all those methods to use the Holder() they get from the info. In GetPropertyWithAccessor you can see that the holder passed to the PropertyCallbackArguments is the holder of the accessors. I'm pretty sure that's the object from which you want to actually fetch the value. That should fix the get semantics at least.
My plan is to, in the future, make ExecutableAccessInfo behave entirely like a data property which just so happens to be implemented using accessors internally. That will affect the store behavior. I can't do it yet, since I'm waiting for dom accessors to be turned into real JS accessors; until then it would break things like window.all. That will fix the ExecutableAccessorInfo setter problem you are having with unscopable. hth, Toon On Wed, Jul 23, 2014 at 4:12 PM, Erik Arvidsson <[email protected]> wrote: > > > > On Wed, Jul 23, 2014 at 8:08 AM, <[email protected]> wrote: > >> My comment below is based on a short explanation of the problem by >> rossberg. Let >> me know if some things are still unclear. >> >> Overall: Please don't work around "bugs" (wasn't really a bug until >> unscopable >> since it wasn't observable in any way) in one part of the system by >> obfuscating >> another part. >> >> >> https://codereview.chromium.org/384963002/diff/120001/src/objects.cc >> File src/objects.cc (right): >> >> https://codereview.chromium.org/384963002/diff/120001/src/ >> objects.cc#newcode165 >> src/objects.cc:165: } >> Euh, no. >> >> What you want to do is fix a bug in Array.length; but what you do >> instead is modify get-property to change the semantics of all API >> accessors... >> >> The right fix is to change ArrayLengthGetter to not use This() (via >> GetThisFrom), but rather read the value from the Holder(). That also >> removes the need for FindInstanceOf. > > > I'm happy to hear that you do not think this work around is the right > approach. > > I think I might still be a bit confused. > > I was under the impression that this was not a one off bug but a > conceptual problem with API accessors. If that is not the case I'm happy to > comment out the failing tests and then fix the one off bugs > (ArrayLengthGetter and others). > > I thought the issue was that `holder[[Get]](name, receiver)` for an API > accessor always get the actual value from the receiver? Looking at > accessors.cc, it seems that almost all of these callbacks use `info.This()` > so all of them should be updated? > > > > > >> >> >> https://codereview.chromium.org/384963002/ >> >> -- >> -- >> v8-dev mailing list >> [email protected] >> http://groups.google.com/group/v8-dev >> ---You received this message because you are subscribed to the Google >> Groups "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> For more options, visit https://groups.google.com/d/optout. >> > > > > -- > erik > > > -- > -- > v8-dev mailing list > [email protected] > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to the Google Groups > "v8-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
