[webkit-reviews] review granted: [Bug 173746] Web Inspector: Script Timeline bubbles sometimes appear to miss large events : [Attachment 313672] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has granted Joseph Pecoraro <joepeck at webkit.org>'s
request for review:
Bug 173746: Web Inspector: Script Timeline bubbles sometimes appear to miss
large events
https://bugs.webkit.org/show_bug.cgi?id=173746

Attachment 313672: [PATCH] Proposed Fix

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




--- Comment #4 from Brian Burg <bburg at apple.com> ---
Comment on attachment 313672
  --> https://bugs.webkit.org/attachment.cgi?id=313672
[PATCH] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Models/Timeline.js:74
> +	   // Because records can be nested, its posisble that outer records
with an early start time

Nit: possible

> Source/WebInspectorUI/UserInterface/Models/Timeline.js:133
> +    _tryInsertInSortedOrder(record)

The name seems funny to me without mentioning 'record' somehow. Maybe
Objective-C has corrupted me.

> Source/WebInspectorUI/UserInterface/Models/Timeline.js:144
> +	   let end = Math.max(this._records.length - 20, 0);

Likewise, these were terse and I thought they were backwards given the for loop
header. Maybe earliestRecordIndex and latestRecordIndex?