Make the flame graph and stack chart always non-inverted

src/actions/profile-view.js

@@ -105,26 +105,30 @@ export function changeSelectedCallNode(
   selectedCallNodePath: CallNodePath,
   context: SelectionContext = { source: 'auto' },
   optionalExpandedToCallNodePath?: CallNodePath
-): Action {
-  if (optionalExpandedToCallNodePath) {
-    for (let i = 0; i < selectedCallNodePath.length; i++) {
-      if (selectedCallNodePath[i] !== optionalExpandedToCallNodePath[i]) {
-        // This assertion ensures that the selectedCallNode will be correctly expanded.
-        throw new Error(
-          oneLine`
-            The optional expanded call node path provided to the changeSelectedCallNode
-            must contain the selected call node path.
-          `
-        );
+): ThunkAction<void> {
+  return (dispatch, getState) => {
+    if (optionalExpandedToCallNodePath) {
+      for (let i = 0; i < selectedCallNodePath.length; i++) {
+        if (selectedCallNodePath[i] !== optionalExpandedToCallNodePath[i]) {
+          // This assertion ensures that the selectedCallNode will be correctly expanded.
+          throw new Error(
+            oneLine`
+              The optional expanded call node path provided to the changeSelectedCallNode
+              must contain the selected call node path.
+            `
+          );
+        }
       }
     }
-  }
-  return {
-    type: 'CHANGE_SELECTED_CALL_NODE',
-    selectedCallNodePath,
-    optionalExpandedToCallNodePath,
-    threadsKey,
-    context,
+    const isInverted = getInvertCallstack(getState());
+    dispatch({
+      type: 'CHANGE_SELECTED_CALL_NODE',
+      isInverted,
+      selectedCallNodePath,
+      optionalExpandedToCallNodePath,
+      threadsKey,
+      context,
+    });
   };
 }
 
@@ -1637,14 +1641,17 @@ export function expandAllCallNodeDescendants(
 export function changeExpandedCallNodes(
   threadsKey: ThreadsKey,
   expandedCallNodePaths: Array<CallNodePath>
-): Action {
-  return {
-    type: 'CHANGE_EXPANDED_CALL_NODES',
-    threadsKey,
-    expandedCallNodePaths,
+): ThunkAction<void> {
+  return (dispatch, getState) => {
+    const isInverted = getInvertCallstack(getState());
+    dispatch({
+      type: 'CHANGE_EXPANDED_CALL_NODES',
+      isInverted,
+      threadsKey,
+      expandedCallNodePaths,
+    });
   };
 }
-
 export function changeSelectedMarker(
   threadsKey: ThreadsKey,
   selectedMarker: MarkerIndex | null,
@@ -1772,13 +1779,17 @@ export function changeInvertCallstack(
       eventCategory: 'profile',
       eventAction: 'change invert callstack',
     });
+    const callTree = selectedThreadSelectors.getCallTree(getState());
+    const selectedCallNode = selectedThreadSelectors.getSelectedCallNodeIndex(
+      getState()
+    );
+    const newSelectedCallNodePath =
+      callTree.findHeavyPathToSameFunctionAfterInversion(selectedCallNode);
     dispatch({
       type: 'CHANGE_INVERT_CALLSTACK',
       invertCallstack,
       selectedThreadIndexes: getSelectedThreadIndexes(getState()),
-      callTree: selectedThreadSelectors.getCallTree(getState()),
-      callNodeTable: selectedThreadSelectors.getCallNodeInfo(getState())
-        .callNodeTable,
+      newSelectedCallNodePath,
     });
   };
 }

src/components/flame-graph/MaybeFlameGraph.js

@@ -16,6 +16,10 @@ import type { ConnectedProps } from 'firefox-profiler/utils/connect';
 
 import './MaybeFlameGraph.css';
 
+// TODO: This component isn't needed any more. Whenever the selected tab
+// is "flame-graph", `invertCallstack` will be `false`. <MaybeFlameGraph /> is
+// only used in the "flame-graph" tab.
+
 type StateProps = {|
   +isPreviewSelectionEmpty: boolean,
   +invertCallstack: boolean,

src/components/stack-chart/index.js

@@ -238,7 +238,7 @@ class StackChartImpl extends React.PureComponent<Props> {
         role="tabpanel"
         aria-labelledby="stack-chart-tab-button"
       >
-        <StackSettings />
+        <StackSettings hideInvertCallstack={true} />
         <TransformNavigator />
         {maxStackDepth === 0 && userTimings.length === 0 ? (
           <StackChartEmptyReasons />

src/profile-logic/call-tree.js

@@ -20,6 +20,7 @@ import type {
   SamplesLikeTable,
   WeightType,
   CallNodeTable,
+  CallNodePath,
   IndexIntoCallNodeTable,
   CallNodeInfo,
   CallNodeData,
@@ -362,6 +363,58 @@ export class CallTree {
       this._thread
     );
   }
+
+  /**
+   * Take a CallNodeIndex, and compute an inverted path for it.
+   *
+   * e.g:
+   *   (invertedPath, invertedCallTree) => path
+   *   (path, callTree) => invertedPath
+   *
+   * Call trees are sorted with the CallNodes with the heaviest total time as the first
+   * entry. This function walks to the tip of the heaviest branches to find the leaf node,
+   * then construct an inverted CallNodePath with the result. This gives a pretty decent
+   * result, but it doesn't guarantee that it will select the heaviest CallNodePath for the
+   * INVERTED call tree. This would require doing a round trip through the reducers or
+   * some other mechanism in order to first calculate the next inverted call tree. This is
+   * probably not worth it, so go ahead and use the uninverted call tree, as it's probably
+   * good enough.
+   */
+  findHeavyPathToSameFunctionAfterInversion(
+    callNodeIndex: IndexIntoCallNodeTable | null
+  ): CallNodePath {
+    if (callNodeIndex === null) {
+      return [];
+    }
+    let parentSelf = 0;
+    let children = [callNodeIndex];
+    const pathToLeaf = [];
+    do {
+      // Walk down the tree's depth to construct a path to the leaf node, this should
+      // be the heaviest branch of the tree.
+      const firstChild = children[0];
+      const { total, self } = this.getNodeData(firstChild);
+      if (total < parentSelf) {
+        // The parent node was the node with the highest self time. We don't need
+        // to descend any deeper because we won't find another node with higher
+        // self time in this subtree because this subtree's total is already lower.
+        // ... except if we have negative weights, i.e. a diff profile, but we
+        // don't bother to handle those separately.
+        break;
+      }
+      parentSelf = self;
+      pathToLeaf.push(firstChild);
+      children = this.getChildren(firstChild);
+    } while (children && children.length > 0);
+
+    return (
+      pathToLeaf
+        // Map the CallNodeIndex to FuncIndex.
+        .map((index) => this._callNodeTable.func[index])
+        // Reverse it so that it's in the proper inverted order.
+        .reverse()
+    );
+  }
 }
 
 function _getInvertedStackSelf(

src/profile-logic/transforms.js

@@ -9,7 +9,6 @@ import {
 } from '../utils/uintarray-encoding';
 import {
   toValidImplementationFilter,
-  getCallNodeIndexFromPath,
   updateThreadStacks,
   updateThreadStacksByGeneratingNewStackColumns,
   getMapStackUpdater,
@@ -18,7 +17,6 @@ import {
 import { timeCode } from '../utils/time-code';
 import { assertExhaustiveCheck, convertToTransformType } from '../utils/flow';
 import { canonicalizeRangeSet } from '../utils/range-set';
-import { CallTree } from '../profile-logic/call-tree';
 import { getSearchFilteredMarkerIndexes } from '../profile-logic/marker-data';
 import { shallowCloneFrameTable, getEmptyStackTable } from './data-structures';
 import { getFunctionName } from './function-info';
@@ -699,53 +697,6 @@ function _callNodePathHasPrefixPath(
   );
 }
 
-/**
- * Take a CallNodePath, and invert it given a CallTree. Note that if the CallTree
- * is itself inverted, you will get back the uninverted CallNodePath to the regular
- * CallTree.
- *
- * e.g:
- *   (invertedPath, invertedCallTree) => path
- *   (path, callTree) => invertedPath
- *
- * Call trees are sorted with the CallNodes with the heaviest total time as the first
- * entry. This function walks to the tip of the heaviest branches to find the leaf node,
- * then construct an inverted CallNodePath with the result. This gives a pretty decent
- * result, but it doesn't guarantee that it will select the heaviest CallNodePath for the
- * INVERTED call tree. This would require doing a round trip through the reducers or
- * some other mechanism in order to first calculate the next inverted call tree. This is
- * probably not worth it, so go ahead and use the uninverted call tree, as it's probably
- * good enough.
- */
-export function invertCallNodePath(
-  path: CallNodePath,
-  callTree: CallTree,
-  callNodeTable: CallNodeTable
-): CallNodePath {
-  let callNodeIndex = getCallNodeIndexFromPath(path, callNodeTable);
-  if (callNodeIndex === null) {
-    // No path was found, return an empty CallNodePath.
-    return [];
-  }
-  let children = [callNodeIndex];
-  const pathToLeaf = [];
-  do {
-    // Walk down the tree's depth to construct a path to the leaf node, this should
-    // be the heaviest branch of the tree.
-    callNodeIndex = children[0];
-    pathToLeaf.push(callNodeIndex);
-    children = callTree.getChildren(callNodeIndex);
-  } while (children && children.length > 0);
-
-  return (
-    pathToLeaf
-      // Map the CallNodeIndex to FuncIndex.
-      .map((index) => callNodeTable.func[index])
-      // Reverse it so that it's in the proper inverted order.
-      .reverse()
-  );
-}
-
 /**
  * Transform a thread's stacks to merge stacks that match the CallNodePath into
  * the calling stack. See `src/types/transforms.js` for more information about the