Bug 1544530 - [sync 132] - Sort SourcesTree by url when files share same name r=jlast
authorAnthony Xie <ax1e92@gmail.com>
Tue, 16 Apr 2019 16:52:23 +0000
changeset 469772 fb9539c87371
parent 469771 4f6f63643ad2
child 469773 0a9b60065774
push id35882
push usercbrindusan@mozilla.com
push dateWed, 17 Apr 2019 15:54:01 +0000
treeherdermozilla-central@37185c0ae520 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1544530
milestone68.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 1544530 - [sync 132] - Sort SourcesTree by url when files share same name r=jlast Differential Revision: https://phabricator.services.mozilla.com/D27716
devtools/client/debugger/src/utils/sources-tree/addToTree.js
devtools/client/debugger/src/utils/sources-tree/treeOrder.js
devtools/client/debugger/test/mochitest/browser_dbg-sources-querystring.js
--- a/devtools/client/debugger/src/utils/sources-tree/addToTree.js
+++ b/devtools/client/debugger/src/utils/sources-tree/addToTree.js
@@ -43,17 +43,18 @@ function createNodeInTree(
  */
 function findOrCreateNode(
   parts: string[],
   subTree: TreeDirectory,
   path: string,
   part: string,
   index: number,
   url: Object,
-  debuggeeHost: ?string
+  debuggeeHost: ?string,
+  source: Source
 ): TreeDirectory {
   const addedPartIsFile = partIsFile(index, parts, url);
 
   const { found: childFound, index: childIndex } = findNodeInContents(
     subTree,
     createTreeNodeMatcher(part, !addedPartIsFile, debuggeeHost)
   );
 
@@ -64,47 +65,54 @@ function findOrCreateNode(
 
   // we found a path with the same name as the part. We need to determine
   // if this is the correct child, or if we have a naming conflict
   const child = subTree.contents[childIndex];
   const childIsFile = !nodeHasChildren(child);
 
   // if we have a naming conflict, we'll create a new node
   if (child.type === "source" || (!childIsFile && addedPartIsFile)) {
-    return createNodeInTree(part, path, subTree, childIndex);
+    // pass true to findNodeInContents to sort node by url
+    const { index: insertIndex } = findNodeInContents(
+      subTree,
+      createTreeNodeMatcher(part, !addedPartIsFile, debuggeeHost, source, true)
+    );
+    return createNodeInTree(part, path, subTree, insertIndex);
   }
 
   // if there is no naming conflict, we can traverse into the child
   return child;
 }
 
 /*
  * walk the source tree to the final node for a given url,
  * adding new nodes along the way
  */
 function traverseTree(
   url: ParsedURL,
   tree: TreeDirectory,
-  debuggeeHost: ?string
+  debuggeeHost: ?string,
+  source: Source
 ): TreeNode {
   const parts = url.path.split("/").filter(p => p !== "");
   parts.unshift(url.group);
 
   let path = "";
   return parts.reduce((subTree, part, index) => {
     path = path ? `${path}/${part}` : part;
     const debuggeeHostIfRoot = index === 0 ? debuggeeHost : null;
     return findOrCreateNode(
       parts,
       subTree,
       path,
       part,
       index,
       url,
-      debuggeeHostIfRoot
+      debuggeeHostIfRoot,
+      source
     );
   }, tree);
 }
 
 /*
  * Add a source file to a directory node in the tree
  */
 function addSourceToNode(
@@ -160,13 +168,13 @@ export function addToTree(
   debuggeeHost: ?string
 ) {
   const url = getURL(source, debuggeeHost);
 
   if (isInvalidUrl(url, source)) {
     return;
   }
 
-  const finalNode = traverseTree(url, tree, debuggeeHost);
+  const finalNode = traverseTree(url, tree, debuggeeHost, source);
 
   // $FlowIgnore
   finalNode.contents = addSourceToNode(finalNode, url, source);
 }
--- a/devtools/client/debugger/src/utils/sources-tree/treeOrder.js
+++ b/devtools/client/debugger/src/utils/sources-tree/treeOrder.js
@@ -5,16 +5,18 @@
 // @flow
 
 import { parse } from "../url";
 
 import { nodeHasChildren } from "./utils";
 
 import type { TreeNode } from "./types";
 
+import type { Source } from "../../types";
+
 /*
  * Gets domain from url (without www prefix)
  */
 export function getDomain(url?: string): ?string {
   // TODO: define how files should be ordered on the browser debugger
   if (!url) {
     return null;
   }
@@ -89,54 +91,67 @@ function createTreeNodeMatcherWithDebugg
     }
     return isExactDomainMatch(node.name, debuggeeHost) ? 0 : 1;
   };
 }
 
 function createTreeNodeMatcherWithNameAndOther(
   part: string,
   isDir: boolean,
-  debuggeeHost: ?string
+  debuggeeHost: ?string,
+  source?: Source,
+  sortByUrl?: boolean
 ): FindNodeInContentsMatcher {
   return (node: TreeNode) => {
     if (node.name === IndexName) {
       return -1;
     }
     if (debuggeeHost && isExactDomainMatch(node.name, debuggeeHost)) {
       return -1;
     }
     const nodeIsDir = nodeHasChildren(node);
     if (nodeIsDir && !isDir) {
       return -1;
     } else if (!nodeIsDir && isDir) {
       return 1;
     }
+    if (sortByUrl && node.type === "source" && source) {
+      return node.contents.url.localeCompare(source.url);
+    }
 
     return node.name.localeCompare(part);
   };
 }
 
 /*
  * Creates a matcher for findNodeInContents.
  * The sorting order of nodes during comparison is:
  * - "(index)" node
  * - root node with the debuggee host/domain
  * - hosts/directories (not files) sorted by name
  * - files sorted by name
  */
 export function createTreeNodeMatcher(
   part: string,
   isDir: boolean,
-  debuggeeHost: ?string
+  debuggeeHost: ?string,
+  source?: Source,
+  sortByUrl?: boolean
 ): FindNodeInContentsMatcher {
   if (part === IndexName) {
     // Specialied matcher, when we are looking for "(index)" position.
     return createTreeNodeMatcherWithIndex();
   }
 
   if (debuggeeHost && isExactDomainMatch(part, debuggeeHost)) {
     // Specialied matcher, when we are looking for domain position.
     return createTreeNodeMatcherWithDebuggeeHost(debuggeeHost);
   }
 
   // Rest of the cases, without mentioned above.
-  return createTreeNodeMatcherWithNameAndOther(part, isDir, debuggeeHost);
+  return createTreeNodeMatcherWithNameAndOther(
+    part,
+    isDir,
+    debuggeeHost,
+    source,
+    sortByUrl
+  );
 }
--- a/devtools/client/debugger/test/mochitest/browser_dbg-sources-querystring.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg-sources-querystring.js
@@ -22,19 +22,24 @@ add_task(async function() {
   } = dbg;
 
   // Expand nodes and make sure more sources appear.
   await assertSourceCount(dbg, 2);
   await clickElement(dbg, "sourceDirectoryLabel", 2);
 
   const labels = [getLabel(dbg, 4), getLabel(dbg, 3)];
   is(
-    labels.includes("simple1.js?x=1") && labels.includes("simple1.js?x=2"),
-    true,
-    "simple1.js?x=1 and simple2.jsx=2 exist"
+    getLabel(dbg, 3),
+    "simple1.js?x=1",
+    "simple1.js?x=1 exists"
+  );
+  is(
+    getLabel(dbg, 4),
+    "simple1.js?x=2",
+    "simple1.js?x=2 exists"
   );
 
   const source = findSource(dbg, "simple1.js?x=1");
   await selectSource(dbg, source);
   const tab = findElement(dbg, "activeTab");
   is(tab.innerText, "simple1.js?x=1", "Tab label is simple1.js?x=1");
   await addBreakpoint(dbg, "simple1.js?x=1", 6);
   assertBreakpointHeading(dbg, "simple1.js?x=1", 2);