Fix longstanding bug where watchpoints didn't work with JSPROP_SETTER (127243, r=rginda, sr=shaver, a=dbaron).

js/src/jsdbgapi.c

@@ -232,6 +232,7 @@ JS_ClearInterrupt(JSRuntime *rt, JSTrapHandler *handlerp, void **closurep)
     return JS_TRUE;
 }
 
+/************************************************************************/
 
 typedef struct JSWatchPoint {
     JSCList             links;
@@ -271,6 +272,18 @@ DropWatchPoint(JSContext *cx, JSWatchPoint *wp)
     return JS_TRUE;
 }
 
+void
+js_MarkWatchPoints(JSRuntime *rt)
+{
+    JSWatchPoint *wp;
+
+    for (wp = (JSWatchPoint *)rt->watchPointList.next;
+         wp != (JSWatchPoint *)&rt->watchPointList;
+         wp = (JSWatchPoint *)wp->links.next) {
+        MARK_SCOPE_PROPERTY(wp->sprop);
+    }
+}
+
 static JSWatchPoint *
 FindWatchPoint(JSRuntime *rt, JSScope *scope, jsid id)
 {
@@ -352,7 +365,11 @@ js_watch_set(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
                 frame.fun = fun;
                 frame.down = cx->fp;
                 cx->fp = &frame;
-                ok = !wp->setter || wp->setter(cx, obj, id, vp);
+                ok = !wp->setter ||
+                     ((sprop->attrs & JSPROP_SETTER)
+                      ? js_InternalCall(cx, obj, OBJECT_TO_JSVAL(wp->setter),
+                                        1, vp, vp)
+                      : wp->setter(cx, OBJ_THIS_OBJECT(cx, obj), userid, vp));
                 cx->fp = frame.down;
             }
             return DropWatchPoint(cx, wp);
@@ -362,6 +379,45 @@ js_watch_set(JSContext *cx, JSObject *obj, jsval id, jsval *vp)
     return JS_FALSE;
 }
 
+JSBool JS_DLL_CALLBACK
+js_watch_set_wrapper(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
+                     jsval *rval)
+{
+    JSObject *funobj;
+    JSFunction *wrapper;
+    jsval userid;
+
+    funobj = JSVAL_TO_OBJECT(argv[-2]);
+    wrapper = (JSFunction *) JS_GetPrivate(cx, funobj);
+    userid = ATOM_KEY(wrapper->atom);
+    *rval = argv[0];
+    return js_watch_set(cx, obj, userid, rval);
+}
+
+extern JSPropertyOp
+js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, JSPropertyOp setter)
+{
+    JSAtom *atom;
+    JSFunction *wrapper;
+
+    if (!(attrs & JSPROP_SETTER))
+        return js_watch_set;
+
+    if (!JSVAL_IS_INT(id)) {
+        atom = (JSAtom *)id;
+    } else {
+        atom = js_AtomizeInt(cx, JSVAL_TO_INT(id), 0);
+        if (!atom)
+            return NULL;
+    }
+    wrapper = js_NewFunction(cx, NULL, js_watch_set_wrapper, 1, 0,
+                             OBJ_GET_PARENT(cx, (JSObject *)setter),
+                             atom);
+    if (!wrapper)
+        return NULL;
+    return (JSPropertyOp) wrapper->object;
+}
+
 JS_PUBLIC_API(JSBool)
 JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
                  JSWatchPointHandler handler, void *closure)
@@ -372,6 +428,7 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
     JSScopeProperty *sprop;
     JSRuntime *rt;
     JSWatchPoint *wp;
+    JSPropertyOp watcher;
 
     if (!OBJ_IS_NATIVE(obj)) {
         JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, JSMSG_CANT_WATCH,
@@ -436,6 +493,10 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
 
     wp = FindWatchPoint(rt, OBJ_SCOPE(obj), propid);
     if (!wp) {
+        watcher = js_WrapWatchedSetter(cx, propid, sprop->attrs, sprop->setter);
+        if (!watcher)
+            return JS_FALSE;
+
         wp = (JSWatchPoint *) JS_malloc(cx, sizeof *wp);
         if (!wp)
             return JS_FALSE;
@@ -452,7 +513,7 @@ JS_SetWatchPoint(JSContext *cx, JSObject *obj, jsval id,
         wp->setter = sprop->setter;
         wp->nrefs = 1;
         sprop = js_ChangeNativePropertyAttrs(cx, obj, sprop, 0, sprop->attrs,
-                                             sprop->getter, js_watch_set);
+                                             sprop->getter, watcher);
         if (!sprop)
             return DropWatchPoint(cx, wp);
     }
@@ -522,6 +583,8 @@ JS_ClearAllWatchPoints(JSContext *cx)
     return JS_TRUE;
 }
 
+/************************************************************************/
+
 JS_PUBLIC_API(uintN)
 JS_PCToLineNumber(JSContext *cx, JSScript *script, jsbytecode *pc)
 {
@@ -546,6 +609,7 @@ JS_GetScriptPrincipals(JSContext *cx, JSScript *script)
     return script->principals;
 }
 
+/************************************************************************/
 
 /*
  *  Stack Frame Iterator

js/src/jsdbgapi.h

@@ -93,6 +93,9 @@ JS_ClearAllWatchPoints(JSContext *cx);
  * Hide these non-API function prototypes by testing whether the internal
  * header file "jsconfig.h" has been included.
  */
+extern void
+js_MarkWatchPoints(JSRuntime *rt);
+
 extern JSScopeProperty *
 js_FindWatchPoint(JSRuntime *rt, JSScope *scope, jsid id);
 
@@ -102,7 +105,15 @@ js_GetWatchedSetter(JSRuntime *rt, JSScope *scope,
 
 extern JSBool JS_DLL_CALLBACK
 js_watch_set(JSContext *cx, JSObject *obj, jsval id, jsval *vp);
-#endif
+
+extern JSBool JS_DLL_CALLBACK
+js_watch_set_wrapper(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
+                     jsval *rval);
+
+extern JSPropertyOp
+js_WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, JSPropertyOp setter);
+
+#endif /* JS_HAS_OBJ_WATCHPOINT */
 
 /************************************************************************/
 

js/src/jsobj.c

@@ -61,9 +61,7 @@
 #include "jsstr.h"
 #include "jsopcode.h"
 
-#if JS_HAS_OBJ_WATCHPOINT
-#include "jsdbgapi.h"
-#endif
+#include "jsdbgapi.h"   /* whether or not JS_HAS_OBJ_WATCHPOINT */
 
 #ifdef JS_THREADSAFE
 #define NATIVE_DROP_PROPERTY js_DropProperty
@@ -1783,10 +1781,8 @@ js_FinalizeObject(JSContext *cx, JSObject *obj)
     if (cx->runtime->objectHook)
         cx->runtime->objectHook(cx, obj, JS_FALSE, cx->runtime->objectHookData);
 
-#if JS_HAS_OBJ_WATCHPOINT
     /* Remove all watchpoints with weak links to obj. */
     JS_ClearWatchPointsForObject(cx, obj);
-#endif
 
     /*
      * Finalize obj first, in case it needs map and slots.  Optimized to use
@@ -2590,9 +2586,15 @@ js_SetProperty(JSContext *cx, JSObject *obj, jsid id, jsval *vp)
             shortid = sprop->shortid;
             getter = sprop->getter;
             setter = sprop->setter;
-            if (setter == js_watch_set)
-                setter = js_GetWatchedSetter(rt, scope, sprop);
             JS_UNLOCK_SCOPE(cx, scope);
+
+            /* Recover watched setter *after* releasing scope's lock. */
+            if ((attrs & JSPROP_SETTER)
+                ? ((JSFunction *)JS_GetPrivate(cx, (JSObject *)setter))->native
+                  == js_watch_set_wrapper
+                : setter == js_watch_set) {
+                setter = js_GetWatchedSetter(rt, scope, sprop);
+            }
             sprop = NULL;
         }
 #ifdef __GNUC__         /* suppress bogus gcc warnings */

js/src/jsscope.c

@@ -1086,7 +1086,9 @@ js_AddScopeProperty(JSContext *cx, JSScope *scope, jsid id,
          */
         if (!JS_CLIST_IS_EMPTY(&cx->runtime->watchPointList) &&
             js_FindWatchPoint(cx->runtime, scope, id)) {
-            setter = js_watch_set;
+            setter = js_WrapWatchedSetter(cx, id, attrs, setter);
+            if (!setter)
+                goto fail_overwrite;
         }
 
         /* Find or create a property tree node labeled by our arguments. */
@@ -1424,6 +1426,9 @@ js_SweepScopeProperties(JSRuntime *rt)
     memset(js_nkids_hist, 0, sizeof js_nkids_hist);
 #endif
 
+    /* Mark watched scope properties hidden in the runtime before we sweep. */
+    js_MarkWatchPoints(rt);
+
     ap = &rt->propertyArenaPool.first.next;
     while ((a = *ap) != NULL) {
         limit = (JSScopeProperty *) a->avail;