[webkit-reviews] review granted: [Bug 230287] Web Inspector: `TreeOutline` should return early when failing to find an ancestor while populating the tree : [Attachment 438201] Patch v1.0

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Patrick Angle <pangle at apple.com>'s
request for review:
Bug 230287: Web Inspector: `TreeOutline` should return early when failing to
find an ancestor while populating the tree
https://bugs.webkit.org/show_bug.cgi?id=230287

Attachment 438201: Patch v1.0

https://bugs.webkit.org/attachment.cgi?id=438201&action=review




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 438201
  --> https://bugs.webkit.org/attachment.cgi?id=438201
Patch v1.0

View in context: https://bugs.webkit.org/attachment.cgi?id=438201&action=review

r=me, good catch :)

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:518
>	       item = this.findTreeElement(ancestors[i], isAncestor,
getParent);

Aside: I wonder if maybe this should just be
`this.getCachedTreeElement(ancestors[i])`, as there's really no point to
walking up the `getParent` chain again since we've already done that.  Tho you
might need to duplicate the bit that iterates over `this.children`, but
conceptually if we have an item inside `this.children` then it should already
be cached.

> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:520
> +		   return;

this should be `return null;` to match the other `return`