From 87e88accfb6daece78bd5d18828b7faa660a126a Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Thu, 1 May 2014 17:05:34 -0400 Subject: [PATCH] Backed out changeset 2b48d8dcac21 (bug 995252) for linux debug mochitest-dt failures. --- .../devtools/debugger/debugger-controller.js | 61 ++++++++----------- .../test/browser_dbg_breakpoints-highlight.js | 3 +- .../browser_dbg_server-conditional-bp-03.js | 2 +- .../browser_dbg_server-conditional-bp-04.js | 2 +- toolkit/devtools/client/dbg-client.jsm | 39 ++++-------- toolkit/devtools/server/actors/script.js | 10 ++- 6 files changed, 51 insertions(+), 66 deletions(-) diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index 3d2aa85c4a9f..29f2aebf8f61 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -90,7 +90,6 @@ const FRAME_TYPE = { 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"); @@ -610,12 +609,12 @@ StackFrames.prototype = { // 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; }}); } } @@ -1836,23 +1835,24 @@ Breakpoints.prototype = { * 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(); @@ -1862,7 +1862,7 @@ Breakpoints.prototype = { 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) { @@ -1871,6 +1871,11 @@ Breakpoints.prototype = { 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 @@ -1878,23 +1883,13 @@ Breakpoints.prototype = { // 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 @@ -1910,10 +1905,10 @@ Breakpoints.prototype = { // Notify that we've added a breakpoint. window.emit(EVENTS.BREAKPOINT_ADDED, aBreakpointClient); deferred.resolve(aBreakpointClient); - }.bind(this))); + }); return deferred.promise; - }), + }, /** * Remove a breakpoint. @@ -2037,14 +2032,12 @@ Breakpoints.prototype = { '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; }, /** diff --git a/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js b/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js index c0ddc30ee254..0a3418d06473 100644 --- a/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js +++ b/browser/devtools/debugger/test/browser_dbg_breakpoints-highlight.js @@ -26,8 +26,7 @@ function test() { .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); }); diff --git a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js index c9dc2f2d2b62..86121b7ff1d1 100644 --- a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js +++ b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-03.js @@ -55,7 +55,7 @@ function test() { } function setConditional(aClient) { - return gBreakpoints.updateCondition(aClient.location, "hello"); + aClient.condition = "hello"; } function toggleBreakpoint() { diff --git a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js index 45809f7aa74f..19803785add5 100644 --- a/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js +++ b/browser/devtools/debugger/test/browser_dbg_server-conditional-bp-04.js @@ -58,7 +58,7 @@ function test() { 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() { diff --git a/toolkit/devtools/client/dbg-client.jsm b/toolkit/devtools/client/dbg-client.jsm index 0f911f57fc65..eea6b3b2604c 100644 --- a/toolkit/devtools/client/dbg-client.jsm +++ b/toolkit/devtools/client/dbg-client.jsm @@ -1437,10 +1437,11 @@ ThreadClient.prototype = { 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); @@ -2257,35 +2258,19 @@ BreakpointClient.prototype = { 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; diff --git a/toolkit/devtools/server/actors/script.js b/toolkit/devtools/server/actors/script.js index b1ab269e177c..0d8201ca1307 100644 --- a/toolkit/devtools/server/actors/script.js +++ b/toolkit/devtools/server/actors/script.js @@ -114,16 +114,24 @@ BreakpointStore.prototype = { 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++; + } }, /**