| Summary: | Web Inspector: Update the current WebInspector.TimelineView when time range changes | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||
| Component: | Web Inspector | Assignee: | Timothy Hatcher <timothy> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | graouts, joepeck, timothy, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
Timothy Hatcher
2014-01-08 14:50:28 PST
Created attachment 220671 [details]
Patch
Comment on attachment 220671 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220671&action=review r=me > Source/WebInspectorUI/UserInterface/TimelineContentView.js:196 > + var endTime = Math.max(WebInspector.timelineManager.recording.endTime || startTime, startTime); Hmm, this doesn't sound right. Maybe the "|| startTime" should just go away, no? > Source/WebInspectorUI/UserInterface/TimelineOverview.js:167 > + // Update all ruler element to the new required width. Typo: "all ruler" => "ruler" > Source/WebInspectorUI/UserInterface/TimelineView.js:169 > + this._scheduledLayoutUpdateIdentifier = requestAnimationFrame(this.updateLayout.bind(this)); How often will we updateLayout? Maybe we should cache an updateLayout.bind(this) to prevent function creation churn? Comment on attachment 220671 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=220671&action=review >> Source/WebInspectorUI/UserInterface/TimelineContentView.js:196 >> + var endTime = Math.max(WebInspector.timelineManager.recording.endTime || startTime, startTime); > > Hmm, this doesn't sound right. Maybe the "|| startTime" should just go away, no? I meant to revet this line. At one point I had other doe in there for the Math.max. Now WebInspector.timelineManager.recording.endTime is always what I want. >> Source/WebInspectorUI/UserInterface/TimelineView.js:169 >> + this._scheduledLayoutUpdateIdentifier = requestAnimationFrame(this.updateLayout.bind(this)); > > How often will we updateLayout? Maybe we should cache an updateLayout.bind(this) to prevent function creation churn? Good idea. I'll do that in a follow up with all the places this happens. |