[webkit-reviews] review denied: [Bug 136351] Web Inspector: timelines should not count time elapsed while paused in the debugger : [Attachment 238789] Patch

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at apple.com> has denied Brian Burg
<burg at cs.washington.edu>'s request for review:
Bug 136351: Web Inspector: timelines should not count time elapsed while paused
in the debugger
https://bugs.webkit.org/show_bug.cgi?id=136351

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

------- Additional Comments from Timothy Hatcher <timothy at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=238789&action=review


> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:134
> +	   this._capturingStartedTimestamp = Date.now();

The device time might be drastically different from the local machine's time.
So this has a high likelihood of never meeting the "recordPayload.startTime >
this._capturingStartedTimestamp" check in processRecord. Even if the device
time and local machine time are in sync, the protocol delay can cause the
backend times to be a few ms earlier, so the first few records would have bad
time values.

The Timeline rulers and graphs simply us TimelineRecording.startTime, which is
the earliest record startTime seen. It should always be the first record. So
maybe in processRecord, just do something like:

if (isNaN(this._capturingStartedTimestamp))
    this._capturingStartedTimestamp = recordPayload.startTime;