From 99f1a4b2870cc3a3727b0c78fcf60d6f5e740570 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 17 Apr 2012 21:16:57 +0300 Subject: [PATCH] Bug 741325 - Sort the scripts in the menulist by filename; r=rcampbell --- .../devtools/debugger/debugger-controller.js | 22 ++-- browser/devtools/debugger/debugger-view.js | 118 ++++++++++++++--- browser/devtools/debugger/test/Makefile.in | 1 + .../test/browser_dbg_propertyview-01.js | 6 +- .../test/browser_dbg_scripts-sorting.js | 124 ++++++++++++++++++ 5 files changed, 243 insertions(+), 28 deletions(-) create mode 100644 browser/devtools/debugger/test/browser_dbg_scripts-sorting.js diff --git a/browser/devtools/debugger/debugger-controller.js b/browser/devtools/debugger/debugger-controller.js index d8ac29e699d..5929d583ad9 100644 --- a/browser/devtools/debugger/debugger-controller.js +++ b/browser/devtools/debugger/debugger-controller.js @@ -705,7 +705,7 @@ SourceScripts.prototype = { * Handler for the debugger client's unsolicited newScript notification. */ _onNewScript: function SS__onNewScript(aNotification, aPacket) { - this._addScript({ url: aPacket.url, startLine: aPacket.startLine }); + this._addScript({ url: aPacket.url, startLine: aPacket.startLine }, true); }, /** @@ -713,8 +713,9 @@ SourceScripts.prototype = { */ _onScriptsAdded: function SS__onScriptsAdded() { for each (let script in this.activeThread.cachedScripts) { - this._addScript(script); + this._addScript(script, false); } + DebuggerView.Scripts.commitScripts(); }, /** @@ -819,15 +820,16 @@ SourceScripts.prototype = { }, /** - * Add the specified script to the list and display it in the editor if the - * editor is empty. + * Add the specified script to the list. + * + * @param object aScript + * The script object coming from the active thread. + * @param boolean aForceFlag + * True to force the script to be immediately added. */ - _addScript: function SS__addScript(aScript) { - DebuggerView.Scripts.addScript(this._getScriptLabel(aScript.url), aScript); - - if (DebuggerView.editor.getCharCount() == 0) { - this.showScript(aScript); - } + _addScript: function SS__addScript(aScript, aForceFlag) { + DebuggerView.Scripts.addScript( + this._getScriptLabel(aScript.url), aScript, aForceFlag); }, /** diff --git a/browser/devtools/debugger/debugger-view.js b/browser/devtools/debugger/debugger-view.js index 4620d6f3237..5de15e57f25 100644 --- a/browser/devtools/debugger/debugger-view.js +++ b/browser/devtools/debugger/debugger-view.js @@ -112,6 +112,11 @@ ScriptsView.prototype = { * @return boolean */ contains: function DVS_contains(aUrl) { + if (this._tmpScripts.some(function(element) { + return element.script.url == aUrl; + })) { + return true; + } if (this._scripts.getElementsByAttribute("value", aUrl).length > 0) { return true; } @@ -127,6 +132,11 @@ ScriptsView.prototype = { * @return boolean */ containsLabel: function DVS_containsLabel(aLabel) { + if (this._tmpScripts.some(function(element) { + return element.label == aLabel; + })) { + return true; + } if (this._scripts.getElementsByAttribute("label", aLabel).length > 0) { return true; } @@ -171,6 +181,18 @@ ScriptsView.prototype = { this._scripts.selectedItem.value : null; }, + /** + * Returns the list of labels in the scripts container. + * @return array + */ + get scriptLabels() { + let labels = []; + for (let i = 0, l = this._scripts.itemCount; i < l; i++) { + labels.push(this._scripts.getItemAtIndex(i).label); + } + return labels; + }, + /** * Returns the list of URIs for scripts in the page. * @return array @@ -184,29 +206,94 @@ ScriptsView.prototype = { }, /** - * Adds a script to the scripts container. - * If the script already exists (was previously added), null is returned. - * Otherwise, the newly created element is returned. + * Prepares a script to be added to the scripts container. This allows + * for a large number of scripts to be batched up before being + * alphabetically sorted and added in the container. + * @see ScriptsView.commitScripts + * + * If aForceFlag is true, the script will be immediately inserted at the + * necessary position in the container so that all the scripts remain sorted. + * This can be much slower than batching up multiple scripts. * * @param string aLabel * The simplified script location to be shown. * @param string aScript * The source script. - * @return object - * The newly created html node representing the added script. + * @param boolean aForceFlag + * True to force the script to be immediately added. */ - addScript: function DVS_addScript(aLabel, aScript) { - // Make sure we don't duplicate anything. - if (this.containsLabel(aLabel)) { - return null; + addScript: function DVS_addScript(aLabel, aScript, aForceFlag) { + // Batch the script to be added later. + if (!aForceFlag) { + this._tmpScripts.push({ label: aLabel, script: aScript }); + return; } - let script = this._scripts.appendItem(aLabel, aScript.url); - script.setAttribute("tooltiptext", aScript.url); - script.setUserData("sourceScript", aScript, null); + // Find the target position in the menulist and insert the script there. + for (let i = 0, l = this._scripts.itemCount; i < l; i++) { + if (this._scripts.getItemAtIndex(i).label > aLabel) { + this._createScriptElement(aLabel, aScript, i); + return; + } + } + // The script is alphabetically the last one. + this._createScriptElement(aLabel, aScript, -1, true); + }, - this._scripts.selectedItem = script; - return script; + /** + * Adds all the prepared scripts to the scripts container. + * If a script already exists (was previously added), nothing happens. + */ + commitScripts: function DVS_commitScripts() { + let newScripts = this._tmpScripts; + this._tmpScripts = []; + + if (!newScripts || !newScripts.length) { + return; + } + newScripts.sort(function(a, b) { + return a.label.toLowerCase() > b.label.toLowerCase(); + }); + + for (let i = 0, l = newScripts.length; i < l; i++) { + let item = newScripts[i]; + this._createScriptElement(item.label, item.script, -1, true); + } + }, + + /** + * Creates a custom script element and adds it to the scripts container. + * If the script with the specified label already exists, nothing happens. + * + * @param string aLabel + * The simplified script location to be shown. + * @param string aScript + * The source script. + * @param number aIndex + * The index where to insert to new script in the container. + * Pass -1 to append the script at the end. + * @param boolean aSelectIfEmptyFlag + * True to set the newly created script as the currently selected item + * if there are no other existing scripts in the container. + */ + _createScriptElement: function DVS__createScriptElement( + aLabel, aScript, aIndex, aSelectIfEmptyFlag) + { + // Make sure we don't duplicate anything. + if (aLabel == "null" || this.containsLabel(aLabel)) { + return; + } + + let scriptItem = + aIndex == -1 ? this._scripts.appendItem(aLabel, aScript.url) + : this._scripts.insertItemAt(aIndex, aLabel, aScript.url); + + scriptItem.setAttribute("tooltiptext", aScript.url); + scriptItem.setUserData("sourceScript", aScript, null); + + if (this._scripts.itemCount == 1 && aSelectIfEmptyFlag) { + this._scripts.selectedItem = scriptItem; + } }, /** @@ -228,6 +315,7 @@ ScriptsView.prototype = { initialize: function DVS_initialize() { this._scripts = document.getElementById("scripts"); this._scripts.addEventListener("select", this._onScriptsChange, false); + this.commitScripts(); }, /** @@ -381,7 +469,7 @@ StackFramesView.prototype = { * The frame depth specified by the debugger. */ unhighlightFrame: function DVF_unhighlightFrame(aDepth) { - this.highlightFrame(aDepth, true) + this.highlightFrame(aDepth, true); }, /** diff --git a/browser/devtools/debugger/test/Makefile.in b/browser/devtools/debugger/test/Makefile.in index 6e5da8a604a..6074ee38bab 100644 --- a/browser/devtools/debugger/test/Makefile.in +++ b/browser/devtools/debugger/test/Makefile.in @@ -70,6 +70,7 @@ _BROWSER_TEST_FILES = \ browser_dbg_stack-05.js \ browser_dbg_location-changes.js \ browser_dbg_script-switching.js \ + browser_dbg_scripts-sorting.js \ browser_dbg_pause-resume.js \ browser_dbg_update-editor-mode.js \ $(warning browser_dbg_select-line.js temporarily disabled due to oranges, see bug 726609) \ diff --git a/browser/devtools/debugger/test/browser_dbg_propertyview-01.js b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js index f92c0a04ffc..663ebfd6096 100644 --- a/browser/devtools/debugger/test/browser_dbg_propertyview-01.js +++ b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js @@ -85,7 +85,8 @@ function resumeAndFinish() { gDebugger.DebuggerController.activeThread.resume(function() { let vs = gDebugger.DebuggerView.Scripts; let ss = gDebugger.DebuggerController.SourceScripts; - ss._onScriptsCleared(); + vs.empty(); + vs._scripts.removeEventListener("select", vs._onScriptsChange, false); is(ss._trimUrlQuery("a/b/c.d?test=1&random=4"), "a/b/c.d", "Trimming the url query isn't done properly."); @@ -101,12 +102,11 @@ function resumeAndFinish() { { href: "si://interesting.address.moc/random/x/y/", leaf: "script.js?a=1&b=2&c=3" } ]; - vs._scripts.removeEventListener("select", vs._onScriptsChange, false); - urls.forEach(function(url) { executeSoon(function() { let loc = url.href + url.leaf; vs.addScript(ss._getScriptLabel(loc, url.href), { url: loc }); + vs.commitScripts(); }); }); diff --git a/browser/devtools/debugger/test/browser_dbg_scripts-sorting.js b/browser/devtools/debugger/test/browser_dbg_scripts-sorting.js new file mode 100644 index 00000000000..b2ae619ef5b --- /dev/null +++ b/browser/devtools/debugger/test/browser_dbg_scripts-sorting.js @@ -0,0 +1,124 @@ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ +var gPane = null; +var gTab = null; +var gDebuggee = null; +var gDebugger = null; + +function test() { + debug_tab_pane(STACK_URL, function(aTab, aDebuggee, aPane) { + gTab = aTab; + gDebuggee = aDebuggee; + gPane = aPane; + gDebugger = gPane.debuggerWindow; + + testSimpleCall(); + }); +} + +function testSimpleCall() { + gDebugger.DebuggerController.activeThread.addOneTimeListener("framesadded", function() { + Services.tm.currentThread.dispatch({ run: function() { + resumeAndFinish(); + }}, 0); + }); + + gDebuggee.simpleCall(); +} + +function resumeAndFinish() { + gDebugger.DebuggerController.activeThread.resume(function() { + checkScriptsOrder(); + addScriptsAndCheckOrder(1, function() { + addScriptsAndCheckOrder(2, function() { + addScriptsAndCheckOrder(3, function() { + closeDebuggerAndFinish(gTab); + }); + }); + }); + }); +} + +function addScriptsAndCheckOrder(method, callback) { + let vs = gDebugger.DebuggerView.Scripts; + let ss = gDebugger.DebuggerController.SourceScripts; + vs.empty(); + vs._scripts.removeEventListener("select", vs._onScriptsChange, false); + + let urls = [ + { href: "ici://some.address.com/random/", leaf: "subrandom/" }, + { href: "ni://another.address.org/random/subrandom/", leaf: "page.html" }, + { href: "san://interesting.address.gro/random/", leaf: "script.js" }, + { href: "si://interesting.address.moc/random/", leaf: "script.js" }, + { href: "si://interesting.address.moc/random/", leaf: "x/script.js" }, + { href: "si://interesting.address.moc/random/", leaf: "x/y/script.js?a=1" }, + { href: "si://interesting.address.moc/random/x/", leaf: "y/script.js?a=1&b=2" }, + { href: "si://interesting.address.moc/random/x/y/", leaf: "script.js?a=1&b=2&c=3" } + ]; + + urls.sort(function(a, b) { + return Math.random() - 0.5; + }); + + switch (method) { + case 1: + urls.forEach(function(url) { + let loc = url.href + url.leaf; + vs.addScript(ss._getScriptLabel(loc, url.href), { url: loc }); + }); + vs.commitScripts(); + break; + + case 2: + urls.forEach(function(url) { + let loc = url.href + url.leaf; + vs.addScript(ss._getScriptLabel(loc, url.href), { url: loc }, true); + }); + break; + + case 3: + let i = 0 + for (; i < urls.length / 2; i++) { + let url = urls[i]; + let loc = url.href + url.leaf; + vs.addScript(ss._getScriptLabel(loc, url.href), { url: loc }); + } + vs.commitScripts(); + + for (; i < urls.length; i++) { + let url = urls[i]; + let loc = url.href + url.leaf; + vs.addScript(ss._getScriptLabel(loc, url.href), { url: loc }, true); + } + break; + } + + executeSoon(function() { + checkScriptsOrder(method); + callback(); + }); +} + +function checkScriptsOrder(method) { + let labels = gDebugger.DebuggerView.Scripts.scriptLabels; + let sorted = labels.reduce(function(prev, curr, index, array) { + return array[index - 1] < array[index]; + }); + + ok(sorted, + "Using method " + method + ", " + + "the scripts weren't in the correct order: " + labels.toSource()); + + return sorted; +} + +registerCleanupFunction(function() { + removeTab(gTab); + gPane = null; + gTab = null; + gDebuggee = null; + gDebugger = null; +});