[webkit-reviews] review granted: [Bug 194110] Web Inspector: Timeline graphs have drawing issues with multiple discontinuities : [Attachment 360782] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 194110: Web Inspector: Timeline graphs have drawing issues with multiple
discontinuities
https://bugs.webkit.org/show_bug.cgi?id=194110

Attachment 360782: [PATCH] Proposed Fix

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




--- Comment #3 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 360782
  --> https://bugs.webkit.org/attachment.cgi?id=360782
[PATCH] Proposed Fix

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

rs=me, with some issues/questions

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:145
>	       dataPoints.push({time, size: usage});

Unrelated: should we be using `size` here?  What about `mainThreadUsage` and
`nonMainThreadUsage`?

> Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js:203
> +	       insertDiscontinuity.call(this, previousRecord,
discontinuities[0], discontinuities[0], null);

Is it necessary to also update the equivalent of this in
>CPUTimelineView.js:151?

> Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js:-252
> -	       let size = new WI.Size(xScale(graphEndTime),
memoryCategoryViewHeight);

Why did this move?  We need `size` to be defined in `yScale`.