Bug 153392 – Web Inspector: Reduce unnecessary forced layouts in TimelineOverview

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

* SUMMARY
Reduce unnecessary forced layouts in TimelineOverview.

It looks like autoscroll when not autoscrolling would set the scrollContainerElement.scrollLeft to 0 (which it already was) and force a synchronous layout. Seems we can just skip this work if the calculated scrollLeft value would have been 0. This eliminated the forced layouts I saw triggered by TimelineOverview recording reloads of numerous pages.

[Radar WebKit Bug Importer]

<rdar://problem/24312344>

[Joseph Pecoraro]

Created attachment 269646 [details]
[PATCH] Proposed Fix

[Timothy Hatcher]

Comment on attachment 269646 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:390
> +            if (scrollLeft)

Does this need to be: scrollLeft || this._scrollContainerElement.scrollLeft

That way if it is no zero already it will set it still. Or scrollLeft !== this._scrollContainerElement.scrollLeft

[Joseph Pecoraro]

(In reply to comment #3)
> Comment on attachment 269646 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=269646&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:390
> > +            if (scrollLeft)
> 
> Does this need to be: scrollLeft || this._scrollContainerElement.scrollLeft
> 
> That way if it is no zero already it will set it still. Or scrollLeft !==
> this._scrollContainerElement.scrollLeft

This was intentional.

I do not want to actually call `element.scrollLeft` since that could force a layout. I don't think autoscroll will every intentionally scroll you to zero. Starting a new recording automatically resets the scroll position anyways. So this was a cheap hack, if the scrollLeft we calculate is zero, do no work. I didn't find any cases where this was a problem, and I saw fewer (zero) forced layouts caused by TimelineOverview in reload recordings that didn't cause the overview to auto-scroll.

[WebKit Commit Bot]

Comment on attachment 269646 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 269646

Committed r195544: <http://trac.webkit.org/changeset/195544>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.