Bug 1141507 - Some preliminary refactors;r=jlong
authorEddy Bruël <ejpbruel@gmail.com>
Thu, 26 Mar 2015 20:15:36 +0100
changeset 266170 a10fbddc674ce5a5aac54f949d870bfbf4f0e718
parent 266169 ed5ed949d31c5d7d23c6150262569e7a3778ef27
child 266171 c2c24e1903f45db035462d803ee765203cc0acaf
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlong
bugs1141507
milestone39.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
Bug 1141507 - Some preliminary refactors;r=jlong
testing/xpcshell/head.js
toolkit/devtools/server/actors/script.js
toolkit/devtools/server/actors/utils/TabSources.js
--- a/testing/xpcshell/head.js
+++ b/testing/xpcshell/head.js
@@ -396,17 +396,17 @@ function _setupDebuggerServer(breakpoint
       case "devtools-thread-resumed":
         // Exceptions in here aren't reported and block the debugger from
         // resuming, so...
         try {
           // Add a breakpoint for the first line in our test files.
           let threadActor = subject.wrappedJSObject;
           for (let file of breakpointFiles) {
             let sourceActor = threadActor.sources.source({originalUrl: file});
-            sourceActor._setBreakpoint(new OriginalLocation(sourceActor, 1));
+            sourceActor._getOrCreateBreakpointActor(new OriginalLocation(sourceActor, 1));
           }
         } catch (ex) {
           do_print("Failed to initialize breakpoints: " + ex + "\n" + ex.stack);
         }
         break;
       case "xpcshell-test-devtools-shutdown":
         // the debugger has shutdown before we got a resume event - nothing
         // special to do here.
--- a/toolkit/devtools/server/actors/script.js
+++ b/toolkit/devtools/server/actors/script.js
@@ -1216,17 +1216,17 @@ ThreadActor.prototype = {
     for (let line = 0, n = offsets.length; line < n; line++) {
       if (offsets[line]) {
         // N.B. Hidden breakpoints do not have an original location, and are not
         // stored in the breakpoint actor map.
         let actor = new BreakpointActor(this);
         this.threadLifetimePool.addActor(actor);
         let scripts = this.scripts.getScriptsBySourceAndLine(script.source, line);
         let entryPoints = findEntryPointsForLine(scripts, line);
-        setBreakpointForActorAtEntryPoints(actor, entryPoints);
+        setBreakpointAtEntryPoints(actor, entryPoints);
         this._hiddenBreakpoints.set(actor.actorID, actor);
         break;
       }
     }
   },
 
   /**
    * Helper method that returns the next frame when stepping.
@@ -2041,25 +2041,28 @@ ThreadActor.prototype = {
     let endLine = aScript.startLine + aScript.lineCount - 1;
     for (let _actor of this.breakpointActorMap.findActors()) {
       // XXX bug 1142115: We do async work in here, so we need to
       // create a fresh binding because for/of does not yet do that in
       // SpiderMonkey
       let actor = _actor;
 
       if (actor.isPending) {
-        promises.push(sourceActor._setBreakpointForActor(actor));
+        promises.push(sourceActor._setBreakpoint(actor));
       } else {
         promises.push(this.sources.getGeneratedLocation(actor.originalLocation)
                                   .then((generatedLocation) => {
           // Limit the search to the line numbers contained in the new script.
           if (generatedLocation.generatedSourceActor.actorID === sourceActor.actorID &&
               generatedLocation.generatedLine >= aScript.startLine &&
               generatedLocation.generatedLine <= endLine) {
-            sourceActor._setBreakpointForActorAtLocation(actor, generatedLocation);
+            sourceActor._setBreakpointAtGeneratedLocation(
+              actor,
+              generatedLocation
+            );
           }
         }));
       }
     }
 
     if (promises.length > 0) {
       this.synchronize(Promise.all(promises));
     }
@@ -2295,16 +2298,17 @@ SourceActor.prototype = {
   _addonPath: null,
 
   get isSourceMapped() {
     return this._originalURL || this._generatedSource ||
            this.threadActor.sources.isPrettyPrinted(this.url);
   },
 
   get threadActor() { return this._threadActor; },
+  get sources() { return this._threadActor.sources; },
   get dbg() { return this.threadActor.dbg; },
   get scripts() { return this.threadActor.scripts; },
   get source() { return this._source; },
   get generatedSource() { return this._generatedSource; },
   get breakpointActorMap() { return this.threadActor.breakpointActorMap; },
   get url() {
     if (this.source) {
       return getSourceURL(this.source);
@@ -2738,17 +2742,20 @@ SourceActor.prototype = {
       return {
         error: "wrongState",
         message: "Cannot set breakpoint while debuggee is running."
       };
     }
 
     let { location: { line, column }, condition } = request;
     let location = new OriginalLocation(this, line, column);
-    return this._setBreakpoint(location, condition).then((actor) => {
+    return this._getOrCreateBreakpointActor(
+      location,
+      condition
+    ).then((actor) => {
       if (actor.isPending) {
         return {
           error: "noCodeAtLocation",
           actor: actor.actorID
         };
       } else {
         let response = {
           actor: actor.actorID
@@ -2774,27 +2781,27 @@ SourceActor.prototype = {
    *        the original source.
    * @param String condition
    *        A string that is evaluated whenever the breakpoint is hit. If the
    *        string evaluates to false, the breakpoint is ignored.
    *
    * @returns BreakpointActor
    *          A BreakpointActor representing the breakpoint.
    */
-  _setBreakpoint: function (originalLocation, condition) {
+  _getOrCreateBreakpointActor: function (originalLocation, condition) {
     let actor = this.breakpointActorMap.getActor(originalLocation);
     if (!actor) {
       actor = new BreakpointActor(this.threadActor, originalLocation);
       this.threadActor.threadLifetimePool.addActor(actor);
       this.breakpointActorMap.setActor(originalLocation, actor);
     }
 
     actor.condition = condition;
 
-    return this._setBreakpointForActor(actor);
+    return this._setBreakpoint(actor);
   },
 
   /*
    * Ensure the given BreakpointActor is set as a breakpoint handler on all
    * scripts that match its location in the original source.
    *
    * 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
@@ -2807,154 +2814,190 @@ SourceActor.prototype = {
    * 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) {
+  _setBreakpoint: function (actor) {
     let { originalLocation } = actor;
-
-    if (this.isSourceMapped) {
-      // TODO: Refactor breakpoint sliding for source mapped sources.
-      return this.threadActor.sources.getGeneratedLocation(originalLocation)
-                                     .then((generatedLocation) => {
-        return generatedLocation.generatedSourceActor
-                                ._setBreakpointForActorAtLocationWithSliding(
-          actor,
-          generatedLocation
-        );
-      });
-    } else {
-      // 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);
+    let { originalColumn } = originalLocation;
+
+    return this._setBreakpointAtOriginalLocation(actor, originalLocation)
+               .then((actualLocation) => {
+      if (actualLocation) {
+        return actualLocation;
       }
 
-      // 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;
+      if (!this.isSourceMapped) {
+        // There were no scripts that matched the given location, so we need to
+        // perform breakpoint sliding.
+        if (originalColumn === 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 });
               }
-              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;
+
+          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) {
+              setBreakpointAtEntryPoints(actor, entryPoints);
+              break;
+            }
+            ++actualLine;
           }
-          ++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(
+          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 originalLocation;
+          }
+
+          return 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);
-          }
+        } else {
+          // TODO: Implement breakpoint sliding for column breakpoints
+          return originalLocation;
+        }
+      } else {
+        // TODO: Refactor breakpoint sliding for source mapped sources.
+        return this.threadActor.sources.getGeneratedLocation(originalLocation)
+                                       .then((generatedLocation) => {
+          return generatedLocation.generatedSourceActor
+                                  ._setBreakpointAtLocationWithSliding(
+            actor,
+            generatedLocation
+          );
+        });
+      }
+    }).then((actualLocation) => {
+      // If the actual location on which the BreakpointActor ended up being
+      // set differs from the original line that was requested, both the
+      // BreakpointActor and the BreakpointActorMap need to be updated
+      // accordingly.
+      if (!actualLocation.equals(originalLocation)) {
+        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);
+      }
+
+      return actor;
+    });
+  },
+
+  _setBreakpointAtOriginalLocation: function (actor, originalLocation) {
+    if (!this.isSourceMapped) {
+      if (!this._setBreakpointAtGeneratedLocation(
+        actor,
+        GeneratedLocation.fromOriginalLocation(originalLocation)
+      )) {
+        return Promise.resolve(null);
       }
-    }
+
+      return Promise.resolve(originalLocation);
+    } else {
+      return this.sources.getAllGeneratedLocations(originalLocation)
+                         .then((generatedLocations) => {
+        if (!this._setBreakpointAtAllGeneratedLocations(
+          actor,
+          generatedLocations
+        )) {
+          return null;
+        }
+
+        return this.threadActor.sources.getOriginalLocation(generatedLocations[0]);
+      });
+    }
+  },
+
+  _setBreakpointAtAllGeneratedLocations: function (actor, generatedLocations) {
+    let success = false;
+    for (let generatedLocation of generatedLocations) {
+      if (this._setBreakpointAtGeneratedLocation(
+        actor,
+        generatedLocation
+      )) {
+        success = true;
+      }
+    }
+    return success;
   },
 
   /*
    * 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) {
+  _setBreakpointAtGeneratedLocation: 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));
 
@@ -2985,44 +3028,44 @@ SourceActor.prototype = {
           }
         }
       }
     }
 
     if (entryPoints.length === 0) {
       return false;
     }
-    setBreakpointForActorAtEntryPoints(actor, entryPoints);
+    setBreakpointAtEntryPoints(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
+   * The only place this method is still used is from setBreakpoint
    * 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) {
+  _setBreakpointAtLocationWithSliding: 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);
@@ -3077,45 +3120,25 @@ SourceActor.prototype = {
           generatedLocation.generatedSourceActor,
           result.line,
           generatedLocation.generatedColumn
         );
       } else {
         actualGeneratedLocation = generatedLocation;
       }
 
-      setBreakpointForActorAtEntryPoints(actor, result.entryPoints);
+      setBreakpointAtEntryPoints(actor, result.entryPoints);
     }
 
     return Promise.resolve().then(() => {
       if (actualGeneratedLocation.generatedSourceActor.source) {
         return this.threadActor.sources.getOriginalLocation(actualGeneratedLocation);
       } else {
         return OriginalLocation.fromGeneratedLocation(actualGeneratedLocation);
       }
-    }).then((actualOriginalLocation) => {
-      if (!actualOriginalLocation.equals(originalLocation)) {
-        // Check whether we already have a breakpoint actor for the actual
-        // location. If we do have an existing actor, then the actor we created
-        // 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.breakpointActorMap.getActor(actualOriginalLocation);
-        if (existingActor) {
-          actor.onDelete();
-          this.breakpointActorMap.deleteActor(originalLocation);
-          actor = existingActor;
-        } else {
-          actor.originalLocation = actualOriginalLocation;
-          this.breakpointActorMap.deleteActor(originalLocation);
-          this.breakpointActorMap.setActor(actualOriginalLocation, actor);
-        }
-      }
-
-      return actor;
     });
   },
 
   /**
    * Set breakpoints at the offsets closest to our target location's column.
    *
    * @param Array scripts
    *        The set of Debugger.Script instances to consider.
@@ -5427,16 +5450,16 @@ function findEntryPointsForLine(scripts,
  * Set breakpoints on all the given entry points with the given
  * BreakpointActor as the handler.
  *
  * @param BreakpointActor actor
  *        The actor handling the breakpoint hits.
  * @param Array entryPoints
  *        An array of objects of the form `{ script, offsets }`.
  */
-function setBreakpointForActorAtEntryPoints(actor, entryPoints) {
+function setBreakpointAtEntryPoints(actor, entryPoints) {
   for (let { script, offsets } of entryPoints) {
     actor.addScript(script);
     for (let offset of offsets) {
       script.setBreakpoint(offset, actor);
     }
   }
 }
--- a/toolkit/devtools/server/actors/utils/TabSources.js
+++ b/toolkit/devtools/server/actors/utils/TabSources.js
@@ -585,16 +585,27 @@ TabSources.prototype = {
         );
       }
 
       // No source map
       return OriginalLocation.fromGeneratedLocation(generatedLocation);
     });
   },
 
+  getAllGeneratedLocations: function (originalLocation) {
+    return this.getGeneratedLocation(originalLocation)
+               .then((generatedLocation) => {
+      if (generatedLocation.generatedLine === null &&
+          generatedLocation.generatedColumn === null) {
+        return [];
+      }
+      return [generatedLocation];
+    });
+  },
+
   /**
    * Returns a promise of the location in the generated source corresponding to
    * the original source and line given.
    *
    * When we pass a script S representing generated code to `sourceMap`,
    * above, that returns a promise P. The process of resolving P populates
    * the tables this function uses; thus, it won't know that S's original
    * source URLs map to S until P is resolved.