Backed out changeset a4c0703e9a10 (bug 1008380) for browser_dbg_breakpoints-actual-location.js failures.
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 03 Jun 2015 13:34:46 -0400
changeset 247123 70fa9da159a08a2a057747060a7ad7df1d31f235
parent 247122 b617a57d6bf1a18d3f487be2f989feef3dc4a072
child 247124 2923c5cc5dcc87f4fd6e186bba163d3f0094820f
push id28854
push userryanvm@gmail.com
push dateThu, 04 Jun 2015 13:24:20 +0000
treeherdermozilla-central@5b4c240e1a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1008380
milestone41.0a1
backs outa4c0703e9a107ce9ec67cb6ea175ba19c5e77bed
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
Backed out changeset a4c0703e9a10 (bug 1008380) for browser_dbg_breakpoints-actual-location.js failures.
browser/devtools/debugger/debugger-controller.js
browser/devtools/debugger/test/browser_dbg_breakpoints-actual-location2.js
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -1905,16 +1905,26 @@ Breakpoints.prototype = {
     let actor = DebuggerView.Sources.selectedValue;
     let location = { actor: actor, line: aLine + 1 };
 
     // Initialize the breakpoint, but don't update the editor, since this
     // callback is invoked because a breakpoint was added in the
     // editor itself.
     let breakpointClient = yield this.addBreakpoint(location, { noEditorUpdate: true });
 
+    // If the breakpoint client has a "requestedLocation" attached, then
+    // the original requested placement for the breakpoint wasn't accepted.
+    // In this case, we need to update the editor with the new location.
+    if (breakpointClient.requestedLocation) {
+      DebuggerView.editor.moveBreakpoint(
+        breakpointClient.requestedLocation.line - 1,
+        breakpointClient.location.line - 1
+      );
+    }
+
     // Notify that we've shown a breakpoint in the source editor.
     window.emit(EVENTS.BREAKPOINT_SHOWN_IN_EDITOR);
   }),
 
   /**
    * Event handler for breakpoints that are removed from the editor.
    *
    * @param number aLine
@@ -2015,50 +2025,21 @@ Breakpoints.prototype = {
 
     let source = gThreadClient.source(
       DebuggerView.Sources.getItemByValue(aLocation.actor).attachment.source
     );
 
     source.setBreakpoint(aLocation, Task.async(function*(aResponse, aBreakpointClient) {
       // If the breakpoint response has an "actualLocation" attached, then
       // the original requested placement for the breakpoint wasn't accepted.
-      let actualLocation = aResponse.actualLocation;
-      if (actualLocation) {
-        // Update the editor to reflect the new location of the breakpoint. We
-        // always need to do this, even when we already have a breakpoint for
-        // the actual location, because the editor already as already shown the
-        // breakpoint at the original location at this point. Calling
-        // moveBreakpoint will hide the breakpoint at the original location, and
-        // show it at the actual location, if necessary.
-        //
-        // FIXME: The call to moveBreakpoint triggers another call to remove-
-        // and addBreakpoint, respectively. These calls do not have any effect,
-        // because there is no breakpoint to remove at the old location, and
-        // the breakpoint is already being added at the new location, but they
-        // are redundant and confusing.
-        DebuggerView.editor.moveBreakpoint(
-          aBreakpointClient.location.line - 1,
-          actualLocation.line - 1
-        );
-
-        aBreakpointClient.location = actualLocation;
-        aBreakpointClient.location.actor = actualLocation.source
-                                         ? actualLocation.source.actor
-                                         : null;
-
+      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);
-
-        if ((addedPromise = this._getAdded(actualLocation))) {
-          deferred.resolve(addedPromise);
-          return;
-        }
-
-        // Remember the initialization promise for the new location instead.
-        let newIdentifier = identifier = this.getIdentifier(actualLocation);
         this._added.set(newIdentifier, deferred.promise);
       }
 
       // 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) {
@@ -2069,24 +2050,34 @@ Breakpoints.prototype = {
         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.
+        let actualLoc = aResponse.actualLocation;
+        aBreakpointClient.requestedLocation = aLocation;
+        aBreakpointClient.location = actualLoc;
+        aBreakpointClient.location.actor = actualLoc.source ? actualLoc.source.actor : null;
+      }
+
       // 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 breakpoints pane, and resolve.
+      // Show the breakpoint in the editor and breakpoints pane, and
+      // resolve.
       yield 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;
--- a/browser/devtools/debugger/test/browser_dbg_breakpoints-actual-location2.js
+++ b/browser/devtools/debugger/test/browser_dbg_breakpoints-actual-location2.js
@@ -44,17 +44,16 @@ function test() {
         actor: gSources.selectedValue,
         line: 20
       });
 
       let movedBpClient = yield gPanel.addBreakpoint({
         actor: gSources.selectedValue,
         line: 17
       });
-
       testMovedLocation(movedBpClient);
 
       yield resumeAndTestBreakpoint(19);
 
       yield gPanel.removeBreakpoint({
         actor: gSources.selectedValue,
         line: 19
       });
@@ -89,10 +88,15 @@ function test() {
 
   function testMovedLocation(breakpointClient) {
     ok(breakpointClient,
       "Breakpoint added, client received.");
     is(breakpointClient.location.actor, gSources.selectedValue,
       "Breakpoint client url is the same.");
     is(breakpointClient.location.line, 19,
       "Breakpoint client line is new.");
+
+    is(breakpointClient.requestedLocation.actor, gSources.selectedValue,
+      "Requested location url is correct");
+    is(breakpointClient.requestedLocation.line, 17,
+      "Requested location line is correct");
   }
 }