Don't call ensureIndexIsVisible with an out of bounds argument and fix a race in browser_dbg_variables-view-edit-getset-02.js (bug 1013941). r=vporof
☠☠ backed out by 9ffcc4c89435 ☠ ☠
authorPanos Astithas <past@mozilla.com>
Tue, 29 Jul 2014 16:14:34 +0300
changeset 196818 d3d2b622400feda2ac07489f80605c926298ad3f
parent 196817 bf57c733f1fed786ee52b38f407234d97ed96bf9
child 196819 a2b0f17e7dbaf54772989a964a36d0d5b3c05675
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersvporof
bugs1013941
milestone34.0a1
Don't call ensureIndexIsVisible with an out of bounds argument and fix a race in browser_dbg_variables-view-edit-getset-02.js (bug 1013941). r=vporof
browser/devtools/debugger/debugger-controller.js
browser/devtools/debugger/debugger-toolbar.js
browser/devtools/debugger/test/browser_dbg_variables-view-edit-getset-02.js
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -656,17 +656,16 @@ StackFrames.prototype = {
       let { depth, where: { url, line }, source } = frame;
       let isBlackBoxed = source ? this.activeThread.source(source).isBlackBoxed : false;
       let location = NetworkHelper.convertToUnicode(unescape(url));
       let title = StackFrameUtils.getFrameTitle(frame);
       DebuggerView.StackFrames.addFrame(title, location, line, depth, isBlackBoxed);
     }
 
     DebuggerView.StackFrames.selectedDepth = Math.max(this.currentFrameDepth, 0);
-    DebuggerView.StackFrames.dirty = this.activeThread.moreFrames;
 
     window.emit(EVENTS.AFTER_FRAMES_REFILLED);
   },
 
   /**
    * Handler for the thread client's framescleared notification.
    */
   _onFramesCleared: function() {
--- a/browser/devtools/debugger/debugger-toolbar.js
+++ b/browser/devtools/debugger/debugger-toolbar.js
@@ -467,21 +467,16 @@ StackFramesView.prototype = Heritage.ext
    * the bottommost, not topmost.
    * @return number
    */
   get selectedDepth() {
     return this.selectedItem.attachment.depth;
   },
 
   /**
-   * Specifies if the active thread has more frames that need to be loaded.
-   */
-  dirty: false,
-
-  /**
    * Customization function for creating an item's UI.
    *
    * @param string aTitle
    *        The frame title to be displayed in the list.
    * @param string aUrl
    *        The frame source url.
    * @param string aLine
    *        The frame line number.
@@ -556,35 +551,41 @@ StackFramesView.prototype = Heritage.ext
     }
   },
 
   /**
    * The scroll listener for the stackframes container.
    */
   _onScroll: function() {
     // Update the stackframes container only if we have to.
-    if (!this.dirty) {
+    if (!DebuggerController.activeThread.moreFrames) {
       return;
     }
     // Allow requests to settle down first.
     setNamedTimeout("stack-scroll", STACK_FRAMES_SCROLL_DELAY, this._afterScroll);
   },
 
   /**
    * Requests the addition of more frames from the controller.
    */
   _afterScroll: function() {
+    // Check again if we have to update the stackframes container, because in
+    // some cases (e.g. browser_dbg_variables-view-edit-getset-02.js) the value
+    // might have changed from the time the setNamedTimeout call was made.
+    if (!DebuggerController.activeThread.moreFrames) {
+      return;
+    }
     let scrollPosition = this.widget.getAttribute("scrollPosition");
     let scrollWidth = this.widget.getAttribute("scrollWidth");
 
     // If the stackframes container scrolled almost to the end, with only
     // 1/10 of a breadcrumb remaining, load more content.
     if (scrollPosition - scrollWidth / 10 < 1) {
-      this.ensureIndexIsVisible(CALL_STACK_PAGE_SIZE - 1);
-      this.dirty = false;
+      let index = Math.min(CALL_STACK_PAGE_SIZE - 1, this.items.length - 1);
+      this.ensureIndexIsVisible(index);
 
       // Loads more stack frames from the debugger server cache.
       DebuggerController.StackFrames.addMoreFrames();
     }
   },
 
   _mirror: null,
   _prevBlackBoxedUrl: null
--- a/browser/devtools/debugger/test/browser_dbg_variables-view-edit-getset-02.js
+++ b/browser/devtools/debugger/test/browser_dbg_variables-view-edit-getset-02.js
@@ -1,105 +1,84 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 /**
  * Make sure that the variables view is able to override getter properties
  * to plain value properties.
  */
 
-const TAB_URL = EXAMPLE_URL + "doc_frame-parameters.html";
+function test() {
+  const TAB_URL = EXAMPLE_URL + "doc_frame-parameters.html";
 
-let gTab, gDebuggee, gPanel, gDebugger;
-let gL10N, gEditor, gVars, gWatch;
-
-function test() {
   // Debug test slaves are a bit slow at this test.
   requestLongerTimeout(2);
 
-  initDebugger(TAB_URL).then(([aTab, aDebuggee, aPanel]) => {
-    gTab = aTab;
-    gDebuggee = aDebuggee;
-    gPanel = aPanel;
-    gDebugger = gPanel.panelWin;
-    gL10N = gDebugger.L10N;
-    gEditor = gDebugger.DebuggerView.editor;
-    gVars = gDebugger.DebuggerView.Variables;
-    gWatch = gDebugger.DebuggerView.WatchExpressions;
-
-    gVars.switch = function() {};
-    gVars.delete = function() {};
+  Task.spawn(function* () {
+    let [, debuggee, panel] = yield initDebugger(TAB_URL);
+    let win = panel.panelWin;
+    let L10N = win.L10N;
+    let editor = win.DebuggerView.editor;
+    let vars = win.DebuggerView.Variables;
+    let watch = win.DebuggerView.WatchExpressions;
 
-    waitForSourceAndCaretAndScopes(gPanel, ".html", 24)
-      .then(() => addWatchExpression())
-      .then(() => testEdit("\"xlerb\"", "xlerb"))
-      .then(() => resumeDebuggerThenCloseAndFinish(gPanel))
-      .then(null, aError => {
-        ok(false, "Got an error: " + aError.message + "\n" + aError.stack);
-      });
-
-    EventUtils.sendMouseEvent({ type: "click" },
-      gDebuggee.document.querySelector("button"),
-      gDebuggee);
-  });
-}
+    vars.switch = function() {};
+    vars.delete = function() {};
 
-function addWatchExpression() {
-  let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_WATCH_EXPRESSIONS);
-
-  gWatch.addExpression("myVar.prop");
-  gEditor.focus();
-
-  return finished;
-}
-
-function testEdit(aString, aExpected) {
-  let localScope = gVars.getScopeAtIndex(1);
-  let myVar = localScope.get("myVar");
+    let paused = waitForSourceAndCaretAndScopes(panel, ".html", 24);
+    // Spin the event loop before causing the debuggee to pause, to allow
+    // this function to return first.
+    executeSoon(() => {
+      EventUtils.sendMouseEvent({ type: "click" },
+        debuggee.document.querySelector("button"),
+        debuggee);
+    });
+    yield paused;
 
-  let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_PROPERTIES).then(() => {
-    let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_WATCH_EXPRESSIONS).then(() => {
-      let exprScope = gVars.getScopeAtIndex(0);
+    // Add a watch expression for direct observation of the value change.
+    let addedWatch = waitForDebuggerEvents(panel, win.EVENTS.FETCHED_WATCH_EXPRESSIONS);
+    watch.addExpression("myVar.prop");
+    editor.focus();
+    yield addedWatch;
 
-      ok(exprScope,
-        "There should be a wach expressions scope in the variables view.");
-      is(exprScope.name, gL10N.getStr("watchExpressionsScopeLabel"),
-        "The scope's name should be marked as 'Watch Expressions'.");
-      is(exprScope._store.size, 1,
-        "There should be one evaluation available.");
+    // Scroll myVar into view.
+    vars.focusLastVisibleItem();
 
-      is(exprScope.get("myVar.prop").value, aExpected,
-        "The expression value is correct after the edit.");
-    });
+    let localScope = vars.getScopeAtIndex(1);
+    let myVar = localScope.get("myVar");
+
+    myVar.expand();
+    vars.clearHierarchy();
+
+    yield waitForDebuggerEvents(panel, win.EVENTS.FETCHED_PROPERTIES);
 
     let editTarget = myVar.get("prop").target;
 
     // Allow the target variable to get painted, so that clicking on
     // its value would scroll the new textbox node into view.
     executeSoon(() => {
       let varEdit = editTarget.querySelector(".title > .variables-view-edit");
-      EventUtils.sendMouseEvent({ type: "mousedown" }, varEdit, gDebugger);
+      EventUtils.sendMouseEvent({ type: "mousedown" }, varEdit, win);
 
       let varInput = editTarget.querySelector(".title > .element-value-input");
-      setText(varInput, aString);
-      EventUtils.sendKey("RETURN", gDebugger);
+      setText(varInput, "\"xlerb\"");
+      EventUtils.sendKey("RETURN", win);
     });
 
-    return finished;
-  });
+    yield waitForDebuggerEvents(panel, win.EVENTS.FETCHED_WATCH_EXPRESSIONS);
 
-  myVar.expand();
-  gVars.clearHierarchy();
-
-  return finished;
-}
+    let exprScope = vars.getScopeAtIndex(0);
 
-registerCleanupFunction(function() {
-  gTab = null;
-  gDebuggee = null;
-  gPanel = null;
-  gDebugger = null;
-  gL10N = null;
-  gEditor = null;
-  gVars = null;
-  gWatch = null;
-});
+    ok(exprScope,
+      "There should be a wach expressions scope in the variables view.");
+    is(exprScope.name, L10N.getStr("watchExpressionsScopeLabel"),
+      "The scope's name should be marked as 'Watch Expressions'.");
+    is(exprScope._store.size, 1,
+      "There should be one evaluation available.");
 
+    is(exprScope.get("myVar.prop").value, "xlerb",
+      "The expression value is correct after the edit.");
+
+    yield resumeDebuggerThenCloseAndFinish(panel);
+  }).then(null, aError => {
+      ok(false, "Got an error: " + aError.message + "\n" + aError.stack);
+  });
+}