Bug 151410 – Web Inspector: Convert remaining timeline views to use View base class

• Component: Web Inspector
• Status: RESOLVED FIXED

[Matt Baker]

* SUMMARY
Convert remaining timeline views to use View base class.

[Radar WebKit Bug Importer]

<rdar://problem/23601828>

[Matt Baker]

Created attachment 265786 [details]
[Patch] Proposed Fix

[Blaze Burg]

Comment on attachment 265786 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js:214
> +        this._scheduledSelectedRecordLayoutUpdateIdentifier = requestAnimationFrame(() => {

Maybe I'm just forgetting, but why do the graphs have their own rAF system? Is it a fast-path for non-resizing relayout?

[Matt Baker]

Comment on attachment 265786 [details]
[Patch] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js:214
>> +        this._scheduledSelectedRecordLayoutUpdateIdentifier = requestAnimationFrame(() => {
> 
> Maybe I'm just forgetting, but why do the graphs have their own rAF system? Is it a fast-path for non-resizing relayout?

I added this for https://bugs.webkit.org/show_bug.cgi?id=147013. It's a hook for derived TimelineOverviewGraphs to style the selected record without laying out everything else (similar to ruler markers/selection). Currently it's only used by the rendering frames graph.

[Timothy Hatcher]

Comment on attachment 265786 [details]
[Patch] Proposed Fix

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

>>> Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js:214
>>> +        this._scheduledSelectedRecordLayoutUpdateIdentifier = requestAnimationFrame(() => {
>> 
>> Maybe I'm just forgetting, but why do the graphs have their own rAF system? Is it a fast-path for non-resizing relayout?
> 
> I added this for https://bugs.webkit.org/show_bug.cgi?id=147013. It's a hook for derived TimelineOverviewGraphs to style the selected record without laying out everything else (similar to ruler markers/selection). Currently it's only used by the rendering frames graph.

Why does it need to happen on an animation frame?

[WebKit Commit Bot]

Comment on attachment 265786 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 265786

Committed r192616: <http://trac.webkit.org/changeset/192616>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Matt Baker]

(In reply to comment #5)
> Comment on attachment 265786 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265786&action=review
> 
> >>> Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js:214
> >>> +        this._scheduledSelectedRecordLayoutUpdateIdentifier = requestAnimationFrame(() => {
> >> 
> >> Maybe I'm just forgetting, but why do the graphs have their own rAF system? Is it a fast-path for non-resizing relayout?
> > 
> > I added this for https://bugs.webkit.org/show_bug.cgi?id=147013. It's a hook for derived TimelineOverviewGraphs to style the selected record without laying out everything else (similar to ruler markers/selection). Currently it's only used by the rendering frames graph.
> 
> Why does it need to happen on an animation frame?

It probably doesn't. We should revisit this when we add record selection to the other timeline graphs.