Subj : Class-specific getters/setters getting lost
To   : netscape.public.mozilla.jseng
From : "Steven C. Cole" <scole@planetweb.com>
Date : Wed Apr 23 2003 01:03 pm

This is a multi-part message in MIME format.
--------------040505080901050400020506
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Something has changed recently in the tip of the Spidermonkey source, 
and now class-specific properties are getting somehow "lost".

When I make a JSClass that has only one property, with a 
property-specific getter and setter pair, the property itself gets 
overridden the first time it's set.

In other words: a generic class:

static JSClass BusyIcon_Class =
{ "BusyIcon", 0,
   JS_PropertyStub,  JS_PropertyStub, JS_PropertyStub, JS_PropertyStub,
   JS_EnumerateStub, JS_ResolveStub,  JS_ConvertStub,  JS_FinalizeStub,
   JSCLASS_NO_OPTIONAL_MEMBERS
};

And one simple property:

static JSPropertySpec BusyIcon_Properties[] =
{
     { "x", ID_BusyIcon_x, JSPROP_ENUMERATE, BusyIcon_GetXPos, 
BusyIcon_SetXPos },
     { 0 }
};

When something gets this 'x' property, everything works fine.  But when 
someone sets the 'x' property, things are turned off.  After 
instrumenting the add and get callbacks, it looks like a new property is 
being created ('x') and stored in the object, NOT using the tinyid. 
This new property is then just a standard JS prop and doesn't cause the 
getter/setter callbacks to be used anymore.

I'll attach a diff to js.c at the end of this message that allows the 
test to be compiled.

A simple script log looks like (my getters and setters are instrumented):

js> busyIcon
JS: Adding new BusyIcon object to 002F8B40 (as "busyIcon").
[object BusyIcon]
js> busyIcon.x
JS: BusyIcon_GetXPos --> 0
0
js> busyIcon.x = 3;
3
js> busyIcon.x
3
js>

Notice that the getter is called the first time, but not the second 
time.  The setter is never called.

There is a workaround:  Setting JSPROP_SHARED on the properties makes 
everything work just fine.  And it's arguably the right thing to do even 
if this bug is fixed.  Still.  What's going on here shouldn't happen, I 
think.

--Steve Cole

--------------040505080901050400020506
Content-Type: text/plain;
 name="js.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="js.patch"

Index: js.c
===================================================================
RCS file: /export/home/cvs/external/mozilla/js/src/js.c,v
retrieving revision 1.1.1.24
diff -u -r1.1.1.24 js.c
--- js.c	2003/04/07 17:17:11	1.1.1.24
+++ js.c	2003/04/23 19:02:03
@@ -1913,6 +1913,9 @@
 #endif
 }
 
+static JSBool pw_js_BusyIconCreate(JSContext *cx, JSObject *parent,
+                                   const char *name, uintN attrs);
+
 static JSBool
 global_resolve(JSContext *cx, JSObject *obj, jsval id, uintN flags,
                JSObject **objp)
@@ -1930,6 +1933,14 @@
     }
 #endif
 
+    if (JSVAL_IS_STRING (id) && strcmp(JS_GetStringBytes(JSVAL_TO_STRING(id)), "busyIcon") == 0)
+    {
+        if (!pw_js_BusyIconCreate(cx, obj, "busyIcon", JSPROP_ENUMERATE))
+            return JS_FALSE;
+        *objp = obj;
+        return JS_TRUE;
+    }
+
 #if defined(SHELL_HACK) && defined(DEBUG) && defined(XP_UNIX)
     if ((flags & (JSRESOLVE_QUALIFIED | JSRESOLVE_ASSIGNING)) == 0) {
         /*
@@ -2149,4 +2160,80 @@
     JS_DestroyRuntime(rt);
     JS_ShutDown();
     return result;
+}
+
+
+enum BusyIcon_IDs {
+    ID_BusyIcon_x       = -1,
+};
+
+static JSBool BusyIcon_GetXPos(JSContext *cx, JSObject *obj, jsval id,
+                                  jsval *vp);
+static JSBool BusyIcon_SetXPos(JSContext *cx, JSObject *obj, jsval id,
+                                  jsval *vp);
+
+static JSClass BusyIcon_Class =
+{
+    "BusyIcon",                         /* name */
+    0,                                  /* flags */
+    JS_PropertyStub,                    /* addProperty */
+    JS_PropertyStub,                    /* delProperty */
+    JS_PropertyStub,                    /* getProperty */
+    JS_PropertyStub,                    /* setProperty */
+    JS_EnumerateStub,                   /* enumerate */
+    JS_ResolveStub,                     /* resolve */
+    JS_ConvertStub,                     /* convert */
+    JS_FinalizeStub,                    /* finalize */
+    JSCLASS_NO_OPTIONAL_MEMBERS
+};
+
+int32 bi_x;
+
+#define PW_GetBusyIconX() bi_x
+#define PW_SetBusyIconX(x) (bi_x=(x))
+
+static JSPropertySpec BusyIcon_Properties[] =
+{
+    { "x",       ID_BusyIcon_x,       JSPROP_ENUMERATE, BusyIcon_GetXPos,       BusyIcon_SetXPos       },
+    { 0 }
+}; 
+
+static JSBool pw_js_BusyIconCreate(JSContext *cx, JSObject *parent,
+                                   const char *name, uintN attrs)
+{
+    JSObject *proto;
+
+    fprintf(gOutFile, "JS: Adding new BusyIcon object to %p (as \"%s\").\n",
+            parent, name);
+
+    proto = JS_InitClass(cx, parent, NULL, &BusyIcon_Class, NULL, 0,
+                         BusyIcon_Properties, NULL, NULL, NULL);
+    if (proto == NULL)
+        return JS_FALSE;
+
+    if (NULL == JS_DefineObject(cx, parent, name, &BusyIcon_Class, proto, attrs))
+        return JS_FALSE;
+
+    return JS_TRUE;
+}
+
+static JSBool BusyIcon_GetXPos(JSContext *cx, JSObject *obj, jsval id,
+                                  jsval *vp)
+{
+    *vp = INT_TO_JSVAL(PW_GetBusyIconX());
+    fprintf(gOutFile, "JS: BusyIcon_GetXPos --> %s\n", JS_GetStringBytes(JS_ValueToString(cx, *vp)));
+    return JS_TRUE;
+}
+
+static JSBool BusyIcon_SetXPos(JSContext *cx, JSObject *obj, jsval id,
+                                  jsval *vp)
+{
+    int32 x;
+
+    fprintf(gOutFile, "JS: BusyIcon_SetXPos(%s)\n", JS_GetStringBytes(JS_ValueToString(cx, *vp)));
+
+    if (!JS_ValueToECMAInt32(cx, *vp, &x))
+        return JS_FALSE;
+    PW_SetBusyIconX(x);
+    return JS_TRUE;
 }

--------------040505080901050400020506--

.