[webkit-reviews] review granted: [Bug 239872] Web Inspector: Regression(r267038) Import a timeline does not render the timeline, only lists the events : [Attachment 458560] 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 239872: Web Inspector: Regression(r267038) Import a timeline does not
render the timeline, only lists the events
https://bugs.webkit.org/show_bug.cgi?id=239872

Attachment 458560: Patch v1.0

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




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

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

r=me, nice fix =D

> Source/WebInspectorUI/ChangeLog:18
> +	   The second issue is that when importing a timeline, we should ignore
the current capturing state when setting
> +	   the current, start, and end times, otherwise not all the values will
be commited since most likely no recording
> +	   is in progress at the same time a recording is being imported.

Along these lines, i feel like we should always ignore the current capturing
state.	We don't want an imported recording to suddenly start showing new data
because we started another recording.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js:127
> +	       this._updateTimes(startTime, endTime, endTime, {fromImport:
true});

NIT: do we actually need a parameter, or can we just use
`this._recording.imported` instead at each usage?