From 315c0f03b40eba8231a99406e0cfc6622d741be2 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Tue, 8 Feb 2011 15:45:12 -0600 Subject: [PATCH] Bug 631723 - Make js_UpdateWatchpointsForShape return the new shape (and fix a few coding style nits). r=jimb. --HG-- extra : rebase_source : c09fac13e674d0317f4edd7d64e5938c68c0e28a --- js/src/jit-test/tests/jaeger/bug630366.js | 7 +++++ js/src/jsdbgapi.cpp | 24 +++++++-------- js/src/jsdbgapiinlines.h | 8 ++--- js/src/jsscope.cpp | 29 +++++++++---------- js/src/tests/js1_8_5/extensions/jstests.list | 1 + .../js1_8_5/extensions/regress-631723.js | 10 +++++++ 6 files changed, 48 insertions(+), 31 deletions(-) create mode 100644 js/src/jit-test/tests/jaeger/bug630366.js create mode 100644 js/src/tests/js1_8_5/extensions/regress-631723.js diff --git a/js/src/jit-test/tests/jaeger/bug630366.js b/js/src/jit-test/tests/jaeger/bug630366.js new file mode 100644 index 00000000000..acac8d3ef4c --- /dev/null +++ b/js/src/jit-test/tests/jaeger/bug630366.js @@ -0,0 +1,7 @@ +var o = {}; +for(var i=0; i<5; i++) { + o.p = 2; + o.watch("p", function() { }); + o.p = 2; + delete o.p; +} diff --git a/js/src/jsdbgapi.cpp b/js/src/jsdbgapi.cpp index ae3cd590772..02d79c0c388 100644 --- a/js/src/jsdbgapi.cpp +++ b/js/src/jsdbgapi.cpp @@ -825,23 +825,23 @@ WrapWatchedSetter(JSContext *cx, jsid id, uintN attrs, StrictPropertyOp setter) return CastAsStrictPropertyOp(FUN_OBJECT(wrapper)); } -static bool -UpdateWatchpointShape(JSContext *cx, JSWatchPoint *wp, const js::Shape *newShape) +static const Shape * +UpdateWatchpointShape(JSContext *cx, JSWatchPoint *wp, const Shape *newShape) { JS_ASSERT_IF(wp->shape, wp->shape->id == newShape->id); JS_ASSERT(!IsWatchedProperty(cx, newShape)); /* Create a watching setter we can substitute for the new shape's setter. */ - js::StrictPropertyOp watchingSetter = + StrictPropertyOp watchingSetter = WrapWatchedSetter(cx, newShape->id, newShape->attributes(), newShape->setter()); if (!watchingSetter) - return false; + return NULL; /* * Save the shape's setter; we don't know whether js_ChangeNativePropertyAttrs will * return a new shape, or mutate this one. */ - js::StrictPropertyOp originalSetter = newShape->setter(); + StrictPropertyOp originalSetter = newShape->setter(); /* * Drop the watching setter into the object, in place of newShape. Note that a single @@ -849,21 +849,21 @@ UpdateWatchpointShape(JSContext *cx, JSWatchPoint *wp, const js::Shape *newShape * wrap all (JSPropertyOp, not JSObject *) setters with js_watch_set, so shapes that * differ only in their setter may all get wrapped to the same shape. */ - const js::Shape *watchingShape = + const Shape *watchingShape = js_ChangeNativePropertyAttrs(cx, wp->object, newShape, 0, newShape->attributes(), newShape->getter(), watchingSetter); if (!watchingShape) - return false; + return NULL; /* Update the watchpoint with the new shape and its original setter. */ wp->setter = originalSetter; wp->shape = watchingShape; - return true; + return watchingShape; } -bool -js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape) +const Shape * +js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const Shape *newShape) { /* * The watchpoint code uses the normal property-modification functions to install its @@ -873,11 +873,11 @@ js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Sha * proceed without interference. */ if (IsWatchedProperty(cx, newShape)) - return true; + return newShape; JSWatchPoint *wp = FindWatchPoint(cx->runtime, obj, newShape->id); if (!wp) - return true; + return newShape; return UpdateWatchpointShape(cx, wp, newShape); } diff --git a/js/src/jsdbgapiinlines.h b/js/src/jsdbgapiinlines.h index fc1a08ff5a1..b469ee2fd29 100644 --- a/js/src/jsdbgapiinlines.h +++ b/js/src/jsdbgapiinlines.h @@ -46,19 +46,19 @@ #if defined(JS_HAS_OBJ_WATCHPOINT) && defined(__cplusplus) -extern bool +extern const js::Shape * js_SlowPathUpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape); /* - * Update any watchpoints on |obj| on |new_shape->id| to use |new_shape|. Property-manipulating + * Update any watchpoints on |obj| on |newShape->id| to use |newShape|. Property-manipulating * functions must call this any time it takes on a new shape to represent a potentially * watched property, or when it mutates a shape's attributes/setter/getter. */ -static inline bool +static inline const js::Shape * js_UpdateWatchpointsForShape(JSContext *cx, JSObject *obj, const js::Shape *newShape) { if (JS_CLIST_IS_EMPTY(&cx->runtime->watchPointList)) - return true; + return newShape; return js_SlowPathUpdateWatchpointsForShape(cx, obj, newShape); } diff --git a/js/src/jsscope.cpp b/js/src/jsscope.cpp index b67b20603f1..b829f1b01cd 100644 --- a/js/src/jsscope.cpp +++ b/js/src/jsscope.cpp @@ -768,10 +768,9 @@ JSObject::addProperty(JSContext *cx, jsid id, return NULL; /* Update any watchpoints referring to this property. */ - if (!js_UpdateWatchpointsForShape(cx, this, shape)) { + shape = js_UpdateWatchpointsForShape(cx, this, shape); + if (!shape) METER(wrapWatchFails); - return NULL; - } return shape; } @@ -896,13 +895,14 @@ JSObject::putProperty(JSContext *cx, jsid id, return NULL; } - const Shape *new_shape = + const Shape *newShape = addPropertyInternal(cx, id, getter, setter, slot, attrs, flags, shortid, spp); - if (!js_UpdateWatchpointsForShape(cx, this, new_shape)) { - METER(wrapWatchFails); + if (!newShape) return NULL; - } - return new_shape; + newShape = js_UpdateWatchpointsForShape(cx, this, newShape); + if (!newShape) + METER(wrapWatchFails); + return newShape; } /* Property exists: search must have returned a valid *spp. */ @@ -1038,12 +1038,10 @@ JSObject::putProperty(JSContext *cx, jsid id, CHECK_SHAPE_CONSISTENCY(this); METER(puts); - if (!js_UpdateWatchpointsForShape(cx, this, shape)) { + const Shape *newShape = js_UpdateWatchpointsForShape(cx, this, shape); + if (!newShape) METER(wrapWatchFails); - return NULL; - } - - return shape; + return newShape; } const Shape * @@ -1109,11 +1107,12 @@ JSObject::changeProperty(JSContext *cx, const Shape *shape, uintN attrs, uintN m lastProp->shape = js_GenerateShape(cx); clearOwnShape(); - if (!js_UpdateWatchpointsForShape(cx, this, shape)) { + shape = js_UpdateWatchpointsForShape(cx, this, shape); + if (!shape) { METER(wrapWatchFails); return NULL; } - + JS_ASSERT(shape == mutableShape); newShape = mutableShape; } else if (shape == lastProp) { Shape child(shape->id, getter, setter, shape->slot, attrs, shape->flags, shape->shortid); diff --git a/js/src/tests/js1_8_5/extensions/jstests.list b/js/src/tests/js1_8_5/extensions/jstests.list index 5631f523d9b..92d81bddcfd 100644 --- a/js/src/tests/js1_8_5/extensions/jstests.list +++ b/js/src/tests/js1_8_5/extensions/jstests.list @@ -25,3 +25,4 @@ skip-if(!xulRuntime.shell) script clone-errors.js skip-if(!xulRuntime.shell) script clone-forge.js script set-property-non-extensible.js script recursion.js +script regress-631723.js diff --git a/js/src/tests/js1_8_5/extensions/regress-631723.js b/js/src/tests/js1_8_5/extensions/regress-631723.js new file mode 100644 index 00000000000..f7c755603cc --- /dev/null +++ b/js/src/tests/js1_8_5/extensions/regress-631723.js @@ -0,0 +1,10 @@ +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/licenses/publicdomain/ + +var o = {a:1, b:2}; +o.watch("p", function() { return 13; }); +delete o.p; +o.p = 0; +assertEq(o.p, 13); + +reportCompare(0, 0, 'ok');