542864 - with js -z, getters don't resolve correctly against the split global object

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steven Parkes]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 GTB7.0
Build Identifier: 

Gettters don't seem to resolve correctly against the global split object in the shell.

Exceuting this code in the shell works fine with a normal global and fails in the second lookup with a split global:

  this.__defineGetter__("foo",function(){return"fooy";});
  a = this.foo;
  b = foo;


Reproducible: Always

Comment 1

[Steven Parkes]

This patch works. I think it might not be quite right; there are subtitles here I'm not 100% on. But I think it might be close, so:

http://gist.github.com/298781

I'm not sure about the setting of the global object part of the diff. I mostly left it in to remind me to ask; I haven't seen anywhere where it makes a difference, but the tests I've been running don't rebind outer. My thoughts were based upon 

http://groups.google.com/group/mozilla.dev.tech.js-engine/msg/df81825b338fb84f

the wording "Every web page has a global object. This is an inner window, and has
the same lifetime as the page" in particular. But I'm not sure it matters. I'm not sure I see any place the global object is accessible since inner overrides the scope "this" to point to outer.

I think. This stuff makes my head hurt.

Here's the test I use

http://gist.github.com/298789

Comment 2

[Steven Parkes]

You know, I thought this looked strange (the asId stuff). Pretty sure the

- return JS_DefinePropertyById(cx, cpx->inner, asId, *vp, NULL, NULL, JSPROP_ENUMERATE);

In the original should have been

- return JS_DefinePropertyById(cx, cpx->inner, id, asId, NULL, NULL, JSPROP_ENUMERATE);

even without the getter/setter support. Now I know why all my values where showing up as keys ...

So the current version for me is 

static JSBool
split_addProperty(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
{
    ComplexObject *cpx;
    jsid asId;

    cpx = split_get_private(cx, obj);

    if (!cpx)
        return JS_TRUE;
    if (!cpx->isInner && cpx->inner) {
        /* Make sure to define this property on the inner object. */

        /* Patched. See https://bugzilla.mozilla.org/show_bug.cgi?id=542864 */

        JSPropertyOp setter = 0;
        JSPropertyOp getter = 0;
        uintN attrs = 0;
        JSBool found;

        JS_GetPropertyAttrsGetterAndSetterById(cx, obj, id, &attrs, &found, &getter, &setter);

        if (!JS_ValueToId(cx, *vp, &asId)) {
            return JS_FALSE;
        }
        
        return JS_DefinePropertyById(cx, cpx->inner, id, asId, getter, setter, attrs | JSPROP_ENUMERATE);
    }
    return JS_TRUE;
}

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Real fix

You were close, but this is the fix. A really stupid typo if I've ever made one.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/7af412b6d23f

Comment 5

[Steven Parkes]

Ah. Right. I see that.

But that doesn't seem to address the actual root issue regarding getters/setters, does it? Did I miss something?

This http://gist.github.com/298789 still seems to generate the wrong result w/o the JS_GetPropertyAttrsGetterAndSetterById?

Comment 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/7af412b6d23f

Comment 7

[Steven Parkes]

The test in the original description does seem to work against tip, but the more extensive case that I put in later (at http://gist.github.com/298789) still seems to fail with -z (but works w/o -z). It shouldn't print anything.

I could reopen this.
Or open a new bug.
Or go away. :)