This adds the `getImmediatelyDominated` method to `DominatorTree` which takes a node id and returns the set of each node ids for every node that is immediately dominated by the node with the given id. The results are sorted by greatest to least retained size. In conjunction with the `root` attribute, this can be used to traverse the whole dominator tree.

Once again: r?bz for webidl changes r?sfink for the rest Thanks for bearing with me :)

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

Comment on attachment 8689794 [details] [diff] [review] Expose a method to get a node's set of immediately dominated nodes in the dominator tree >+static MallocSizeOf getCurrentThreadDebuggerMallocSizeOf() { Curly on next line, please. And function name and return type on different lines. >+struct CompareByRetainedSize I don't see how this works in general, if partway through the sort we start giving different answers. There's no guarantee the sort will even converge... Seems like we should have an array of structs that contain an ubi::Node and its retained size, grab all the sizes up front (throwing if any fail), then do the sort. >+ auto node = mHeapSnapshot->getNodeById(id); >+ auto range = mDominatorTree.getDominatedSet(*node); >+ auto rootId = Root(); >+ auto id = node.identifier(); Would like actual types for those.

Getting the retained size before sorting was a good call, I think it is a lot simpler now.

Comment on attachment 8690170 [details] [diff] [review] Expose a method to get a node's set of immediately dominated nodes in the dominator tree This file could benefit from a "using JS::ubi::Node" up at the top, I think. Would reduce the temptation to use auto. ;) >+ for (const JS::ubi::Node& node : *range) { This is shadowing the "node" above that was returned from getNodeById. Maybe call this one dominatedNode or something? >+ aOutResult.SetValue(nsTArray<uint64_t>(length)); Mmm, rvalue refs. OK, this doesn't make an extra copy, good. >+ // The root dominates itself, but we don't want to expose that to JS. Then don't we need to size aOutResult to length - 1? Otherwise it'll have a trailing 0, no? Did the tests not catch this? Or better yet, can we size dominatedNodes to length - 1, skip adding root to it to start with, then size aOutResult to dominatedNodes.Length(). r=me with the above fixed. Thanks for updating!

Comment on attachment 8690170 [details] [diff] [review] Expose a method to get a node's set of immediately dominated nodes in the dominator tree Review of attachment 8690170 [details] [diff] [review]: ----------------------------------------------------------------- I don't have anything beyond what bz said.

(In reply to Boris Zbarsky [:bz] from comment #5) > >+ // The root dominates itself, but we don't want to expose that to JS. > > Then don't we need to size aOutResult to length - 1? Otherwise it'll have a > trailing 0, no? Did the tests not catch this? > > Or better yet, can we size dominatedNodes to length - 1, skip adding root to > it to start with, then size aOutResult to dominatedNodes.Length(). bz and I talked about this last week on IRC, but for posterity and sfink's benefit: in this case the _capacity_ of the array would be one larger than the length, but the length is unaffected by the number passed to the constructor here. This is less slop than we would have if we didn't specify an initial capacity at all (and more performant) and instead just dynamically resized as we appended elements.