Backed out changeset 2b48d8dcac21 (bug 995252) for linux debug mochitest-dt failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Thu, 01 May 2014 17:05:34 -0400
changeset 181680 4021cb194737c6dd7ddde42a12e3bbf8805b33cd
parent 181679 cfd5a38a44c58075bfc12e2db3d501d9e9f86d16
child 181681 4fbc58906fac211908c724083a5634b0b0742b5a
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
bugs995252
milestone32.0a1
backs out2b48d8dcac21190c20a9e0e146e7abe1106ea76d
Backed out changeset 2b48d8dcac21 (bug 995252) for linux debug mochitest-dt failures.
browser/devtools/debugger/debugger-controller.js
browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js
browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js
browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js
toolkit/devtools/client/dbg-client.jsm
toolkit/devtools/server/actors/script.js
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -85,17 +85,16 @@ const FRAME_TYPE = {
   CONDITIONAL_BREAKPOINT_EVAL: 1,
   WATCH_EXPRESSIONS_EVAL: 2,
   PUBLIC_CLIENT_EVAL: 3
 };
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 Cu.import("resource://gre/modules/devtools/event-emitter.js");
-Cu.import("resource://gre/modules/Task.jsm");
 Cu.import("resource:///modules/devtools/SimpleListWidget.jsm");
 Cu.import("resource:///modules/devtools/BreadcrumbsWidget.jsm");
 Cu.import("resource:///modules/devtools/SideMenuWidget.jsm");
 Cu.import("resource:///modules/devtools/VariablesView.jsm");
 Cu.import("resource:///modules/devtools/VariablesViewController.jsm");
 Cu.import("resource:///modules/devtools/ViewHelpers.jsm");
 
 const require = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}).devtools.require;
@@ -605,22 +604,22 @@ StackFrames.prototype = {
     if (!client.mainRoot.traits.conditionalBreakpoints) {
       // Conditional breakpoints are { breakpoint, expression } tuples. The
       // boolean evaluation of the expression decides if the active thread
       // automatically resumes execution or not.
       if (breakLocation) {
         // Make sure a breakpoint actually exists at the specified url and line.
         let breakpointPromise = DebuggerController.Breakpoints._getAdded(breakLocation);
         if (breakpointPromise) {
-          waitForNextPause = true;
           breakpointPromise.then(({ conditionalExpression: e }) => { if (e) {
             // Evaluating the current breakpoint's conditional expression will
             // cause the stack frames to be cleared and active thread to pause,
             // sending a 'clientEvaluated' packed and adding the frames again.
             this.evaluate(e, { depth: 0, meta: FRAME_TYPE.CONDITIONAL_BREAKPOINT_EVAL });
+            waitForNextPause = true;
           }});
         }
       }
       // We'll get our evaluation of the current breakpoint's conditional
       // expression the next time the thread client pauses...
       if (waitForNextPause) {
         return;
       }
@@ -1831,94 +1830,90 @@ Breakpoints.prototype = {
    *        Additional options or flags supported by this operation:
    *          - openPopup: tells if the expression popup should be shown.
    *          - noEditorUpdate: tells if you want to skip editor updates.
    *          - noPaneUpdate: tells if you want to skip breakpoint pane updates.
    * @return object
    *         A promise that is resolved after the breakpoint is added, or
    *         rejected if there was an error.
    */
-  addBreakpoint: Task.async(function*(aLocation, aOptions = {}) {
+  addBreakpoint: function(aLocation, aOptions = {}) {
     // Make sure a proper location is available.
     if (!aLocation) {
-      throw new Error("Invalid breakpoint location.");
+      return promise.reject(new Error("Invalid breakpoint location."));
     }
-    let addedPromise, removingPromise;
 
     // If the breakpoint was already added, or is currently being added at the
     // specified location, then return that promise immediately.
-    if ((addedPromise = this._getAdded(aLocation))) {
+    let addedPromise = this._getAdded(aLocation);
+    if (addedPromise) {
       return addedPromise;
     }
 
     // If the breakpoint is currently being removed from the specified location,
-    // then wait for that to finish.
-    if ((removingPromise = this._getRemoving(aLocation))) {
-      yield removingPromise;
+    // then wait for that to finish and retry afterwards.
+    let removingPromise = this._getRemoving(aLocation);
+    if (removingPromise) {
+      return removingPromise.then(() => this.addBreakpoint(aLocation, aOptions));
     }
 
     let deferred = promise.defer();
 
     // Remember the breakpoint initialization promise in the store.
     let identifier = this.getIdentifier(aLocation);
     this._added.set(identifier, deferred.promise);
 
     // Try adding the breakpoint.
-    gThreadClient.setBreakpoint(aLocation, Task.async(function*(aResponse, aBreakpointClient) {
+    gThreadClient.setBreakpoint(aLocation, (aResponse, aBreakpointClient) => {
       // If the breakpoint response has an "actualLocation" attached, then
       // the original requested placement for the breakpoint wasn't accepted.
       if (aResponse.actualLocation) {
         // Remember the initialization promise for the new location instead.
         let oldIdentifier = identifier;
         let newIdentifier = identifier = this.getIdentifier(aResponse.actualLocation);
         this._added.delete(oldIdentifier);
         this._added.set(newIdentifier, deferred.promise);
+
+        // Store the originally requested location in case it's ever needed
+        // and update the breakpoint client with the actual location.
+        aBreakpointClient.requestedLocation = aLocation;
+        aBreakpointClient.location = aResponse.actualLocation;
       }
 
       // By default, new breakpoints are always enabled. Disabled breakpoints
       // are, in fact, removed from the server but preserved in the frontend,
       // so that they may not be forgotten across target navigations.
       let disabledPromise = this._disabled.get(identifier);
       if (disabledPromise) {
-        let aPrevBreakpointClient = yield disabledPromise;
-        let condition = aPrevBreakpointClient.getCondition();
+        disabledPromise.then((aPrevBreakpointClient) => {
+          let condition = aPrevBreakpointClient.getCondition();
+          if (condition) {
+            aBreakpointClient.setCondition(gThreadClient, condition);
+          }
+        });
         this._disabled.delete(identifier);
-
-        if (condition) {
-          aBreakpointClient = yield aBreakpointClient.setCondition(
-            gThreadClient,
-            condition
-          );
-        }
-      }
-
-      if (aResponse.actualLocation) {
-        // Store the originally requested location in case it's ever needed
-        // and update the breakpoint client with the actual location.
-        aBreakpointClient.requestedLocation = aLocation;
-        aBreakpointClient.location = aResponse.actualLocation;
       }
 
       // Preserve information about the breakpoint's line text, to display it
       // in the sources pane without requiring fetching the source (for example,
       // after the target navigated). Note that this will get out of sync
       // if the source text contents change.
       let line = aBreakpointClient.location.line - 1;
       aBreakpointClient.text = DebuggerView.editor.getText(line).trim();
 
       // Show the breakpoint in the editor and breakpoints pane, and resolve.
       this._showBreakpoint(aBreakpointClient, aOptions);
 
       // Notify that we've added a breakpoint.
       window.emit(EVENTS.BREAKPOINT_ADDED, aBreakpointClient);
       deferred.resolve(aBreakpointClient);
-    }.bind(this)));
+    });
 
     return deferred.promise;
-  }),
+  },
 
   /**
    * Remove a breakpoint.
    *
    * @param object aLocation
    *        @see DebuggerController.Breakpoints.addBreakpoint
    * @param object aOptions [optional]
    *        @see DebuggerController.Breakpoints.addBreakpoint
@@ -2032,24 +2027,22 @@ Breakpoints.prototype = {
    */
   updateCondition: function(aLocation, aCondition) {
     let addedPromise = this._getAdded(aLocation);
     if (!addedPromise) {
       return promise.reject(new Error('breakpoint does not exist ' +
                                       'in specified location'));
     }
 
-    var promise = addedPromise.then(aBreakpointClient => {
+    return addedPromise.then(aBreakpointClient => {
       return aBreakpointClient.setCondition(gThreadClient, aCondition);
+    }, err => {
+      DevToolsUtils.reportException("Breakpoints.prototype.updateCondition",
+                                    err);
     });
-
-    // `setCondition` returns a new breakpoint that has the condition,
-    // so we need to update the store
-    this._added.set(this.getIdentifier(aLocation), promise);
-    return promise;
   },
 
   /**
    * Update the editor and breakpoints pane to show a specified breakpoint.
    *
    * @param object aBreakpointData
    *        Information about the breakpoint to be shown.
    *        This object must have the following properties:
--- a/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js
+++ b/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js
@@ -21,18 +21,17 @@ function test() {
 
     waitForSourceShown(gPanel, "-01.js")
       .then(addBreakpoints)
       .then(() => clickBreakpointAndCheck(0, 0, 5))
       .then(() => clickBreakpointAndCheck(1, 1, 6))
       .then(() => clickBreakpointAndCheck(2, 1, 7))
       .then(() => clickBreakpointAndCheck(3, 1, 8))
       .then(() => clickBreakpointAndCheck(4, 1, 9))
-      .then(() => ensureThreadClientState(gPanel, "resumed"))
-      .then(() => closeDebuggerAndFinish(gPanel))
+      .then(() => resumeDebuggerThenCloseAndFinish(gPanel))
       .then(null, aError => {
         ok(false, "Got an error: " + aError.message + "\n" + aError.stack);
       });
   });
 
   function addBreakpoints() {
     return promise.resolve(null)
       .then(() => initialChecks(0, 1))
--- a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js
+++ b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js
@@ -50,17 +50,17 @@ function test() {
     gDebuggee.ermahgerd();
   });
 
   function addBreakpoint() {
     return gPanel.addBreakpoint(gLocation);
   }
 
   function setConditional(aClient) {
-    return gBreakpoints.updateCondition(aClient.location, "hello");
+    aClient.condition = "hello";
   }
 
   function toggleBreakpoint() {
     EventUtils.sendMouseEvent({ type: "click" },
       gDebugger.document.querySelector(".dbg-breakpoint-checkbox"),
       gDebugger);
   }
 
--- a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js
+++ b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js
@@ -53,17 +53,17 @@ function test() {
 
   function addBreakpoint() {
     return gPanel.addBreakpoint(gLocation);
   }
 
   function setDummyConditional(aClient) {
     // This happens when a conditional expression input popup is shown
     // but the user doesn't type anything into it.
-    return gBreakpoints.updateCondition(aClient.location, '');
+    aClient.condition = "";
   }
 
   function toggleBreakpoint() {
     EventUtils.sendMouseEvent({ type: "click" },
       gDebugger.document.querySelector(".dbg-breakpoint-checkbox"),
       gDebugger);
   }
 
--- a/toolkit/devtools/client/dbg-client.jsm
+++ b/toolkit/devtools/client/dbg-client.jsm
@@ -1432,20 +1432,21 @@ ThreadClient.prototype = {
         if (aOnResponse) {
           let root = this.client.mainRoot;
           let bpClient = new BreakpointClient(
             this.client,
             aResponse.actor,
             location,
             root.traits.conditionalBreakpoints ? condition : undefined
           );
-          aOnResponse(aResponse, bpClient);
-        }
-        if (aCallback) {
-          aCallback();
+          if (aCallback) {
+            aCallback(aOnResponse(aResponse, bpClient));
+          } else {
+            aOnResponse(aResponse, bpClient);
+          }
         }
       }.bind(this));
     }.bind(this);
 
     // If the debuggee is paused, just set the breakpoint.
     if (this.paused) {
       doSetBreakpoint();
       return;
@@ -2252,45 +2253,29 @@ BreakpointClient.prototype = {
   setCondition: function(gThreadClient, aCondition) {
     let root = this._client.mainRoot;
     let deferred = promise.defer();
 
     if (root.traits.conditionalBreakpoints) {
       let info = {
         url: this.location.url,
         line: this.location.line,
-        column: this.location.column,
         condition: aCondition
       };
-
-      // Remove the current breakpoint and add a new one with the
-      // condition.
-      this.remove(aResponse => {
-        if (aResponse && aResponse.error) {
+      gThreadClient.setBreakpoint(info, (aResponse, ignoredBreakpoint) => {
+        if(aResponse && aResponse.error) {
           deferred.reject(aResponse);
-          return;
+        } else {
+          this.condition = aCondition;
+          deferred.resolve(null);
         }
-
-        gThreadClient.setBreakpoint(info, (aResponse, aNewBreakpoint) => {
-          if (aResponse && aResponse.error) {
-            deferred.reject(aResponse);
-          } else {
-            deferred.resolve(aNewBreakpoint);
-          }
-        });
       });
     } else {
-      // The property shouldn't even exist if the condition is blank
-      if(aCondition === "") {
-        delete this.conditionalExpression;
-      }
-      else {
-        this.conditionalExpression = aCondition;
-      }
-      deferred.resolve(this);
+      this.conditionalExpression = aCondition;
+      deferred.resolve(null);
     }
 
     return deferred.promise;
   }
 };
 
 eventSource(BreakpointClient.prototype);
 
--- a/toolkit/devtools/server/actors/script.js
+++ b/toolkit/devtools/server/actors/script.js
@@ -109,26 +109,34 @@ BreakpointStore.prototype = {
 
     if (column != null) {
       if (!this._breakpoints[url]) {
         this._breakpoints[url] = [];
       }
       if (!this._breakpoints[url][line]) {
         this._breakpoints[url][line] = [];
       }
+      if(this._breakpoints[url][line][column]) {
+        updating = true;
+      }
       this._breakpoints[url][line][column] = aBreakpoint;
     } else {
       // Add a breakpoint that breaks on the whole line.
       if (!this._wholeLineBreakpoints[url]) {
         this._wholeLineBreakpoints[url] = [];
       }
+      if(this._wholeLineBreakpoints[url][line]) {
+        updating = true;
+      }
       this._wholeLineBreakpoints[url][line] = aBreakpoint;
     }
 
-    this._size++;
+    if (!updating) {
+      this._size++;
+    }
   },
 
   /**
    * Remove a breakpoint from the breakpoint store.
    *
    * @param Object aBreakpoint
    *        The breakpoint to be removed. It is an object with the following
    *        properties: