[webkit-reviews] review granted: [Bug 127900] Web Inspector: Show profile data in the discrete Scripts timeline view : [Attachment 222653] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Timothy Hatcher
<timothy at apple.com>'s request for review:
Bug 127900: Web Inspector: Show profile data in the discrete Scripts timeline
view
https://bugs.webkit.org/show_bug.cgi?id=127900

Attachment 222653: Patch
https://bugs.webkit.org/attachment.cgi?id=222653&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=222653&action=review


r=me

> Source/WebInspectorUI/ChangeLog:3
> +	   Show profile data in the desecrate Scripts timeline view.

Typo: "desecrate"

> Source/WebInspectorUI/UserInterface/NavigationSidebarPanel.js:543
> +	       // Keep all the elements at the same depth once the maximum is
reached.
> +	       childrenSubstring += i === maximumSidebarTreeDepth ? "
.children" : " > .children";
> +	       styleText += "." +
WebInspector.NavigationSidebarPanel.ContentTreeOutlineElementStyleClassName +
childrenSubstring + " > .item { ";

This must be some kind of a record for longest selector.

> Source/WebInspectorUI/UserInterface/ProfileNodeDataGridNode.js:35
> +    this._rangeEndTime = rangeEndTime || Infinity;

"0 || 0" is harmless, but  "0 || Infinity" could lead to a crazy accident.
Maybe we should be a bit safer here?

> Source/WebInspectorUI/UserInterface/ScriptTimelineDataGridNode.js:33
> +    this._rangeEndTime = rangeEndTime || Infinity;

Ditto, 0 || Infinity.

> Source/WebInspectorUI/UserInterface/ScriptTimelineView.js:119
> +		   dataGridNode = dataGridNode.traverseNextNode(false, null,
true);

Gosh, these traverse functions need a better name to read at call sites. Same
with that reveal function with a bunch of bools.

> Source/WebInspectorUI/UserInterface/TimelineDataGrid.js:274
> +	   // Collect parent nodes htat need their children sorted. So this in
two phases since

Typo: "htat"