Bug 31784 – Web Inspector: Reimplement TimelinePanel to make it scale

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Pavel Feldman]

Today instrumenting 30 seconds of cnn.com or techcrunch.com makes timeline unresponsive. The reason is that several thousands or recurds result in proportional number of elements attached to the dom.

What this change does:
- Extracts grid and overview classes into separate files
- Makes timeline create only divs for visible rows
- It makes timeline really fast and scalable. We can further optimize it via bucketing events by time if that is needed (like in order to handle >100K entries).

[Pavel Feldman]

Created attachment 43675 [details]
[PATCH] the patch.

[Timothy Hatcher]

Comment on attachment 43675 [details]
[PATCH] the patch.

> +        var visibleTop = this._containerElement.scrollTop;
> +        var visibleBottom = visibleTop + this._containerElement.clientHeight;
> +        var rowHeight = 18;
> +        var startIndex = Math.max(0, Math.floor(visibleTop / rowHeight) - 1);
> +        var endIndex = Math.min(recordsInWindow.length, Math.ceil(visibleBottom / rowHeight));

Use const instead of var for these.


> +        var top = (startIndex * rowHeight) + "px";

Use const.


>  .sidebar {
>      position: absolute;
>      top: 0;
> +    min-height: 100%;
>      left: 0;
> -    bottom: 0;

Why was this needed?


>  .sidebar-resizer-vertical {
>      position: absolute;
>      top: 0;
> -    bottom: 0;
> +    min-height: 100%;

Ditto.


> +.timeline-tree-section {

Is this class uses in your patch? What is this for?


> +    color: rgb(92, 110, 129);
> +    font-family: Helvetica, Arial, sans-serif;

Why is this Helvetica? We don't use Helvetica except in a few rare places. Otherwise the font is inherited from the body.


> +    position: relative;

Why relative, so it have positioned children?


> +    text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px;

Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision anyway.


> +    font-family: Helvetica, Arial, sans-serif;

Why specify the font?

[Pavel Feldman]

> Use const instead of var for these.
> 

Done.

> 
> Use const.
> 

Done.

> 
> >  .sidebar {
> >      position: absolute;
> >      top: 0;
> > +    min-height: 100%;
> >      left: 0;
> > -    bottom: 0;
> 
> Why was this needed?
> 

I shared an image on IRC.

> 
> > +.timeline-tree-section {
> 
> Is this class uses in your patch? What is this for?
> 

Should not be here - from a previous version of patch. Removed.


> Why is this Helvetica? We don't use Helvetica except in a few rare places.
> Otherwise the font is inherited from the body.
> Why relative, so it have positioned children?

I was experimenting and forgot to delete. from a previous version of patch. Removed.


> > +    text-shadow: rgba(255, 255, 255, 0.746094) 0px 1px 0px;
> 
> Just use 0.75 instead of 0.746094. The CSS parser ignores the extra precision
> anyway.
> 

That was a copypaste from inspector. Fixed.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/inspector/timeline-test.js
	M	WebCore/ChangeLog
	M	WebCore/WebCore.gypi
	M	WebCore/WebCore.vcproj/WebCore.vcproj
	M	WebCore/inspector/front-end/AbstractTimelinePanel.js
	A	WebCore/inspector/front-end/TimelineGrid.js
	A	WebCore/inspector/front-end/TimelineOverviewPane.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/WebKit.qrc
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/inspector/front-end/inspector.html
Committed r51296