Bug 1107682 - Clean up the way we set breakpoints on newly introduced scripts. r=fitzgen, a=sledru
authorJames Long <longster@gmail.com>
Tue, 03 Feb 2015 08:22:00 -0500
changeset 243677 484b4f09fa9f
parent 243676 7422906b1a32
child 243678 8f1e0a224fb4
push id4433
push userryanvm@gmail.com
push date2015-02-04 15:40 +0000
treeherdermozilla-beta@a6b21fce4c12 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen, sledru
bugs1107682
milestone36.0
Bug 1107682 - Clean up the way we set breakpoints on newly introduced scripts. r=fitzgen, a=sledru
toolkit/devtools/server/actors/script.js
--- a/toolkit/devtools/server/actors/script.js
+++ b/toolkit/devtools/server/actors/script.js
@@ -3044,44 +3044,55 @@ SourceActor.prototype = {
         return { line, entryPoints };
       }
     }
 
     return null;
   },
 
   /**
-   * Set a breakpoint using the Debugger API. If the line on which the
-   * breakpoint is being set contains no code, then the breakpoint will slide
-   * down to the next line that has runnable code. In this case the server
-   * breakpoint cache will be updated, so callers that iterate over the
-   * breakpoint cache should take that into account.
+   * Get or create a BreakpointActor for the given location, and set it as a
+   * breakpoint handler on all scripts that match the given location for which
+   * the BreakpointActor is not already a breakpoint handler.
+   *
+   * 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
+   * breakpoint cache 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.
+   *
+   * 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, 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 breakpoint cache
+   * accordingly. Note that we only do so if the breakpoint actor is still
+   * pending (i.e. is not set as a breakpoint handler for any script).
    *
    * @param object aLocation
    *        The location of the breakpoint (in the generated source, if source
    *        mapping).
-   * @param Debugger.Script aOnlyThisScript [optional]
-   *        If provided, only set breakpoints in this Debugger.Script, and
-   *        nowhere else.
    */
-  _setBreakpoint: function (aLocation, aOnlyThisScript=null) {
+  _setBreakpoint: function (aLocation) {
     const location = {
       source: this.form(),
       line: aLocation.line,
       column: aLocation.column,
       condition: aLocation.condition
     };
     const actor = location.actor = this._getOrCreateBreakpointActor(location);
 
     // Find all scripts matching the given location. We will almost
     // always have a `source` object to query, but inline HTML scripts
     // are all represented by 1 SourceActor even though they have
     // separate source objects, so we need to query based on the url
     // of the page for them.
-    const scripts = this.dbg.findScripts({
+    let scripts = this.dbg.findScripts({
       source: this.source || undefined,
       url: this._originalUrl || undefined,
       line: location.line,
     });
 
     if (scripts.length === 0) {
       // Since we did not find any scripts to set the breakpoint on now, return
       // early. When a new script that matches this breakpoint location is
@@ -3097,17 +3108,31 @@ SourceActor.prototype = {
     if (location.column) {
       return this._setBreakpointAtColumn(scripts, location, actor);
     }
 
     // Select the first line that has offsets, and is greater than or equal to
     // the requested line. Set breakpoints on each of the offsets that is an
     // entry point to our selected line.
 
-    const result = this._findNextLineWithOffsets(scripts, location.line);
+    let result;
+    // Only try to find the next line with offsets if the breakpoint is still
+    // pending.
+    if (actor.scripts.size === 0) {
+      result = this._findNextLineWithOffsets(scripts, location.line);
+    } else {
+      let entryPoints = this._findEntryPointsForLine(scripts, location.line)
+      if (entryPoints) {
+        result = {
+          line: location.line,
+          entryPoints: entryPoints
+        };
+      }
+    }
+
     if (!result) {
       return {
         error: "noCodeAtLineColumn",
         actor: actor.actorID
       };
     }
 
     const { line, entryPoints } = result;
@@ -3134,21 +3159,21 @@ SourceActor.prototype = {
         actor.location = {
           sourceActor: this,
           line: actualLocation.line
         };
         this.breakpointStore.moveBreakpoint(location, actualLocation);
       }
     }
 
+    // Ignore scripts for which the BreakpointActor is already a breakpoint
+    // handler.
     this._setBreakpointOnEntryPoints(
       actor,
-      aOnlyThisScript
-        ? entryPoints.filter(o => o.script === aOnlyThisScript)
-        : entryPoints
+      entryPoints.filter((entryPoint) => !actor.hasScript(entryPoint.script))
     );
 
     return {
       actor: actor.actorID,
       actualLocation
     };
   },
 
@@ -4837,16 +4862,20 @@ function BreakpointActor(aThreadActor, {
   this.location = { sourceActor, line, column };
   this.condition = condition;
 }
 
 BreakpointActor.prototype = {
   actorPrefix: "breakpoint",
   condition: null,
 
+  hasScript: function (aScript) {
+    return this.scripts.has(aScript);
+  },
+
   /**
    * Called when this same breakpoint is added to another Debugger.Script
    * instance.
    *
    * @param aScript Debugger.Script
    *        The new source script on which the breakpoint has been set.
    * @param ThreadActor aThreadActor
    *        The parent thread actor that contains this breakpoint.