STR: 1) Visit http://harthur.github.com/wwcode/ 2) Set a breakpoint at wwcode.js:36 3) Reload and wait for the breakpoint to hit 4) Resume The terminal says: TypeError: breakpoints[aLocation.line] is undefined And this is the stack trace: TA__setBreakpoint@chrome://global/content/devtools/dbg-script-actors.js:517 TA__addScript@chrome://global/content/devtools/dbg-script-actors.js:1156 TA__discoverScriptsAndSources@chrome://global/content/devtools/dbg-script-actors.js:604 TA_onSources@chrome://global/content/devtools/dbg-script-actors.js:609 DSC_onPacket@chrome://global/content/devtools/dbg-server.js:653 LDT_send/<.run@chrome://global/content/devtools/dbg-transport.js:224

Eddy, any progress here? You mentioned in your blog that you were working on this.

(In reply to Panos Astithas [:past] from comment #1) > Eddy, any progress here? You mentioned in your blog that you were working on > this. Oops. This one dropped from my radar somehow. Sorry about that. I'll look into this tomorrow at the latest.

Looks like I was on the right track with my observation with my assertion that breakpoints[aLocation] and aLocation refer to the same object, but the problem was quite a bit subtler than that. The problem is in the following three lines: breakpoints[actualLocation.line] = breakpoints[aLocation.line]; breakpoints[actualLocation.line].line = actualLocation.line; delete breakpoints[aLocation.line]; The second line overwrites aLocation.line, after which the third line deletes the breakpoint actor at the new location, not the old one, as intended. The fix simply moves the third line above the other two. I've tested it on my machine and the problem seems to have gone away, but by all means double check this. I've also added some direly needed comments while I was at it.

Oh, for what it's worth: the crash you observed was caused by a different issue: the debugger API did not properly handle uncaught exceptions thrown from the onNewScript hook. Bug 858170 contains a fix for that problem too :-)

Comment on attachment 733578 [details] [diff] [review] Fix + comments Review of attachment 733578 [details] [diff] [review]: ----------------------------------------------------------------- I see that the error no longer appears after resuming, but I observed that the breakpoint is no longer hit in subsequent reloads. Also, xpcshell tests fail with this patch. Try running 'mach xpcshell-test toolkit/devtools' to see them. The oldLine temporary variable that was there before bug 820012 was because I had hit the same bug as well in the past. Maybe reusing that will be better? ::: toolkit/devtools/debugger/server/dbg-script-actors.js @@ +500,5 @@ > */ > _setBreakpoint: function TA__setBreakpoint(aLocation) { > let breakpoints = this._breakpointStore[aLocation.url]; > > + // Get or create the breakpoint actor for the given location If I was suffering from OCD I might have pointed out that comments in this file end with a period, but fortunately I'm not that weird.

Eddy, did you make any progress here?

(In reply to Panos Astithas [:past] from comment #6) > Eddy, did you make any progress here? I was occupied with prototyping symbols in SpiderMonkey for a while. This bug is still on my radar though. I'll put up a new patch beginning next week.

The problem was simple. I accidentally moved the line "delete breakpoints[aLocation.line];" above the line "breakpoints[actualLocation.line] = breakpoints[aLocation.line];". That obviously doesn't work. It should be just below that line, and just above the line "breakpoints[actualLocation.line].line = actualLocation.line;". I've updated the patch, and added another warning comment for good measure. Setting a breakpoint on an empty line now seems to work properly on my machine. However, I noticed that sometimes the breakpoint is also moved down to the next line in the debugger UI, but sometimes its not (the breakpoint still gets hit on the correct line after a refresh in that case, though). Is this behavior expected? I also ran the xpcshell tests on my machine this time, and didn't run into any more failing tests.

Comment on attachment 740308 [details] [diff] [review] Fix + comments (version 2) Review of attachment 740308 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! (In reply to Eddy Bruel [:ejpbruel] from comment #8) > Setting a breakpoint on an empty line now seems to work properly on my > machine. However, I noticed that sometimes the breakpoint is also moved down > to the next line in the debugger UI, but sometimes its not (the breakpoint > still gets hit on the correct line after a refresh in that case, though). Is > this behavior expected? Indeed, we want to let the user know that the line doesn't contain any bytecode, but we can't do that for scripts that are scavenged before the debugger is opened (bug 799070). ::: toolkit/devtools/debugger/server/dbg-script-actors.js @@ +713,5 @@ > } > > + /** > + * If we get here, no line matching the given line was found, so just > + * epically. "just fail epically"

(In reply to Panos Astithas [:past] from comment #9) > ::: toolkit/devtools/debugger/server/dbg-script-actors.js > @@ +713,5 @@ > > } > > > > + /** > > + * If we get here, no line matching the given line was found, so just > > + * epically. > > "just fail epically" And of course I forgot to fix it before pushing... *sigh*

Eddy, will you request aurora approval now that this has landed, or do you want me to?

Comment on attachment 740308 [details] [diff] [review] Fix + comments (version 2) This missed 22 when it was in Aurora, so requesting approval for beta. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 820012 User impact if declined: breakpoints in the debugger will sometimes not work Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low risk, doesn't affect regular users, only developers, long bake time in m-c String or IDL/UUID changes made by this patch: none

Comment on attachment 740308 [details] [diff] [review] Fix + comments (version 2) Early enough in the cycle to take this.

Verified as fixed on Firefox 22 beta 2: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:22.0) Gecko/20100101 Firefox/22.0 (20130521223249)