Bug 741325 - Sort the scripts in the menulist by filename; r=rcampbell

browser/devtools/debugger/debugger-controller.js

@@ -710,7 +710,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);
   },
 
   /**
@@ -718,8 +718,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();
   },
 
   /**
@@ -843,15 +844,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);
   },
 
   /**

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);
   },
 
   /**

browser/devtools/debugger/test/Makefile.in

@@ -71,6 +71,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) \

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();
       });
     });
 

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;
+});