Backed out changeset 49218eb557ad (bug 1102441) for XPC Shell Test failures on a CLOSED TREE

--HG--
extra : amend_source : c64c1e23019f599454321f98899d8e99883d7564

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()) {
+    for (let bp of this.breakpointStore.findBreakpoints()) {
-      bpActor.removeScripts();
+      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
+      if (bp.line >= aScript.startLine
-          && bpActor.location.line <= endLine) {
+          && bp.line <= endLine) {
-        source._setBreakpoint(bpActor.location, aScript);
+        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);
+    let actor;
-    if (!actor) {
+    const storedBp = this.breakpointStore.getBreakpoint(location);
-      actor = new BreakpointActor(this.threadActor, {
+
-        sourceActor: this,
+    if (storedBp.actor) {
-        line: location.line,
+      actor = storedBp.actor;
-        column: location.column,
+      actor.condition = location.condition;
-        condition: location.condition
-      });
-      this.threadActor.threadLifetimePool.addActor(actor);
-      this.breakpointStore.addBreakpoint(location, actor);
       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);
+      const existingBreakpoint = this.breakpointStore.hasBreakpoint(actualLocation);
-      if (existingActor) {
+      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.moveBreakpoint(location, actualLocation);
-        this.breakpointStore.addBreakpoint(actualLocation, actor);
       }
     }
 

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() {
+function test_has_breakpoint() {
-  let bpStore = new BreakpointActorMap();
+  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, {});
+  bpStore.addBreakpoint(location);
-  do_check_true(!!bpStore.getBreakpoint(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, {});
+  bpStore.addBreakpoint(columnLocation);
-  do_check_true(!!bpStore.getBreakpoint(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, {});
+  bpStore.addBreakpoint(location);
-  do_check_true(!!bpStore.getBreakpoint(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, {});
+  bpStore.addBreakpoint(location);
-  do_check_true(!!bpStore.getBreakpoint(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.");
+}