Last Comment Bug 604781 - Watchpoints set on setters do not always trigger; and, zombie setters!
: Watchpoints set on setters do not always trigger; and, zombie setters!
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: mozilla13
Assigned To: Jason Orendorff [:jorendorff]
:
Mentors:
Depends on: 637985
Blocks: 609924
  Show dependency treegraph
 
Reported: 2010-10-15 14:34 PDT by Jim Blandy :jimb
Modified: 2012-03-02 06:20 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case. (513 bytes, application/javascript)
2010-10-15 14:38 PDT, Jim Blandy :jimb
no flags Details
v1 (4.29 KB, patch)
2011-04-21 15:56 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Review
v2 (3.93 KB, patch)
2012-02-08 14:15 PST, Jason Orendorff [:jorendorff]
jimb: review+
Details | Diff | Review

Description Jim Blandy :jimb 2010-10-15 14:34:33 PDT
Depending on the details of how the property is defined, watchpoints on setter properties may not trigger. In particular, a watchpoint set on p.x does trigger:

var p = { set x(v) { setter(v); } };

But a watchpoint set on o.x does not:

var o = Object.defineProperty({}, 'x', { set:setter, enumerable:true, configurable:true });

The two should be identical, except for the specific setter function.
Comment 1 Jim Blandy :jimb 2010-10-15 14:38:29 PDT
Created attachment 483622 [details]
Test case.
Comment 2 Jim Blandy :jimb 2010-10-15 14:48:36 PDT
<jorendorff> jimb: In the second case, the property so defined is
             JSPROP_READONLY  [14:45]
<jorendorff> jimb: which i think is nonsense, and we rightly ignore it in most
             places, but perhaps not in the watch_setter
Comment 3 Jim Blandy :jimb 2010-10-15 16:26:29 PDT
Here's another lovely bit of behavior:

function watcher(id,old,newval) { print("assignment watched"); return newval; }
var o = { set x(v) { print("setting x"); } };
o.watch('x', watcher);
Object.defineProperty(o, 'x', { value:3,
                                writable:true,
                                enumerable:true,
                                configurable:true });
o.x = 3;

Both the watchpoint and the setter run!
Comment 4 Jason Orendorff [:jorendorff] 2011-04-21 15:56:20 PDT
Created attachment 527673 [details] [diff] [review]
v1

This fix applies on top of bug 637985. Will request review once that one is ready.
Comment 5 Jason Orendorff [:jorendorff] 2012-02-08 14:15:24 PST
Created attachment 595532 [details] [diff] [review]
v2

Unbitrotted. Let's get this in.
Comment 6 Jason Orendorff [:jorendorff] 2012-02-08 14:22:54 PST
Incidentally the tests pass even without the change to obj_watch, I think because that property is not JSPROP_READONLY anymore; but there is no point having a line of code in obj_watch that doesn't do anything.
Comment 7 Jason Orendorff [:jorendorff] 2012-03-01 07:07:03 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/502b21011c3e
Comment 8 Marco Bonardo [::mak] 2012-03-02 06:20:14 PST
https://hg.mozilla.org/mozilla-central/rev/502b21011c3e

Note You need to log in before you can comment on or make changes to this bug.