Bug 1786467 - [devtools] Sort debugger SourceTree in a meaningful way. r=bomsy

The main thread is always displayed first,
then we display all content process targets (sorted by PID number),
and we end up with worker targets (sorted alphabetically).

This should follow the same order as the js context selector.

Differential Revision: https://phabricator.services.mozilla.com/D155336

devtools/client/debugger/src/client/firefox/create.js

@@ -355,6 +355,7 @@ export function createThread(targetFront) {
     name,
     serviceWorkerStatus: targetFront.debuggerServiceWorkerStatus,
     isWebExtension: targetFront.isWebExtension,
+    processID: targetFront.processID,
   };
 }
 

devtools/client/debugger/src/reducers/sources-tree.js

@@ -143,10 +143,12 @@ export default function update(state = initialSourcesTreeState(), action) {
         addThread(state, action.mainThread);
       }
       return state;
+
     case "INSERT_THREAD":
       state = { ...state };
       addThread(state, action.newThread);
       return state;
+
     case "REMOVE_THREAD": {
       const index = state.threadItems.findIndex(item => {
         return item.threadActorID == action.threadActorID;
@@ -197,7 +199,9 @@ function addThread(state, thread) {
   }
   // Inject the reducer thread object on Thread Tree Items
   // (this is handy shortcut to have access to from React components)
+  // (this is also used by sortThreadItems to sort the thread as a Tree in the Browser Toolbox)
   threadItem.thread = thread;
+  state.threadItems.sort(sortThreadItems);
 }
 
 function updateExpanded(state, action) {
@@ -255,6 +259,7 @@ function addSource(threadItems, thread, source) {
     // Note that threadItems will be cloned once to force a state update
     // by the callsite of `addSourceActor`
     threadItems.push(threadItem);
+    threadItems.sort(sortThreadItems);
   }
 
   // Then ensure creating or fetching the related Group Item
@@ -315,6 +320,63 @@ function sortItems(a, b) {
   return 0;
 }
 
+function sortThreadItems(a, b) {
+  // Jest tests aren't emitting the necessary actions to populate the thread attributes.
+  // Ignore sorting for them.
+  if (!a.thread || !b.thread) {
+    return 0;
+  }
+
+  // Top level target is always listed first
+  if (a.thread.isTopLevel) {
+    return -1;
+  } else if (b.thread.isTopLevel) {
+    return 1;
+  }
+
+  // Process targets should come next and after that frame targets
+  if (a.thread.targetType == "process" && b.thread.targetType == "frame") {
+    return -1;
+  } else if (
+    a.thread.targetType == "frame" &&
+    b.thread.targetType == "process"
+  ) {
+    return 1;
+  }
+
+  // And we display the worker targets last.
+  if (
+    a.thread.targetType.endsWith("worker") &&
+    !b.thread.targetType.endsWith("worker")
+  ) {
+    return 1;
+  } else if (
+    !a.thread.targetType.endsWith("worker") &&
+    b.thread.targetType.endsWith("worker")
+  ) {
+    return -1;
+  }
+
+  // Order the process targets by their process ids
+  if (a.thread.processID > b.thread.processID) {
+    return 1;
+  } else if (a.thread.processID < b.thread.processID) {
+    return 0;
+  }
+
+  // Order the frame targets and the worker targets by their target name
+  if (a.thread.targetType == "frame" && b.thread.targetType == "frame") {
+    return a.thread.name.localeCompare(b.thread.name);
+  } else if (
+    a.thread.targetType.endsWith("worker") &&
+    b.thread.targetType.endsWith("worker")
+  ) {
+    return a.thread.name.localeCompare(b.thread.name);
+  }
+
+  return 0;
+}
+
 /**
  * For a given URL's path, in the given group (i.e. typically a given scheme+domain),
  * return the already existing parent directory item, or create it if it doesn't exists.

devtools/client/debugger/test/mochitest/browser_dbg-features-browser-toolbox-source-tree.js

@@ -80,6 +80,40 @@ add_task(async function testSourceTreeNamesForWebExtensions() {
       // with their extension name.
       await assertSourceTreeNode(dbgx, "Picture-In-Picture");
       await assertSourceTreeNode(dbgx, "Form Autofill");
+
+      const threadLabels = [...findAllElements(dbgx, "sourceTreeThreads")].map(
+        el => {
+          return el.textContent;
+        }
+      );
+      is(
+        threadLabels[0],
+        "Main Thread",
+        "The first thread is always the main thread"
+      );
+      let lastPID = -1,
+        lastThreadLabel = "";
+      for (let i = 1; i < threadLabels.length; i++) {
+        const label = threadLabels[i];
+        if (label.startsWith("(pid ")) {
+          ok(
+            !lastThreadLabel,
+            "We should only have content process threads first after the main thread"
+          );
+          const pid = label.match(/pid (\d+)\)/)[1];
+          ok(
+            pid > lastPID,
+            `The content process threads are sorted by incremental PID ${pid} > ${lastPID}`
+          );
+          lastPID = pid;
+        } else {
+          ok(
+            label.localeCompare(lastThreadLabel) > 0,
+            `Worker thread labels are sorted alphabeticaly: ${label} vs ${lastThreadLabel}`
+          );
+          lastThreadLabel = label;
+        }
+      }
     } catch (e) {
       console.log("Caught exception in spawn", e);
       throw e;