diff --git a/toolkit/devtools/server/actors/script.js b/toolkit/devtools/server/actors/script.js index 1a95a77f8d8..cf3ecb722ca 100644 --- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -107,7 +107,7 @@ BreakpointStore.prototype = { * @returns Object aBreakpoint * The new or existing breakpoint. */ - addBreakpoint: function (aBreakpoint, aActor) { + addBreakpoint: function (aBreakpoint) { let { source: { actor }, line, column } = aBreakpoint; if (column != null) { @@ -119,7 +119,7 @@ BreakpointStore.prototype = { } if (!this._breakpoints[actor][line][column]) { - this._breakpoints[actor][line][column] = aActor; + this._breakpoints[actor][line][column] = aBreakpoint; this._size++; } return this._breakpoints[actor][line][column]; @@ -130,7 +130,7 @@ BreakpointStore.prototype = { } if (!this._wholeLineBreakpoints[actor][line]) { - this._wholeLineBreakpoints[actor][line] = aActor; + this._wholeLineBreakpoints[actor][line] = aBreakpoint; this._size++; } return this._wholeLineBreakpoints[actor][line]; @@ -178,6 +178,53 @@ BreakpointStore.prototype = { } }, + /** + * Move the breakpoint to the new location. + * + * @param Object aBreakpoint + * The breakpoint being moved. See `addBreakpoint` for a description of + * its expected properties. + * @param Object aNewLocation + * The location to move the breakpoint to. Properties: + * - line + * - column (optional; omission implies whole line breakpoint) + */ + moveBreakpoint: function (aBreakpoint, aNewLocation) { + const existingBreakpoint = this.getBreakpoint(aBreakpoint); + this.removeBreakpoint(existingBreakpoint); + + const { line, column } = aNewLocation; + existingBreakpoint.line = line; + existingBreakpoint.column = column; + this.addBreakpoint(existingBreakpoint); + }, + + /** + * Get a breakpoint from the breakpoint store. Will throw an error if the + * breakpoint is not found. + * + * @param Object aLocation + * The location of the breakpoint you are retrieving. It is an object + * with the following properties: + * - source + * - line + * - column (optional) + */ + getBreakpoint: function (aLocation) { + let { source: { actor, url }, line, column } = aLocation; + dbg_assert(actor != null); + dbg_assert(line != null); + + var foundBreakpoint = this.hasBreakpoint(aLocation); + if (foundBreakpoint == null) { + throw new Error("No breakpoint at = " + (url || actor) + + ", line = " + line + + ", column = " + column); + } + + return foundBreakpoint; + }, + /** * Checks if the breakpoint store has a requested breakpoint. * @@ -189,16 +236,16 @@ BreakpointStore.prototype = { * - column (optional) * @returns The stored breakpoint if it exists, null otherwise. */ - getBreakpoint: function (aLocation) { + hasBreakpoint: function (aLocation) { let { source: { actor }, line, column } = aLocation; dbg_assert(actor != null); dbg_assert(line != null); - for (let actor of this.findBreakpoints(aLocation)) { + for (let bp of this.findBreakpoints(aLocation)) { // We will get whole line breakpoints before individual columns, so just // return the first one and if they didn't specify a column then they will // get the whole line breakpoint, and otherwise we will find the correct // one. - return actor; + return bp; } return null; @@ -228,7 +275,7 @@ BreakpointStore.prototype = { for (let actor of this._iterActors(actor)) { for (let line of this._iterLines(actor, aSearchParams.line)) { // Always yield whole line breakpoints first. See comment in - // |BreakpointStore.prototype.getBreakpoint|. + // |BreakpointStore.prototype.hasBreakpoint|. if (aSearchParams.column == null && this._wholeLineBreakpoints[actor] && this._wholeLineBreakpoints[actor][line]) { @@ -1283,16 +1330,18 @@ ThreadActor.prototype = { for (let line = 0, n = offsets.length; line < n; line++) { if (offsets[line]) { let location = { line: line }; - let resp = sourceActor._setBreakpoint(location); + let resp = sourceActor.createAndStoreBreakpoint(location); dbg_assert(!resp.actualLocation, "No actualLocation should be returned"); if (resp.error) { reportError(new Error("Unable to set breakpoint on event listener")); return; } - let bpActor = this.breakpointStore.getBreakpoint({ + let bp = this.breakpointStore.getBreakpoint({ source: sourceActor.form(), line: location.line }); + let bpActor = bp.actor; + dbg_assert(bp, "Breakpoint must exist"); dbg_assert(bpActor, "Breakpoint actor must be created"); this._hiddenBreakpoints.set(bpActor.actorID, bpActor); break; @@ -1447,8 +1496,10 @@ ThreadActor.prototype = { * caches won't hold on to the Debugger.Script objects leaking memory. */ disableAllBreakpoints: function () { - for (let bpActor of this.breakpointStore.findBreakpoints()) { - bpActor.removeScripts(); + for (let bp of this.breakpointStore.findBreakpoints()) { + if (bp.actor) { + bp.actor.removeScripts(); + } } }, @@ -2102,11 +2153,11 @@ ThreadActor.prototype = { let endLine = aScript.startLine + aScript.lineCount - 1; let source = this.sources.source({ source: aScript.source }); - for (let bpActor of this.breakpointStore.findBreakpoints({ source: source.form() })) { + for (let bp of this.breakpointStore.findBreakpoints(source.form())) { // Limit the search to the line numbers contained in the new script. - if (bpActor.location.line >= aScript.startLine - && bpActor.location.line <= endLine) { - source._setBreakpoint(bpActor.location, aScript); + if (bp.line >= aScript.startLine + && bp.line <= endLine) { + source._setBreakpoint(bp, aScript); } } @@ -2790,7 +2841,7 @@ SourceActor.prototype = { _createBreakpoint: function(loc, originalLoc, condition) { return resolve(null).then(() => { - return this._setBreakpoint({ + return this.createAndStoreBreakpoint({ line: loc.line, column: loc.column, condition: condition @@ -2853,6 +2904,23 @@ SourceActor.prototype = { }); }, + /** + * Create a breakpoint at the specified location and store it in the + * cache. Takes ownership of `aRequest`. This is the + * generated location if this source is sourcemapped. + * Used by the XPCShell test harness to set breakpoints in a script before + * it has loaded. + * + * @param Object aRequest + * An object of the form { line[, column, condition] }. The + * location is in the generated source, if sourcemapped. + */ + createAndStoreBreakpoint: function (aRequest) { + let bp = update({}, aRequest, { source: this.form() }); + this.breakpointStore.addBreakpoint(bp); + return this._setBreakpoint(aRequest); + }, + /** Get or create the BreakpointActor for the breakpoint at the given location. * * NB: This will override a pre-existing BreakpointActor's condition with @@ -2864,20 +2932,22 @@ SourceActor.prototype = { * @returns BreakpointActor */ _getOrCreateBreakpointActor: function (location) { - let actor = this.breakpointStore.getBreakpoint(location); - if (!actor) { - actor = new BreakpointActor(this.threadActor, { - sourceActor: this, - line: location.line, - column: location.column, - condition: location.condition - }); - this.threadActor.threadLifetimePool.addActor(actor); - this.breakpointStore.addBreakpoint(location, actor); + let actor; + const storedBp = this.breakpointStore.getBreakpoint(location); + + if (storedBp.actor) { + actor = storedBp.actor; + actor.condition = location.condition; return actor; } - actor.condition = location.condition; + storedBp.actor = actor = new BreakpointActor(this.threadActor, { + sourceActor: this, + line: location.line, + column: location.column, + condition: location.condition + }); + this.threadActor.threadLifetimePool.addActor(actor); return actor; }, @@ -3042,12 +3112,12 @@ SourceActor.prototype = { // above is redundant and must be destroyed. If we do not have an existing // actor, we need to update the breakpoint store with the new location. - let existingActor = this.breakpointStore.getBreakpoint(actualLocation); - if (existingActor) { + const existingBreakpoint = this.breakpointStore.hasBreakpoint(actualLocation); + if (existingBreakpoint && existingBreakpoint.actor) { actor.onDelete(); this.breakpointStore.removeBreakpoint(location); return { - actor: existingActor.actorID, + actor: existingBreakpoint.actor.actorID, actualLocation }; } else { @@ -3056,8 +3126,7 @@ SourceActor.prototype = { sourceActor: this, line: actualLocation.line }; - this.breakpointStore.removeBreakpoint(location); - this.breakpointStore.addBreakpoint(actualLocation, actor); + this.breakpointStore.moveBreakpoint(location, actualLocation); } } diff --git a/toolkit/devtools/server/tests/unit/test_breakpointstore.js b/toolkit/devtools/server/tests/unit/test_breakpointstore.js index 1ce5dbd4c9e..1e4ed102e17 100644 --- a/toolkit/devtools/server/tests/unit/test_breakpointstore.js +++ b/toolkit/devtools/server/tests/unit/test_breakpointstore.js @@ -11,15 +11,16 @@ function run_test() Cu.import("resource://gre/modules/jsdebugger.jsm"); addDebuggerToGlobal(this); - test_get_breakpoint(); + test_has_breakpoint(); test_add_breakpoint(); test_remove_breakpoint(); test_find_breakpoints(); test_duplicate_breakpoints(); + test_move_breakpoint(); } -function test_get_breakpoint() { - let bpStore = new BreakpointActorMap(); +function test_has_breakpoint() { + let bpStore = new BreakpointStore(); let location = { source: { actor: 'actor1' }, line: 3 @@ -31,27 +32,27 @@ function test_get_breakpoint() { }; // Shouldn't have breakpoint - do_check_eq(null, bpStore.getBreakpoint(location), + do_check_eq(null, bpStore.hasBreakpoint(location), "Breakpoint not added and shouldn't exist."); - bpStore.addBreakpoint(location, {}); - do_check_true(!!bpStore.getBreakpoint(location), + bpStore.addBreakpoint(location); + do_check_true(!!bpStore.hasBreakpoint(location), "Breakpoint added but not found in Breakpoint Store."); bpStore.removeBreakpoint(location); - do_check_eq(null, bpStore.getBreakpoint(location), + do_check_eq(null, bpStore.hasBreakpoint(location), "Breakpoint removed but still exists."); // Same checks for breakpoint with a column - do_check_eq(null, bpStore.getBreakpoint(columnLocation), + do_check_eq(null, bpStore.hasBreakpoint(columnLocation), "Breakpoint with column not added and shouldn't exist."); - bpStore.addBreakpoint(columnLocation, {}); - do_check_true(!!bpStore.getBreakpoint(columnLocation), + bpStore.addBreakpoint(columnLocation); + do_check_true(!!bpStore.hasBreakpoint(columnLocation), "Breakpoint with column added but not found in Breakpoint Store."); bpStore.removeBreakpoint(columnLocation); - do_check_eq(null, bpStore.getBreakpoint(columnLocation), + do_check_eq(null, bpStore.hasBreakpoint(columnLocation), "Breakpoint with column removed but still exists in Breakpoint Store."); } @@ -63,8 +64,8 @@ function test_add_breakpoint() { line: 10, column: 9 }; - bpStore.addBreakpoint(location, {}); - do_check_true(!!bpStore.getBreakpoint(location), + bpStore.addBreakpoint(location); + do_check_true(!!bpStore.hasBreakpoint(location), "We should have the column breakpoint we just added"); // Breakpoint without column (whole line breakpoint) @@ -72,8 +73,8 @@ function test_add_breakpoint() { source: { actor: 'actor2' }, line: 103 }; - bpStore.addBreakpoint(location, {}); - do_check_true(!!bpStore.getBreakpoint(location), + bpStore.addBreakpoint(location); + do_check_true(!!bpStore.hasBreakpoint(location), "We should have the whole line breakpoint we just added"); } @@ -85,9 +86,9 @@ function test_remove_breakpoint() { line: 10, column: 9 }; - bpStore.addBreakpoint(location, {}); + bpStore.addBreakpoint(location); bpStore.removeBreakpoint(location); - do_check_eq(bpStore.getBreakpoint(location), null, + do_check_eq(bpStore.hasBreakpoint(location), null, "We should not have the column breakpoint anymore"); // Breakpoint without column (whole line breakpoint) @@ -95,9 +96,9 @@ function test_remove_breakpoint() { source: { actor: 'actor2' }, line: 103 }; - bpStore.addBreakpoint(location, {}); + bpStore.addBreakpoint(location); bpStore.removeBreakpoint(location); - do_check_eq(bpStore.getBreakpoint(location), null, + do_check_eq(bpStore.hasBreakpoint(location), null, "We should not have the whole line breakpoint anymore"); } @@ -116,7 +117,7 @@ function test_find_breakpoints() { let bpStore = new BreakpointStore(); for (let bp of bps) { - bpStore.addBreakpoint(bp, bp); + bpStore.addBreakpoint(bp); } // All breakpoints @@ -165,8 +166,8 @@ function test_duplicate_breakpoints() { line: 10, column: 9 }; - bpStore.addBreakpoint(location, {}); - bpStore.addBreakpoint(location, {}); + bpStore.addBreakpoint(location); + bpStore.addBreakpoint(location); do_check_eq(bpStore.size, 1, "We should have only 1 column breakpoint"); bpStore.removeBreakpoint(location); @@ -175,8 +176,36 @@ function test_duplicate_breakpoints() { source: { actor: "foo-actor" }, line: 15 }; - bpStore.addBreakpoint(location, {}); - bpStore.addBreakpoint(location, {}); + bpStore.addBreakpoint(location); + bpStore.addBreakpoint(location); do_check_eq(bpStore.size, 1, "We should have only 1 whole line breakpoint"); bpStore.removeBreakpoint(location); } + +function test_move_breakpoint() { + let bpStore = new BreakpointStore(); + + let oldLocation = { + source: { actor: "foo-actor" }, + line: 10 + }; + + let newLocation = { + source: { actor: "foo-actor" }, + line: 12 + }; + + bpStore.addBreakpoint(oldLocation); + bpStore.moveBreakpoint(oldLocation, newLocation); + + equal(bpStore.size, 1, "Moving a breakpoint maintains the correct size."); + + let bp = bpStore.getBreakpoint(newLocation); + ok(bp, "We should be able to get a breakpoint at the new location."); + equal(bp.line, newLocation.line, + "We should get the moved line."); + + equal(bpStore.hasBreakpoint({ source: { actor: "foo-actor" }, line: 10 }), + null, + "And we shouldn't be able to get any BP at the old location."); +}