When working with the leaking MouseEvents in https://github.com/TatumCreative/bunny-memory-leak, it was frustrating that we could see that MouseEvents were leaking but couldn't view their retained paths. Retaining paths only make sense for individual objects, and the only view we have dealing with individual objects right now is the dominator tree, so bug 1149385 only adds shortest retaining paths for dominator tree items. The problem is the dominator tree sorts nodes by largest retained size and the MouseEvents were small in comparison. See also bug 1243091, which is the best long term solution for leaks, but we may be able to get some short term wins with this bug.

Note that this patch is stand alone enough that extant tests pass, but the INDIVIDUALS view state changes a little more in later patches. Still hopefully easier to review with this split out.

Mechanical renaming. Just needs a rubber stamp.

And the meat of it all.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b88f5a6422f

Demo!

Fix typo.

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e52a3dd31c96

Comment on attachment 8739558 [details] [diff] [review] Part 0: Add the INDIVIDUALS view state to the memory panel Review of attachment 8739558 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/memory/components/toolbar.js @@ +144,5 @@ > ) > ) > : null; > + } else if (view.state === viewState.INDIVIDUALS){ > + assert(false, "TODO FITZGEN"); Should have a helpful assertion here ::: devtools/client/memory/reducers/view.js @@ +20,5 @@ > + const { newViewState, oldDiffing, oldSelected } = action; > + assert(newViewState); > + > + if (newViewState === viewState.INDIVIDUALS) { > + assert(oldDiffing || oldSelected); needs second arg

Comment on attachment 8739559 [details] [diff] [review] Part 1: Rename "dominator tree display" to "label display" Review of attachment 8739559 [details] [diff] [review]: ----------------------------------------------------------------- No l10n changes needed here?

Comment on attachment 8739566 [details] [diff] [review] Part 2: Implement the census individuals view Review of attachment 8739566 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/shared/heapsnapshot/CensusUtils.js @@ +437,5 @@ > * @overrides Visitor.prototype.enter > */ > GetLeavesVisitor.prototype.enter = function(breakdown, report, edge) { > this._index++; > + console.log("FITZGEN: GetLeavesVisitor/enter:", report); oops, few console logs in this file

Rolled up the patches into one and addressed review comments.

Fix test failures introduced when rebasing + fixing up.

Can someone explain the meaning of multiple "# TODO FITZGEN" in memory.properties? I also wonder about the accessibility of characters like ← or ⁂

D'oh, that's what I get for grepping for TODO in devtools/client/memory and not in all of devtools proper :-/ Will land a follow up soon.

Jim, I'm passing this review to you since Jordan has left us. Small follow up for some really dumb logs/comments that should not have landed with the original patch.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a999a2c7fc9

Comment on attachment 8740635 [details] [diff] [review] Follow up: remove debug logs and TODO comments that should not have landed Review of attachment 8740635 [details] [diff] [review]: ----------------------------------------------------------------- Just some typos. ::: devtools/client/locales/en-US/memory.properties @@ +181,5 @@ > # tool's filter search box. > filter.tooltip=Filter the contents of the heap snapshot > > +# LOCALIZATION NOTE (tree-tiem.view-individuals.tooltip): The tooltip for the > +# button to view individuals in this group. "tree-tiem" @@ +311,5 @@ > # snapshot state ERROR, used in the main heap view. > snapshot.state.error.full=There was an error processing this snapshot. > > +# LOCALIZATION NOTE (individuals.state.error): The short message displayed when > +# there is an error fetching indiduals from a group. "indiduals" @@ +316,4 @@ > individuals.state.error=Error > > +# LOCALIZATION NOTE (individuals.state.error.full): The longer message displayed > +# when there is an error fetching indiduals from a group. "indiduals"

I've added some docs for this, here: https://developer.mozilla.org/en-US/docs/Tools/Memory/Aggregate_view#Type Please let me know if this covers it!

Looks great! Thanks Will!