Bug 897744 - BreakpointStore should have a hasBreakpoint method
☠☠ backed out by fb1778aa9589 ☠ ☠
authorMason Chang <mason.chang@oracle.com>
Tue, 30 Jul 2013 19:34:10 -0700
changeset 153051 041986a971af304ebb05f8619536b0ff0e6d6c88
parent 153050 2466e891446df947807ff3868d40e98b74d2b6fc
child 153052 fb1778aa95891d287e7de357cc4d91020848a50d
push id2859
push userakeybl@mozilla.com
push dateMon, 16 Sep 2013 19:14:59 +0000
treeherdermozilla-beta@87d3c51cd2bf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs897744
milestone25.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 897744 - BreakpointStore should have a hasBreakpoint method
toolkit/devtools/server/actors/script.js
toolkit/devtools/server/tests/unit/test_breakpointstore.js
--- a/toolkit/devtools/server/actors/script.js
+++ b/toolkit/devtools/server/actors/script.js
@@ -101,44 +101,64 @@ BreakpointStore.prototype = {
       if (this._wholeLineBreakpoints[url]) {
         delete this._wholeLineBreakpoints[url][line];
       }
     }
   },
 
   /**
    * Get a breakpoint from the breakpoint store. Will throw an error if the
-   * breakpoint is not found unless you explicitly silence it.
+   * breakpoint is not found.
    *
    * @param Object aLocation
    *        The location of the breakpoint you are retrieving. It is an object
    *        with the following properties:
    *          - url
    *          - line
    *          - column (optional)
-   * @param bool aShouldThrow
-   *        Optional; defaults to true. Whether an error should be thrown when
-   *        there is no breakpoint at the specified locaiton.
    */
-  getBreakpoint: function BS_getBreakpoint(aLocation, aShouldThrow=true) {
+  getBreakpoint: function BS_getBreakpoint(aLocation) {
+    let { url, line, column } = aLocation;
+    dbg_assert(url != null);
+    dbg_assert(line != null);
+
+    var foundBreakpoint = this.hasBreakpoint(aLocation);
+    if (foundBreakpoint == null) {
+      throw new Error("No breakpoint at url = " + url
+          + ", line = " + line
+          + ", column = " + column);
+    }
+
+    return foundBreakpoint;
+  },
+
+  /**
+   * Checks if the breakpoint store has a requested breakpoint
+   * Returns the stored breakpoint if it exists
+   * null otherwise
+   *
+   * @param Object aLocation
+   *        The location of the breakpoint you are retrieving. It is an object
+   *        with the following properties:
+   *          - url
+   *          - line
+   *          - column (optional)
+   */
+  hasBreakpoint: function BS_hasBreakpoint(aLocation) {
     let { url, line, column } = aLocation;
     dbg_assert(url != null);
     dbg_assert(line != null);
     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 bp;
     }
-    if (aShouldThrow) {
-      throw new Error("No breakpoint at url = " + url
-                      + ", line = " + line
-                      + ", column = " + column);
-    }
+
     return null;
   },
 
   /**
    * Iterate over the breakpoints in this breakpoint store. You can optionally
    * provide search parameters to filter the set of breakpoints down to those
    * that match your parameters.
    *
@@ -1073,17 +1093,18 @@ ThreadActor.prototype = {
             };
           }
           found = true;
           break;
         }
       }
     }
     if (found) {
-      let existingBp = this.breakpointStore.getBreakpoint(actualLocation, false);
+      let existingBp = this.breakpointStore.hasBreakpoint(actualLocation);
+
       if (existingBp && existingBp.actor) {
         /**
          * We already have a breakpoint actor for the actual location, so
          * actor we created earlier is now redundant. Delete it, update the
          * breakpoint store, and return the actor for the actual location.
          */
         actor.onDelete();
         this.breakpointStore.removeBreakpoint(aLocation);
--- a/toolkit/devtools/server/tests/unit/test_breakpointstore.js
+++ b/toolkit/devtools/server/tests/unit/test_breakpointstore.js
@@ -7,24 +7,62 @@
 function run_test()
 {
   Cu.import("resource://gre/modules/jsdebugger.jsm");
   addDebuggerToGlobal(this);
   let loader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
     .getService(Components.interfaces.mozIJSSubScriptLoader);
   loader.loadSubScript("resource://gre/modules/devtools/server/actors/script.js");
 
+  test_has_breakpoint();
   test_bug_754251();
   test_add_breakpoint();
   test_remove_breakpoint();
   test_find_breakpoints();
 }
 
+function test_has_breakpoint() {
+  let bpStore = new BreakpointStore();
+  let location = {
+    url: "http://example.com/foo.js",
+    line: 3
+  };
+  let columnLocation = {
+    url: "http://example.com/bar.js",
+    line: 5,
+    column: 15
+  };
+
+  // Shouldn't have breakpoint
+  do_check_eq(null, bpStore.hasBreakpoint(location),
+              "Breakpoint not added and shouldn't exist.");
+
+  bpStore.addBreakpoint(location);
+  do_check_true(!!bpStore.hasBreakpoint(location),
+                "Breakpoint added but not found in Breakpoint Store.");
+
+  bpStore.removeBreakpoint(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.hasBreakpoint(columnLocation),
+              "Breakpoint with column not added and shouldn't exist.");
+
+  bpStore.addBreakpoint(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.hasBreakpoint(columnLocation),
+              "Breakpoint with column removed but still exists in Breakpoint Store.");
+}
+
 // Note: Removing this test will regress bug 754251. See comment above
-// ThreadActor._breakpointStore.
+// ThreadActor.breakpointStore.
 function test_bug_754251() {
   let instance1 = new ThreadActor();
   let instance2 = new ThreadActor();
   do_check_true(instance1.breakpointStore instanceof BreakpointStore);
   do_check_eq(instance1.breakpointStore, ThreadActor.breakpointStore);
   do_check_eq(instance2.breakpointStore, ThreadActor.breakpointStore);
 }
 
@@ -32,50 +70,50 @@ function test_add_breakpoint() {
   // Breakpoint with column
   let bpStore = new BreakpointStore();
   let location = {
     url: "http://example.com/foo.js",
     line: 10,
     column: 9
   };
   bpStore.addBreakpoint(location);
-  do_check_true(!!bpStore.getBreakpoint(location, false),
+  do_check_true(!!bpStore.hasBreakpoint(location),
                 "We should have the column breakpoint we just added");
 
   // Breakpoint without column (whole line breakpoint)
   location = {
     url: "http://example.com/bar.js",
     line: 103
   };
   bpStore.addBreakpoint(location);
-  do_check_true(!!bpStore.getBreakpoint(location, false),
+  do_check_true(!!bpStore.hasBreakpoint(location),
                 "We should have the whole line breakpoint we just added");
 }
 
 function test_remove_breakpoint() {
   // Breakpoint with column
   let bpStore = new BreakpointStore();
   let location = {
     url: "http://example.com/foo.js",
     line: 10,
     column: 9
   };
   bpStore.addBreakpoint(location);
   bpStore.removeBreakpoint(location);
-  do_check_eq(bpStore.getBreakpoint(location, false), null,
+  do_check_eq(bpStore.hasBreakpoint(location), null,
               "We should not have the column breakpoint anymore");
 
   // Breakpoint without column (whole line breakpoint)
   location = {
     url: "http://example.com/bar.js",
     line: 103
   };
   bpStore.addBreakpoint(location);
   bpStore.removeBreakpoint(location);
-  do_check_eq(bpStore.getBreakpoint(location, false), null,
+  do_check_eq(bpStore.hasBreakpoint(location), null,
               "We should not have the whole line breakpoint anymore");
 }
 
 function test_find_breakpoints() {
   let bps = [
     { url: "foo.js", line: 10 },
     { url: "foo.js", line: 10, column: 3 },
     { url: "foo.js", line: 10, column: 10 },