Bug 741325 - Sort the scripts in the menulist by filename; r=rcampbell
authorVictor Porof <vporof@mozilla.com>
Tue, 17 Apr 2012 21:16:57 +0300
changeset 95524 3bdab8c3149290ef3945d3c037afdb0d28a7ab61
parent 95523 906f3bb4c5aa1d9f200ffaef7cc8eb55a623fa42
child 95525 d636e92961fadb10e8e56dc2e56661a6926c8cdf
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrcampbell
bugs741325
milestone14.0a1
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
Bug 741325 - Sort the scripts in the menulist by filename; r=rcampbell
browser/devtools/debugger/debugger-controller.js
browser/devtools/debugger/debugger-view.js
browser/devtools/debugger/test/Makefile.in
browser/devtools/debugger/test/browser_dbg_propertyview-01.js
browser/devtools/debugger/test/browser_dbg_scripts-sorting.js
--- a/browser/devtools/debugger/debugger-controller.js
+++ b/browser/devtools/debugger/debugger-controller.js
@@ -705,26 +705,27 @@ SourceScripts.prototype = {
     this.activeThread.removeListener("scriptsadded", this._onScriptsAdded);
     this.activeThread.removeListener("scriptscleared", this._onScriptsCleared);
   },
 
   /**
    * 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);
   },
 
   /**
    * Handler for the thread client's scriptsadded notification.
    */
   _onScriptsAdded: function SS__onScriptsAdded() {
     for each (let script in this.activeThread.cachedScripts) {
-      this._addScript(script);
+      this._addScript(script, false);
     }
+    DebuggerView.Scripts.commitScripts();
   },
 
   /**
    * Handler for the thread client's scriptscleared notification.
    */
   _onScriptsCleared: function SS__onScriptsCleared() {
     DebuggerView.Scripts.empty();
   },
@@ -838,25 +839,26 @@ SourceScripts.prototype = {
    * Clears the labels cache, populated by SS_getScriptLabel.
    * This should be done every time the content location changes.
    */
   _clearLabelsCache: function SS__clearLabelsCache() {
     this._labelsCache = {};
   },
 
   /**
-   * 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);
   },
 
   /**
    * Load the editor with the script text if available, otherwise fire an event
    * to load and display the script text.
    *
    * @param object aScript
    *        The script object coming from the active thread.
--- a/browser/devtools/debugger/debugger-view.js
+++ b/browser/devtools/debugger/debugger-view.js
@@ -107,31 +107,41 @@ ScriptsView.prototype = {
    * Checks whether the script with the specified URL is among the scripts
    * known to the debugger and shown in the list.
    *
    * @param string aUrl
    *        The script URL.
    * @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;
     }
     return false;
   },
 
   /**
    * Checks whether the script with the specified label is among the scripts
    * known to the debugger and shown in the list.
    *
    * @param string aLabel
    *        The script label.
    * @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;
     }
     return false;
   },
 
   /**
    * Selects the script with the specified URL from the list.
@@ -167,51 +177,128 @@ ScriptsView.prototype = {
    * @return string | null
    */
   get selected() {
     return this._scripts.selectedItem ?
            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
    */
   get scriptLocations() {
     let locations = [];
     for (let i = 0, l = this._scripts.itemCount; i < l; i++) {
       locations.push(this._scripts.getItemAtIndex(i).value);
     }
     return locations;
   },
 
   /**
-   * 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);
+  },
+
+  /**
+   * 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);
+    }
+  },
 
-    this._scripts.selectedItem = script;
-    return script;
+  /**
+   * 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;
+    }
   },
 
   /**
    * The cached click listener for the scripts container.
    */
   _onScriptsChange: function DVS__onScriptsChange() {
     let script = this._scripts.selectedItem.getUserData("sourceScript");
     DebuggerController.SourceScripts.showScript(script);
@@ -223,16 +310,17 @@ ScriptsView.prototype = {
   _scripts: null,
 
   /**
    * Initialization function, called when the debugger is initialized.
    */
   initialize: function DVS_initialize() {
     this._scripts = document.getElementById("scripts");
     this._scripts.addEventListener("select", this._onScriptsChange, false);
+    this.commitScripts();
   },
 
   /**
    * Destruction function, called when the debugger is shut down.
    */
   destroy: function DVS_destroy() {
     this._scripts.removeEventListener("select", this._onScriptsChange, false);
     this._scripts = null;
@@ -376,17 +464,17 @@ StackFramesView.prototype = {
 
   /**
    * Deselects a frame from the stackframe container.
    *
    * @param number aDepth
    *        The frame depth specified by the debugger.
    */
   unhighlightFrame: function DVF_unhighlightFrame(aDepth) {
-    this.highlightFrame(aDepth, true)
+    this.highlightFrame(aDepth, true);
   },
 
   /**
    * Gets the current dirty state.
    *
    * @return boolean value
    *         True if should load more frames.
    */
--- a/browser/devtools/debugger/test/Makefile.in
+++ b/browser/devtools/debugger/test/Makefile.in
@@ -66,16 +66,17 @@ include $(topsrcdir)/config/rules.mk
 	browser_dbg_panesize.js \
 	browser_dbg_stack-01.js \
 	browser_dbg_stack-02.js \
 	browser_dbg_stack-03.js \
 	browser_dbg_stack-04.js \
 	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_dbg_clean-exit.js \
 	browser_dbg_bug723069_editor-breakpoints.js \
 	browser_dbg_bug731394_editor-contextmenu.js \
 	browser_dbg_displayName.js \
 	browser_dbg_iframes.js \
--- a/browser/devtools/debugger/test/browser_dbg_propertyview-01.js
+++ b/browser/devtools/debugger/test/browser_dbg_propertyview-01.js
@@ -80,38 +80,38 @@ function testSimpleCall() {
 
   gDebuggee.simpleCall();
 }
 
 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.");
 
     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" }
     ];
 
-    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();
       });
     });
 
     executeSoon(function() {
       for (let i = 0; i < vs._scripts.itemCount; i++) {
         let lab = vs._scripts.getItemAtIndex(i).getAttribute("label");
         let loc = urls[i].href + urls[i].leaf;
 
new file mode 100644
--- /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;
+});