Bug 1129834 - Store BreakpointActors by original location;r=jlong

toolkit/devtools/server/actors/script.js

@@ -88,10 +88,10 @@ BreakpointActorMap.prototype = {
    * Generate all BreakpointActors that match the given location in
    * this BreakpointActorMap.
    *
-   * @param GeneratedLocation location
+   * @param OriginalLocation location
    *        The location for which matching BreakpointActors should be generated.
    */
-  findActors: function* (location = new GeneratedLocation()) {
+  findActors: function* (location = new OriginalLocation()) {
     function* findKeys(object, key) {
       if (key !== undefined) {
         if (key in object) {
@@ -106,12 +106,21 @@ BreakpointActorMap.prototype = {
     }
 
     let query = {
-      sourceActorID: location.generatedSourceActor ? location.generatedSourceActor.actorID : undefined,
+      sourceActorID: location.originalSourceActor ? location.originalSourceActor.actorID : undefined,
-      line: location.generatedLine,
+      line: location.originalLine,
-      beginColumn: location.generatedColumn ? location.generatedColumn : undefined,
-      endColumn: location.generatedColumn ? location.generatedColumn + 1 : undefined
     };
 
+    // If location contains a line, assume we are searching for a whole line
+    // breakpoint, and set begin/endColumn accordingly. Otherwise, we are
+    // searching for all breakpoints, so begin/endColumn should be left unset.
+    if (location.originalLine) {
+      query.beginColumn = location.originalColumn ? location.originalColumn : 0;
+      query.endColumn = location.originalColumn ? location.originalColumn + 1 : Infinity;
+    } else {
+      query.beginColumn = location.originalColumn ? query.originalColumn : undefined;
+      query.endColumn = location.originalColumn ? query.originalColumn + 1 : undefined;
+    }
+
     for (let sourceActorID of findKeys(this._actors, query.sourceActorID))
     for (let line of findKeys(this._actors[sourceActorID], query.line))
     for (let beginColumn of findKeys(this._actors[sourceActorID][line], query.beginColumn))
@@ -124,14 +133,14 @@ BreakpointActorMap.prototype = {
    * Return the BreakpointActor at the given location in this
    * BreakpointActorMap.
    *
-   * @param GeneratedLocation location
+   * @param OriginalLocation location
    *        The location for which the BreakpointActor should be returned.
    *
    * @returns BreakpointActor actor
    *          The BreakpointActor at the given location.
    */
-  getActor: function (location) {
+  getActor: function (originalLocation) {
-    for (let actor of this.findActors(location)) {
+    for (let actor of this.findActors(originalLocation)) {
       return actor;
     }
 
@@ -142,19 +151,19 @@ BreakpointActorMap.prototype = {
    * Set the given BreakpointActor to the given location in this
    * BreakpointActorMap.
    *
-   * @param GeneratedLocation location
+   * @param OriginalLocation location
    *        The location to which the given BreakpointActor should be set.
    *
    * @param BreakpointActor actor
    *        The BreakpointActor to be set to the given location.
    */
   setActor: function (location, actor) {
-    let { generatedSourceActor, generatedLine, generatedColumn } = location;
+    let { originalSourceActor, originalLine, originalColumn } = location;
 
-    let sourceActorID = generatedSourceActor.actorID;
+    let sourceActorID = originalSourceActor.actorID;
-    let line = generatedLine;
+    let line = originalLine;
-    let beginColumn = generatedColumn ? generatedColumn : 0;
+    let beginColumn = originalColumn ? originalColumn : 0;
-    let endColumn = generatedColumn ? generatedColumn + 1 : Infinity;
+    let endColumn = originalColumn ? originalColumn + 1 : Infinity;
 
     if (!this._actors[sourceActorID]) {
       this._actors[sourceActorID] = [];
@@ -175,16 +184,16 @@ BreakpointActorMap.prototype = {
    * Delete the BreakpointActor from the given location in this
    * BreakpointActorMap.
    *
-   * @param GeneratedLocation location
+   * @param OriginalLocation location
    *        The location from which the BreakpointActor should be deleted.
    */
   deleteActor: function (location) {
-    let { generatedSourceActor, generatedLine, generatedColumn } = location;
+    let { originalSourceActor, originalLine, originalColumn } = location;
 
-    let sourceActorID = generatedSourceActor.actorID;
+    let sourceActorID = originalSourceActor.actorID;
-    let line = generatedLine;
+    let line = originalLine;
-    let beginColumn = generatedColumn ? generatedColumn : 0;
+    let beginColumn = originalColumn ? originalColumn : 0;
-    let endColumn = generatedColumn ? generatedColumn + 1 : Infinity;
+    let endColumn = originalColumn ? originalColumn + 1 : Infinity;
 
     if (this._actors[sourceActorID]) {
       if (this._actors[sourceActorID][line]) {
@@ -2027,14 +2036,23 @@ ThreadActor.prototype = {
     }
 
     // Set any stored breakpoints.
+    let promises = [];
+    let sourceActor = this.sources.createNonSourceMappedActor(aScript.source);
     let endLine = aScript.startLine + aScript.lineCount - 1;
-    let source = this.sources.createNonSourceMappedActor(aScript.source);
+    for (let actor of this.breakpointActorMap.findActors()) {
-    for (let bpActor of this.breakpointActorMap.findActors({ sourceActor: source })) {
+      promises.push(this.sources.getGeneratedLocation(actor.originalLocation)
+                                .then((generatedLocation) => {
         // Limit the search to the line numbers contained in the new script.
-      if (bpActor.generatedLocation.generatedLine >= aScript.startLine
+        if (generatedLocation.generatedSourceActor.actorID === sourceActor.actorID &&
-          && bpActor.generatedLocation.generatedLine <= endLine) {
+            generatedLocation.generatedLine >= aScript.startLine &&
-        source.setBreakpointForActor(bpActor);
+            generatedLocation.generatedLine <= endLine) {
+          sourceActor.setBreakpointForActor(actor, generatedLocation);
         }
+      }));
+    }
+
+    if (promises.length > 0) {
+      this.synchronize(Promise.all(promises));
     }
 
     // Go ahead and establish the source actors for this script, which
@@ -2709,21 +2727,21 @@ SourceActor.prototype = {
    * NB: This will override a pre-existing BreakpointActor's condition with
    * the given the location's condition.
    *
-   * @param Object originalLocation
+   * @param OriginalLocation originalLocation
    *        The original location of the breakpoint.
-   * @param Object generatedLocation
+   * @param GeneratedLocation generatedLocation
    *        The generated location of the breakpoint.
    * @returns BreakpointActor
    */
   _getOrCreateBreakpointActor: function (originalLocation, generatedLocation,
                                          condition)
   {
-    let actor = this.breakpointActorMap.getActor(generatedLocation);
+    let actor = this.breakpointActorMap.getActor(originalLocation);
     if (!actor) {
       actor = new BreakpointActor(this.threadActor, originalLocation,
                                   generatedLocation, condition);
       this.threadActor.threadLifetimePool.addActor(actor);
-      this.breakpointActorMap.setActor(generatedLocation, actor);
+      this.breakpointActorMap.setActor(originalLocation, actor);
       return actor;
     }
 
@@ -2832,26 +2850,24 @@ SourceActor.prototype = {
       let actor = this._getOrCreateBreakpointActor(originalLocation,
                                                    generatedLocation,
                                                    condition);
-      return generatedLocation.generatedSourceActor.setBreakpointForActor(actor);
+      return generatedLocation.generatedSourceActor
+                              .setBreakpointForActor(actor, generatedLocation);
     });
   },
 
   /*
-   * Set the given BreakpointActor as breakpoint handler on all scripts that
+   * Ensure the given BreakpointActor is set as breakpoint handler on all
-   * match the given location for which the BreakpointActor is not already a
+   * scripts that match the given generated location.
-   * breakpoint handler.
    *
    * @param BreakpointActor actor
-   *        The BreakpointActor to set as breakpoint handler.
+   *        The BreakpointActor to be set as breakpoint handler for the given
+   *        generated location.
+   * @param GeneratedLocation generatedLocation
+   *        The generated location for which the BreakpointActor should be set
+   *        as breakpoint handler.
    */
-  setBreakpointForActor: function (actor) {
+  setBreakpointForActor: function (actor, generatedLocation) {
     let originalLocation = actor.originalLocation;
-    let generatedLocation = new GeneratedLocation(
-      this,
-      actor.generatedLocation.generatedLine,
-      actor.generatedLocation.generatedColumn
-    );
-
     let { generatedLine, generatedColumn } = generatedLocation;
 
     // Find all scripts matching the given location. We will almost always have
@@ -2919,27 +2935,11 @@ SourceActor.prototype = {
           result.line,
           generatedLocation.generatedColumn
         );
-
-        // 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(actualGeneratedLocation);
-        if (existingActor) {
-          actor.onDelete();
-          this.breakpointActorMap.deleteActor(generatedLocation);
-          actor = existingActor;
       } else {
-          actor.generatedLocation = actualGeneratedLocation;
-          this.breakpointActorMap.deleteActor(generatedLocation);
-          this.breakpointActorMap.setActor(actualGeneratedLocation, actor);
-          setBreakpointOnEntryPoints(this.threadActor, actor, result.entryPoints);
-        }
-      } else {
-        setBreakpointOnEntryPoints(this.threadActor, actor, result.entryPoints);
         actualGeneratedLocation = generatedLocation;
       }
+
+      setBreakpointOnEntryPoints(this.threadActor, actor, result.entryPoints);
     }
 
     return Promise.resolve().then(() => {
@@ -2953,6 +2953,22 @@ SourceActor.prototype = {
       if (actualOriginalLocation.originalSourceActor.url !== originalLocation.originalSourceActor.url ||
           actualOriginalLocation.originalLine !== originalLocation.originalLine)
       {
+        // 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);
+          response.actor = existingActor.actorID;
+        } else {
+          actor.generatedLocation = actualGeneratedLocation;
+          this.breakpointActorMap.deleteActor(originalLocation);
+          this.breakpointActorMap.setActor(actualOriginalLocation, actor);
+        }
+
         response.actualLocation = {
           source: actualOriginalLocation.originalSourceActor.form(),
           line: actualOriginalLocation.originalLine,
@@ -4612,16 +4628,10 @@ FrameActor.prototype.requestTypes = {
  *
  * @param ThreadActor aThreadActor
  *        The parent thread actor that contains this breakpoint.
- * @param object aOriginalLocation
+ * @param OriginalLocation originalLocation
- *        An object with the following properties:
+ *        The original location of the breakpoint.
- *        - sourceActor: A SourceActor that represents the source
+ * @param GeneratedLocation generatedLocation
- *        - line: the specified line
+ *        The generated location of the breakpoint.
- *        - column: the specified column
- * @param object aGeneratedLocation
- *        An object with the following properties:
- *        - sourceActor: A SourceActor that represents the source
- *        - line: the specified line
- *        - column: the specified column
  * @param string aCondition
  *        Optional. A condition which, when false, will cause the breakpoint to
  *        be skipped.
@@ -4723,8 +4733,8 @@ BreakpointActor.prototype = {
    */
   onDelete: function (aRequest) {
     // Remove from the breakpoint store.
-    if (this.generatedLocation) {
+    if (this.originalLocation) {
-      this.threadActor.breakpointActorMap.deleteActor(this.generatedLocation);
+      this.threadActor.breakpointActorMap.deleteActor(this.originalLocation);
     }
     this.threadActor.threadLifetimePool.removeActor(this);
     // Remove the actual breakpoint from the associated scripts.

toolkit/devtools/server/tests/unit/test_breakpoint-actor-map.js

@@ -21,13 +21,13 @@ function run_test()
 function test_get_actor() {
   let bpStore = new BreakpointActorMap();
   let location = {
-    generatedSourceActor: { actor: 'actor1' },
+    originalSourceActor: { actor: 'actor1' },
-    generatedLine: 3
+    originalLine: 3
   };
   let columnLocation = {
-    generatedSourceActor: { actor: 'actor2' },
+    originalSourceActor: { actor: 'actor2' },
-    generatedLine: 5,
+    originalLine: 5,
-    generatedColumn: 15
+    originalColumn: 15
   };
 
   // Shouldn't have breakpoint
@@ -59,9 +59,9 @@ function test_set_actor() {
   // Breakpoint with column
   let bpStore = new BreakpointActorMap();
   let location = {
-    generatedSourceActor: { actor: 'actor1' },
+    originalSourceActor: { actor: 'actor1' },
-    generatedLine: 10,
+    originalLine: 10,
-    generatedColumn: 9
+    originalColumn: 9
   };
   bpStore.setActor(location, {});
   do_check_true(!!bpStore.getActor(location),
@@ -69,8 +69,8 @@ function test_set_actor() {
 
   // Breakpoint without column (whole line breakpoint)
   location = {
-    generatedSourceActor: { actor: 'actor2' },
+    originalSourceActor: { actor: 'actor2' },
-    generatedLine: 103
+    originalLine: 103
   };
   bpStore.setActor(location, {});
   do_check_true(!!bpStore.getActor(location),
@@ -81,9 +81,9 @@ function test_delete_actor() {
   // Breakpoint with column
   let bpStore = new BreakpointActorMap();
   let location = {
-    generatedSourceActor: { actor: 'actor1' },
+    originalSourceActor: { actor: 'actor1' },
-    generatedLine: 10,
+    originalLine: 10,
-    generatedColumn: 9
+    originalColumn: 9
   };
   bpStore.setActor(location, {});
   bpStore.deleteActor(location);
@@ -92,8 +92,8 @@ function test_delete_actor() {
 
   // Breakpoint without column (whole line breakpoint)
   location = {
-    generatedSourceActor: { actor: 'actor2' },
+    originalSourceActor: { actor: 'actor2' },
-    generatedLine: 103
+    originalLine: 103
   };
   bpStore.setActor(location, {});
   bpStore.deleteActor(location);
@@ -103,14 +103,14 @@ function test_delete_actor() {
 
 function test_find_actors() {
   let bps = [
-    { generatedSourceActor: { actor: "actor1" }, generatedLine: 10 },
+    { originalSourceActor: { actor: "actor1" }, originalLine: 10 },
-    { generatedSourceActor: { actor: "actor1" }, generatedLine: 10, generatedColumn: 3 },
+    { originalSourceActor: { actor: "actor1" }, originalLine: 10, originalColumn: 3 },
-    { generatedSourceActor: { actor: "actor1" }, generatedLine: 10, generatedColumn: 10 },
+    { originalSourceActor: { actor: "actor1" }, originalLine: 10, originalColumn: 10 },
-    { generatedSourceActor: { actor: "actor1" }, generatedLine: 23, generatedColumn: 89 },
+    { originalSourceActor: { actor: "actor1" }, originalLine: 23, originalColumn: 89 },
-    { generatedSourceActor: { actor: "actor2" }, generatedLine: 10, generatedColumn: 1 },
+    { originalSourceActor: { actor: "actor2" }, originalLine: 10, originalColumn: 1 },
-    { generatedSourceActor: { actor: "actor2" }, generatedLine: 20, generatedColumn: 5 },
+    { originalSourceActor: { actor: "actor2" }, originalLine: 20, originalColumn: 5 },
-    { generatedSourceActor: { actor: "actor2" }, generatedLine: 30, generatedColumn: 34 },
+    { originalSourceActor: { actor: "actor2" }, originalLine: 30, originalColumn: 34 },
-    { generatedSourceActor: { actor: "actor2" }, generatedLine: 40, generatedColumn: 56 }
+    { originalSourceActor: { actor: "actor2" }, originalLine: 40, originalColumn: 56 }
   ];
 
   let bpStore = new BreakpointActorMap();
@@ -130,8 +130,8 @@ function test_find_actors() {
 
   // Breakpoints by URL
 
-  bpSet = new Set(bps.filter(bp => { return bp.generatedSourceActor.actorID === "actor1" }));
+  bpSet = new Set(bps.filter(bp => { return bp.originalSourceActor.actorID === "actor1" }));
-  for (let bp of bpStore.findActors({ generatedSourceActor: { actorID: "actor1" } })) {
+  for (let bp of bpStore.findActors({ originalSourceActor: { actorID: "actor1" } })) {
     bpSet.delete(bp);
   }
   do_check_eq(bpSet.size, 0,
@@ -139,15 +139,15 @@ function test_find_actors() {
 
   // Breakpoints by URL and line
 
-  bpSet = new Set(bps.filter(bp => { return bp.generatedSourceActor.actorID === "actor1" && bp.generatedLine === 10; }));
+  bpSet = new Set(bps.filter(bp => { return bp.originalSourceActor.actorID === "actor1" && bp.originalLine === 10; }));
   let first = true;
-  for (let bp of bpStore.findActors({ generatedSourceActor: { actorID: "actor1" }, generatedLine: 10 })) {
+  for (let bp of bpStore.findActors({ originalSourceActor: { actorID: "actor1" }, originalLine: 10 })) {
     if (first) {
-      do_check_eq(bp.generatedColumn, undefined,
+      do_check_eq(bp.originalColumn, undefined,
                   "Should always get the whole line breakpoint first");
       first = false;
     } else {
-      do_check_neq(bp.generatedColumn, undefined,
+      do_check_neq(bp.originalColumn, undefined,
                    "Should not get the whole line breakpoint any time other than first.");
     }
     bpSet.delete(bp);
@@ -161,9 +161,9 @@ function test_duplicate_actors() {
 
   // Breakpoint with column
   let location = {
-    generatedSourceActor: { actorID: "foo-actor" },
+    originalSourceActor: { actorID: "foo-actor" },
-    generatedLine: 10,
+    originalLine: 10,
-    generatedColumn: 9
+    originalColumn: 9
   };
   bpStore.setActor(location, {});
   bpStore.setActor(location, {});
@@ -172,8 +172,8 @@ function test_duplicate_actors() {
 
   // Breakpoint without column (whole line breakpoint)
   location = {
-    generatedSourceActor: { actorID: "foo-actor" },
+    originalSourceActor: { actorID: "foo-actor" },
-    generatedLine: 15
+    originalLine: 15
   };
   bpStore.setActor(location, {});
   bpStore.setActor(location, {});