From 42210fb77333414bdfe06e67da23edfd23183e2e Mon Sep 17 00:00:00 2001 From: Panos Astithas Date: Tue, 29 Jul 2014 16:14:34 +0300 Subject: [PATCH] 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 --HG-- extra : rebase_source : bc7c1a5dc8088f13dd0089faf164f070297b803f --- .../devtools/debugger/debugger-controller.js | 1 - browser/devtools/debugger/debugger-toolbar.js | 17 +-- ...owser_dbg_variables-view-edit-getset-02.js | 133 ++++++++---------- 3 files changed, 65 insertions(+), 86 deletions(-) diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index 231ae1ac0794..7c2b622d644e 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -661,7 +661,6 @@ StackFrames.prototype = { } DebuggerView.StackFrames.selectedDepth = Math.max(this.currentFrameDepth, 0); - DebuggerView.StackFrames.dirty = this.activeThread.moreFrames; window.emit(EVENTS.AFTER_FRAMES_REFILLED); }, diff --git a/browser/devtools/debugger/debugger-toolbar.js b/browser/devtools/debugger/debugger-toolbar.js index 804274b3e776..641632ddd76c 100644 --- a/browser/devtools/debugger/debugger-toolbar.js +++ b/browser/devtools/debugger/debugger-toolbar.js @@ -471,11 +471,6 @@ StackFramesView.prototype = Heritage.extend(WidgetMethods, { 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. * @@ -561,7 +556,7 @@ StackFramesView.prototype = Heritage.extend(WidgetMethods, { */ _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. @@ -572,14 +567,20 @@ StackFramesView.prototype = Heritage.extend(WidgetMethods, { * 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(); diff --git 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 index d86304bec85a..36f395535f87 100644 --- 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 @@ -6,69 +6,49 @@ * to plain value properties. */ -const TAB_URL = EXAMPLE_URL + "doc_frame-parameters.html"; - -let gTab, gDebuggee, gPanel, gDebugger; -let gL10N, gEditor, gVars, gWatch; - function test() { + const TAB_URL = EXAMPLE_URL + "doc_frame-parameters.html"; + // 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; + 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; - gVars.switch = function() {}; - gVars.delete = function() {}; + vars.switch = function() {}; + vars.delete = function() {}; - 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); - }); -} - -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 finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_PROPERTIES).then(() => { - let finished = waitForDebuggerEvents(gPanel, gDebugger.EVENTS.FETCHED_WATCH_EXPRESSIONS).then(() => { - let exprScope = gVars.getScopeAtIndex(0); - - 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."); - - is(exprScope.get("myVar.prop").value, aExpected, - "The expression value is correct after the edit."); + 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; + + // 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; + + // Scroll myVar into view. + vars.focusLastVisibleItem(); + + 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; @@ -76,30 +56,29 @@ function testEdit(aString, aExpected) { // 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); + + let exprScope = vars.getScopeAtIndex(0); + + 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); }); - - myVar.expand(); - gVars.clearHierarchy(); - - return finished; } - -registerCleanupFunction(function() { - gTab = null; - gDebuggee = null; - gPanel = null; - gDebugger = null; - gL10N = null; - gEditor = null; - gVars = null; - gWatch = null; -}); -