Bug 135307 – Web Inspector: Should be a way to go directly from an event in the overview view to the specialized timeline for that event

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

[Joseph Pecoraro]

* SUMMARY
In the generic timeline overview view we see events like Forced Layout, or Mouse Down Event Dispatched. In both cases, to find out more information about those individual events you must switch to the specialized timeline (e.g. Layout duration + backtrace, e.g. JavaScript duration and profiling information). Currently there is no way to go directly from the overview to the deeper information about a particular event.

[Radar WebKit Bug Importer]

<rdar://problem/17816221>

[Joseph Pecoraro]

<rdar://problem/17273966>

[Joseph Pecoraro]

This might also be a good opportunity for a force click on hardware that supports it.

[Devin Rousso]

Created attachment 352122 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 352122 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js:734
> +            if (firstRecord.startTime < this.selectionStartTime || lastRecord.endTime > this.selectionStartTime + this.selectionDuration) {
> +                let selectionPadding = this.secondsPerPixel * 10;

Neat!

> Source/WebInspectorUI/UserInterface/Views/TimelineOverviewGraph.js:40
>          this._selectedRecord = null;
> -        this._selectedRecordChanged = false;
> +        this._selectedRecordBar = null;

It seems weird but possible that the selectedRecord and selectedRecordBar might get out of sync. Can we add an assertion somewhere (such as inside of `set selectedRecord` that the record is inside of the selectedRecordBar if that is available)?

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:98
> +.timeline-record-bar:not(.selected).timeline-record-type-network > .segment {

Can the selection colors above just be "!important" and avoid the :not(.selected) classes? I think that is normally how we handle selection styles.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:37
> +        this._element.addEventListener("click", this._handleClick.bind(this));

I wonder about the performance (CPU and Memory) different between doing a bind like this versus doing an arrow function. I mused about this once over here:
https://bugs.webkit.org/show_bug.cgi?id=158315

Compare:

    this._element.addEventListener("click", this._handleClick.bind(this));
    this._element.addEventListener("click", (e) => { this._handleClick(e); });

This is in my head because:

    • Function.prototype.bind is a function call (maybe it gets optimized away though?)
    • JSBoundFunction has extra memory (maybe this has been optimized away for no-args as well?)

This might be a fun thing to measure some time in the future. It just always jumps out at me in Timeline code for things we may be creating many many object instances (like this), how much time we are likely spending inside of Function.prototype.bind.

> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.js:188
> +    set selected(selected)
> +    {
> +        this._element.classList.toggle("selected", !!selected);
> +    }

We should add a getter just for consistency.

[Devin Rousso]

Comment on attachment 352122 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordBar.css:98
>> +.timeline-record-bar:not(.selected).timeline-record-type-network > .segment {
> 
> Can the selection colors above just be "!important" and avoid the :not(.selected) classes? I think that is normally how we handle selection styles.

Really?  Where do we do this?  I personally avoid `!Important` wherever I can as it can really be annoying to deal with if future changes are needed.

[Devin Rousso]

Created attachment 352463 [details]
Patch

[Devin Rousso]

Created attachment 352479 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 352479 [details]
Patch

Clearing flags on attachment: 352479

Committed r237195: <https://trac.webkit.org/changeset/237195>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.