Make 'Reveal in Summary view' work consistently

My recent change caused the 'Reveal in Summary view' command to fail if
the object to find was not in the first category with its class name.
This CL fixes the problem and adds a regression test.

Bug: 357902505
Change-Id: Ice45060c6fbe30b80c95e0a8d91e16d6d1762606
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5967453
Commit-Queue: Seth Brenith <[email protected]>
Reviewed-by: Simon Zünd <[email protected]>

Author: sethbrenith

front_end/entrypoints/heap_snapshot_worker/HeapSnapshot.ts

@@ -1272,7 +1272,7 @@ export abstract class HeapSnapshot {
       // for class keys.
       aggregates = Object.create(null);
       for (const [classKey, aggregate] of aggregatesMap.entries()) {
-        const newKey = typeof classKey === 'number' ? (',' + this.strings[classKey]) : classKey;
+        const newKey = this.classKeyFromClassKeyInternal(classKey);
         aggregates[newKey] = aggregate;
       }
       if (key) {
@@ -2531,10 +2531,17 @@ export abstract class HeapSnapshot {
     return null;
   }
 
-  nodeClassName(snapshotObjectId: number): string|null {
+  // Converts an internal class key, suitable for categorizing within this
+  // snapshot, to a public class key, which can be used in comparisons
+  // between multiple snapshots.
+  classKeyFromClassKeyInternal(key: string|number): string {
+    return typeof key === 'number' ? (',' + this.strings[key]) : key;
+  }
+
+  nodeClassKey(snapshotObjectId: number): string|null {
     const node = this.nodeForSnapshotObjectId(snapshotObjectId);
     if (node) {
-      return node.className();
+      return this.classKeyFromClassKeyInternal(node.classKeyInternal());
     }
     return null;
   }

front_end/panels/profiler/HeapSnapshotDataGrids.ts

@@ -831,17 +831,18 @@ export class HeapSnapshotConstructorsDataGrid extends HeapSnapshotViewportDataGr
       return null;
     }
 
-    const className = await this.snapshot.nodeClassName(parseInt(id, 10));
-    if (!className) {
+    const classKey = await this.snapshot.nodeClassKey(parseInt(id, 10));
+    if (!classKey) {
       return null;
     }
 
-    const parent = this.topLevelNodes().find(classNode => classNode.name === className);
+    const topLevelNodes = this.topLevelNodes() as HeapSnapshotConstructorNode[];
+    const parent = topLevelNodes.find(classNode => classNode.classKey === classKey);
     if (!parent) {
       return null;
     }
 
-    const nodes = await (parent as HeapSnapshotConstructorNode).populateNodeBySnapshotObjectId(parseInt(id, 10));
+    const nodes = await parent.populateNodeBySnapshotObjectId(parseInt(id, 10));
     return nodes.length ? this.revealTreeNode(nodes) : null;
   }
 

front_end/panels/profiler/HeapSnapshotProxy.ts

@@ -314,8 +314,8 @@ export class HeapSnapshotProxy extends HeapSnapshotProxyObject {
     return this.callMethodPromise('calculateSnapshotDiff', baseSnapshotId, baseSnapshotAggregates);
   }
 
-  nodeClassName(snapshotObjectId: number): Promise<string|null> {
-    return this.callMethodPromise('nodeClassName', snapshotObjectId);
+  nodeClassKey(snapshotObjectId: number): Promise<string|null> {
+    return this.callMethodPromise('nodeClassKey', snapshotObjectId);
   }
 
   createEdgesProvider(nodeIndex: number): HeapSnapshotProviderProxy {

test/e2e/helpers/memory-helpers.ts

@@ -296,8 +296,12 @@ export async function changeAllocationSampleViewViaDropdown(newPerspective: stri
   await dropdown.select(optionValue);
 }
 
-export async function focusTableRow(text: string) {
+export async function focusTableRowWithName(text: string) {
   const row = await waitFor(`//span[text()="${text}"]/ancestor::tr`, undefined, undefined, 'xpath');
+  await focusTableRow(row);
+}
+
+export async function focusTableRow(row: puppeteer.ElementHandle<Element>) {
   // Click in a numeric cell, to avoid accidentally clicking a link.
   const cell = await waitFor('.numeric-column', row);
   await clickElement(cell);

test/e2e/memory/memory_test.ts

@@ -29,6 +29,7 @@ import {
   expandFocusedRow,
   findSearchResult,
   focusTableRow,
+  focusTableRowWithName,
   getAddedCountFromComparisonRow,
   getAddedCountFromComparisonRowWithName,
   getCategoryRow,
@@ -433,12 +434,12 @@ describe('The Memory Panel', function() {
     await setSearchFilter('Retainer');
     await waitForSearchResultNumber(4);
     await findSearchResult('Retainer()');
-    await focusTableRow('Retainer()');
+    await focusTableRowWithName('Retainer()');
     await expandFocusedRow();
-    await focusTableRow('customProperty');
+    await focusTableRowWithName('customProperty');
     const sizesForSet = await getSizesFromSelectedRow();
     await expandFocusedRow();
-    await focusTableRow('(internal array)[]');
+    await focusTableRowWithName('(internal array)[]');
     const sizesForBackingStorage = await getSizesFromSelectedRow();
     return {sizesForSet, sizesForBackingStorage};
   }
@@ -582,7 +583,14 @@ describe('The Memory Panel', function() {
     let rows = await waitForMany('tr.data-grid-data-grid-node', 2);
     assert.strictEqual(30, await getCountFromCategoryRow(rows[0]));
     assert.strictEqual(3, await getCountFromCategoryRow(rows[1]));
+    await focusTableRow(rows[0]);
+    await expandFocusedRow();
     const {frontend, target} = await getBrowserAndPages();
+    await frontend.keyboard.press('ArrowDown');
+    await clickOnContextMenuForRetainer('x', 'Reveal in Summary view');
+    await waitUntilRetainerChainSatisfies(
+        retainerChain => retainerChain.length > 0 && retainerChain[0].propertyName === 'a');
+
     await target.bringToFront();
     await target.click('button#update');
     await frontend.bringToFront();

test/e2e/resources/memory/duplicated-names.html

@@ -19,6 +19,8 @@ <h1>Memory Panel (Heap profiler) Test</h1>
   for (let i = 0; i < 30; ++i) {
     holder.d.push(makeObject());
   }
+  holder.a.x = holder.d[0];
+  holder.d[0].x = [1, 2, 3];
   document.getElementById('update').addEventListener('click', function () {
     holder.b = new DuplicatedClassName();
     holder.e = new DuplicatedClassName();