author | Eddy Bruël <ejpbruel@gmail.com> |
Mon, 23 Mar 2015 14:13:03 +0100 | |
changeset 235170 | 844a5d08948ff09177c1e239003da04b51f16f38 |
parent 235169 | 358d0174fee4da8eeeb3d5e75c6ba3e75545e3e2 |
child 235171 | 35d17c53d9d13304b2555168491d4d883c9db423 |
push id | 57353 |
push user | kwierso@gmail.com |
push date | Mon, 23 Mar 2015 23:51:33 +0000 |
treeherder | mozilla-inbound@7f5abc27fd53 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | jlong |
bugs | 1138975 |
milestone | 39.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/devtools/debugger/test/browser_dbg_source-maps-04.js +++ b/browser/devtools/debugger/test/browser_dbg_source-maps-04.js @@ -84,21 +84,21 @@ function disableIgnoreCaughtExceptions() return deferred.promise; } function testSetBreakpoint() { let deferred = promise.defer(); let sourceForm = getSourceForm(gSources, JS_URL); let source = gDebugger.gThreadClient.source(sourceForm); - source.setBreakpoint({ line: 3, column: 61 }, aResponse => { + source.setBreakpoint({ line: 3, column: 18 }, aResponse => { ok(!aResponse.error, "Should be able to set a breakpoint in a js file."); ok(!aResponse.actualLocation, - "Should be able to set a breakpoint on line 3 and column 61."); + "Should be able to set a breakpoint on line 3 and column 18."); deferred.resolve(); }); return deferred.promise; } function reloadPage() {
--- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -2801,68 +2801,238 @@ SourceActor.prototype = { return this._setBreakpointForActor(actor); }, /* * Ensure the given BreakpointActor is set as a breakpoint handler on all * scripts that match its location in the original source. * - * It is possible that no scripts match the given location, because they have - * all been garbage collected. In that case, the BreakpointActor is not set as - * a breakpoint handler for any script, but is still inserted in the - * BreakpointActorMap as a pending breakpoint. Whenever a new script is - * introduced, we call this method again to see if there are now any scripts - * that matches the given location. + * If there are no scripts that match the location of the BreakpointActor, + * we slide its location to the next closest line (for line breakpoints) or + * column (for column breakpoint) that does. * - * The first time we find one or more scripts that matches the given location, - * we check if any of these scripts has any entry points for the given - * location. If not, we assume that the given location does not have any code. - * - * If the given location does not contain any code, we slide the breakpoint - * down to the next closest line that does, and update the BreakpointActorMap - * accordingly. Note that we only do so if the BreakpointActor is still - * pending (i.e. is not set as a breakpoint handler for any script). + * If breakpoint sliding fails, then either there are no scripts that contain + * any code for the given location, or they were all garbage collected before + * the debugger started running. We cannot distinguish between these two + * cases, so we insert the BreakpointActor in the BreakpointActorMap as + * a pending breakpoint. Whenever a new script is introduced, this method is + * called again for each pending breakpoint. * * @param BreakpointActor actor * The BreakpointActor to be set as a breakpoint handler. * * @returns A Promise that resolves to the given BreakpointActor. */ _setBreakpointForActor: function (actor) { + let { originalLocation } = actor; + if (this.isSourceMapped) { - return this.threadActor.sources.getGeneratedLocation( - actor.originalLocation - ).then((generatedLocation) => { + // TODO: Refactor breakpoint sliding for source mapped sources. + return this.threadActor.sources.getGeneratedLocation(originalLocation) + .then((generatedLocation) => { return generatedLocation.generatedSourceActor - ._setBreakpointForActorAtLocation( + ._setBreakpointForActorAtLocationWithSliding( actor, generatedLocation ); }); } else { - return Promise.resolve(this._setBreakpointForActorAtLocation( - actor, - GeneratedLocation.fromOriginalLocation(actor.originalLocation) - )); + // If this is a non-source mapped source, the original location and + // generated location are the same, so we can safely convert between them. + let generatedLocation = GeneratedLocation.fromOriginalLocation(originalLocation); + let { generatedColumn } = generatedLocation; + + // Try to set the breakpoint on the generated location directly. If this + // succeeds, we can avoid the more expensive breakpoint sliding algorithm + // below. + if (this._setBreakpointForActorAtLocation(actor, generatedLocation)) { + return Promise.resolve(actor); + } + + // There were no scripts that matched the given location, so we need to + // perform breakpoint sliding. + if (generatedColumn === undefined) { + // To perform breakpoint sliding for line breakpoints, we need to build + // a map from line numbers to a list of entry points for each line, + // implemented as a sparse array. An entry point is a (script, offsets) + // pair, and represents all offsets in that script that are entry points + // for the corresponding line. + let lineToEntryPointsMap = []; + + // Iterate over all scripts that correspond to this source actor. + let scripts = this.scripts.getScriptsBySourceActor(this); + for (let script of scripts) { + // Get all offsets for each line in the current script. This returns + // a map from line numbers fo a list of offsets for each line, + // implemented as a sparse array. + let lineToOffsetsMap = script.getAllOffsets(); + + // Iterate over each line, and add their list of offsets to the map + // from line numbers to entry points by forming a (script, offsets) + // pair, where script is the current script, and offsets is the list + // of offsets for the current line. + for (let line = 0; line < lineToOffsetsMap.length; ++line) { + let offsets = lineToOffsetsMap[line]; + if (offsets) { + let entryPoints = lineToEntryPointsMap[line]; + if (!entryPoints) { + // We dont have a list of entry points for the current line + // number yet, so create it and add it to the map. + entryPoints = []; + lineToEntryPointsMap[line] = entryPoints; + } + entryPoints.push({ script, offsets }); + } + } + } + + let { + originalSourceActor, + originalLine, + originalColumn + } = originalLocation; + + // Now that we have a map from line numbers to a list of entry points + // for each line, we can use it to perform breakpoint sliding. Start + // at the original line of the breakpoint actor, and keep incrementing + // it by one, until either we find a line that has at least one entry + // point, or we go past the last line in the map. + // + // Note that by computing the entire map up front, and implementing it + // as a sparse array, we can easily tell when we went past the last line + // in the map. + let actualLine = originalLine; + while (actualLine < lineToEntryPointsMap.length) { + let entryPoints = lineToEntryPointsMap[actualLine]; + if (entryPoints) { + setBreakpointForActorAtEntryPoints(actor, entryPoints); + break; + } + ++actualLine; + } + if (actualLine === lineToEntryPointsMap.length) { + // We went past the last line in the map, so breakpoint sliding + // failed. Keep the BreakpointActor in the BreakpointActorMap as a + // pending breakpoint, so we can try again whenever a new script is + // introduced. + return Promise.resolve(actor); + } + + // If the actual line on which the BreakpointActor was set differs from + // the original line that was requested, the BreakpointActor and the + // BreakpointActorMap need to be updated accordingly. + if (actualLine !== originalLine) { + let actualLocation = new OriginalLocation( + originalSourceActor, + actualLine + ); + let existingActor = this.breakpointActorMap.getActor(actualLocation); + if (existingActor) { + actor.onDelete(); + this.breakpointActorMap.deleteActor(originalLocation); + actor = existingActor; + } else { + this.breakpointActorMap.deleteActor(originalLocation); + actor.originalLocation = actualLocation; + this.breakpointActorMap.setActor(actualLocation, actor); + } + } + + return Promise.resolve(actor); + } else { + // TODO: Implement breakpoint sliding for column breakpoints + return Promise.resolve(actor); + } } }, /* * Ensure the given BreakpointActor is set as breakpoint handler on all * scripts that match the given location in the generated source. * * @param BreakpointActor actor * The BreakpointActor to be set as a breakpoint handler. * @param GeneratedLocation generatedLocation * A GeneratedLocation representing the location in the generated * source for which the given BreakpointActor is to be set as a * breakpoint handler. + * + * @returns A Boolean that is true if the BreakpointActor was set as a + * breakpoint handler on at least one script, and false otherwise. */ _setBreakpointForActorAtLocation: function (actor, generatedLocation) { + let { generatedLine, generatedColumn } = generatedLocation; + + // Find all scripts that match the given source actor and line number. + let scripts = this.scripts.getScriptsBySourceActorAndLine( + this, + generatedLine + ).filter((script) => !actor.hasScript(script)); + + // Find all entry points that correspond to the given location. + let entryPoints = []; + if (generatedColumn === undefined) { + // This is a line breakpoint, so we are interested in all offsets + // that correspond to the given line number. + for (let script of scripts) { + let offsets = script.getLineOffsets(generatedLine); + if (offsets.length > 0) { + entryPoints.push({ script, offsets }); + } + } + } else { + // This is a column breakpoint, so we are interested in all column + // offsets that correspond to the given line *and* column number. + for (let script of scripts) { + let columnToOffsetMap = script.getAllColumnOffsets() + .filter(({ lineNumber }) => { + return lineNumber === generatedLine; + }); + for (let { columnNumber: column, offset } of columnToOffsetMap) { + // TODO: What we are actually interested in here is a range of + // columns, rather than a single one. + if (column == generatedColumn) { + entryPoints.push({ script, offsets: [offset] }); + } + } + } + } + + if (entryPoints.length === 0) { + return false; + } + setBreakpointForActorAtEntryPoints(actor, entryPoints); + return true; + }, + + /* + * Ensure the given BreakpointActor is set as breakpoint handler on all + * scripts that match the given location in the generated source. + * + * TODO: This method is bugged, because it performs breakpoint sliding on + * generated locations. Breakpoint sliding should be performed on original + * locations, because there is no guarantee that the next line in the + * generated source corresponds to the next line in an original source. + * + * The only place this method is still used is from setBreakpointForActor + * when called for a source mapped source. Once that code has been refactored, + * this method can be removed. + * + * @param BreakpointActor actor + * The BreakpointActor to be set as a breakpoint handler. + * @param GeneratedLocation generatedLocation + * A GeneratedLocation representing the location in the generated + * source for which the given BreakpointActor is to be set as a + * breakpoint handler. + * + * @returns A Boolean that is true if the BreakpointActor was set as a + * breakpoint handler on at least one script, and false otherwise. + */ + _setBreakpointForActorAtLocationWithSliding: function (actor, generatedLocation) { let originalLocation = actor.originalLocation; let { generatedLine, generatedColumn } = generatedLocation; // Find all scripts matching the given location. We will almost always have // a `source` object to query, but multiple inline HTML scripts are all // represented by a single SourceActor even though they have separate source // objects, so we need to query based on the url of the page for them. let scripts = this.scripts.getScriptsBySourceActorAndLine(this, generatedLine); @@ -2977,20 +3147,16 @@ SourceActor.prototype = { } for (let [script, mappings] of scriptsAndOffsetMappings) { for (let offsetMapping of mappings) { script.setBreakpoint(offsetMapping.offset, actor); } actor.addScript(script, this.threadActor); } - - return { - actor: actor.actorID - }; }, /** * Find the first line that is associated with bytecode offsets, and is * greater than or equal to the given start line. * * @param Array scripts * The set of Debugger.Script instances to consider.
--- a/toolkit/devtools/server/actors/utils/ScriptStore.js +++ b/toolkit/devtools/server/actors/utils/ScriptStore.js @@ -74,16 +74,22 @@ ScriptStore.prototype = { * * NB: The ScriptStore retains ownership of the returned array, and the * ScriptStore's consumers MUST NOT MODIFY its contents! */ getAllScripts() { return this._scripts.items; }, + getScriptsBySourceActor(sourceActor) { + return sourceActor.source ? + this.getScriptsBySource(sourceActor.source) : + this.getScriptsByURL(sourceActor._originalUrl); + }, + getScriptsBySourceActorAndLine(sourceActor, line) { return sourceActor.source ? this.getScriptsBySourceAndLine(sourceActor.source, line) : this.getScriptsByURLAndLine(sourceActor._originalUrl, line); }, /** * Get all scripts produced from the given source.