Bug 36606 – Web Inspector: Short Records filter sould be implemented in Timeline Panel

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

[Ilya Tikhonovsky]

%SUBJ%

[Ilya Tikhonovsky]

Created attachment 51648 [details]
[patch] Initial version for review.

1) additional button was added to status bar. "Show small records";
2) initial state of that button is off;
3) all events that are 15ms or less will be hidden until Show small records" is off;

[Pavel Feldman]

Comment on attachment 51648 [details]
[patch] Initial version for review.

Overall looks good. Please attach screenshot and fix comments!

> +    _toggleFilterButtonClicked: function()
> +    {
> +        this.toggleFilterButton.toggled = !this.toggleFilterButton.toggled;
> +        this._calculator._showShortEvents = this.toggleFilterButton.toggled;
> +        this._scheduleRefresh();

I think you want immediate refresh here.

>              }
> +            record = formattedRecord;
> +            while (record._isLongEvent && record.parent && !record.parent._isLongEvent) {
> +                record = record.parent;
> +                record._hasLongChildrenEvents = true;

Comment this?

> -        if (record.children.length) {
> +        if (record.visibleChildrenCount || (record.collapsed && record.children.length && (record._hasLongChildrenEvents || calculator._showShortEvents))) {

Comment would be nice. Something like: We show expand element for expanded nodes or for nodes that are collapsed and have unfiltered children, or we are not filtering.

>              this._expandElement.style.top = index * this._rowHeight + "px";
>              this._expandElement.style.left = barPosition.left + "px";
>              this._expandElement.style.width = Math.max(12, barPosition.width + 25) + "px";
> @@ -737,6 +758,12 @@ WebInspector.TimelinePanel.FormattedRecord = function(record, parentRecord, reco
>  }
>  
>  WebInspector.TimelinePanel.FormattedRecord.prototype = {
> +    get _isLongEvent()


This should be a function, not getter.

> +    {
> +        const shortEventLength = 0.015;
> +        return this._hasLongChildrenEvents || (this._lastChildEndTime - this.startTime) > shortEventLength;

No need for _hasLongChildrenEvents here.

> +    },
> +

[Ilya Tikhonovsky]

Created attachment 51659 [details]
[patch] second iteration.

1) comment about _hasLongChildrenEvents propagation was added;
2) scheduleRefresh(true);
3) complex boolean expression was redesigned and looks much clear, no comments required I think.

[Pavel Feldman]

Comment on attachment 51659 [details]
[patch] second iteration.

> +            while (record._isLongEvent() && record.parent && !record.parent._isLongEvent()) {
> +                record = record.parent;

No need to call _isLongEvent twice on the same record. I can optimize it while landing.

I'll land manually.

[Pavel Feldman]

Slight variation landed as:

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/English.lproj/localizedStrings.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/inspector.css
Committed r56613