[trace] Document behavior of setHasChildren on BottomUpNode

This code had no bugs, but it threw me (and Gemini) for a loop because
it wasn't clear that "setHasChildren" here was being called correctly.

Fixed: 454088373
Change-Id: I3e01f12b46a4cf5f249301cfccb1be1e5a5474c1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7240306
Reviewed-by: Paul Irish <paulirish@chromium.org>
Auto-Submit: Connor Clark <cjamcl@chromium.org>
Commit-Queue: Connor Clark <cjamcl@chromium.org>

Author: Connor Clark

front_end/models/trace/extras/TraceTree.test.ts

@@ -475,9 +475,7 @@ describeWithEnvironment('TraceTree', () => {
       assert.lengthOf(second.events, 3);
     });
 
-    // TODO: Gemini noticed that "setHasChildren" may not be called correctly ... I think
-    // it's right, but fixing it has proven difficult. So here's a test case for a later day.
-    it.skip('[crbug.com/454088373] correctly identifies which nodes have children in a nested structure', () => {
+    it('correctly identifies which nodes have children in a nested structure', () => {
       // This builds the following simple tree:
       // |----Parent Task----|
       //   |--Child Task--|
@@ -505,8 +503,10 @@ describeWithEnvironment('TraceTree', () => {
       assert.exists(parentNode, 'Parent node was not found');
       assert.exists(childNode, 'Child node was not found');
 
-      assert.isTrue(parentNode.hasChildren(), 'Parent node should have children');
-      assert.isFalse(childNode.hasChildren(), 'Child node should not have children');
+      // Since this is bottom-up, a node with callers should have children.
+      // A node with no callers (a top-level node) has no children.
+      assert.isFalse(parentNode.hasChildren(), 'Parent node (the root) should NOT have children in bottom-up');
+      assert.isTrue(childNode.hasChildren(), 'Child node (the nested event) SHOULD have children in bottom-up');
     });
   });
 

front_end/models/trace/extras/TraceTree.ts

@@ -493,8 +493,10 @@ export class BottomUpRootNode extends Node {
         node.totalTime += totalTimeById.get(id) || 0;
         totalTimeById.delete(id);
       }
-      // TODO: this may be wrong. See the skipped test in TraceTree.test.ts.
-      if (firstNodeStack.length) {
+
+      // An item on this stack means that this current node has a caller. Therefore,
+      // in a bottom-up view it has children.
+      if (idStack.length > 0) {
         node.setHasChildren(true);
       }
     }