Bug 36122 – Web Inspector: Timeline improvements next iteration

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

[Ilya Tikhonovsky]

Top level records should be collapsed.
Virtually linked events should be nested in Timeline like Send Request and corresponding Received Responce, Timer Install and Timer Fire etc.
It should be possible to see Main Resource request.

[Ilya Tikhonovsky]

Created attachment 50716 [details]
Screenshot

[Ilya Tikhonovsky]

Created attachment 50774 [details]
[Patch]: Initial version of patch.

The patch with Timeline improvements
1) Timer Fire events are connecting to Timer Install events;
2) Receive Response and Finish loading events are connecting to Send Request events;
3) The Receive Response event grab point is moved to ResourceLoader and as result can have nested events like Parse etc;
4) Top Level events are in collapsed state by default;
5) The events can be glued at any level of event hierarchy;
6) new semitransparent bar was added for connected events. As example if timer install event got 10 ms and have timeout 100ms then timer install event will have solid bar for 10ms and semitransparent for 100ms + Timer Fire event length. The same for Send Request and Receive Response etc;
7) Tests were fixed;
8) Timeline panel will not be cleared at page reload. That will allow us to see Send Request for main resource and Event(unload);
9) But with zero based Timer timeout was fixed for Timer Install details popup;
10) Some other properties of Send Request event were added to Details popup.

[Pavel Feldman]

Comment on attachment 50774 [details]
[Patch]: Initial version of patch.

>          if (sendRequestRecord) { // False if we started instrumentation in the middle of request.
>              sendRequestRecord._responseReceivedFormattedTime = this.startTime;
>              record.data.url = sendRequestRecord.data.url;
> -            this.startTime = sendRequestRecord.startTime;
>              this.details = this._getRecordDetails(sendRequestRecord, sendRequestRecords);
>              this.callerScriptName = sendRequestRecord.callerScriptName;
>              this.callerScriptLine = sendRequestRecord.callerScriptLine;
> @@ -647,7 +703,6 @@ WebInspector.TimelinePanel.FormattedRecord = function(record, recordStyles, send
>          var sendRequestRecord = sendRequestRecords[record.data.identifier];
>          if (sendRequestRecord) {// False for main resource.
>              record.data.url = sendRequestRecord.data.url;
> -            this.startTime = sendRequestRecord._responseReceivedFormattedTime;
>              this.callerScriptName = sendRequestRecord.callerScriptName;
>              this.callerScriptLine = sendRequestRecord.callerScriptLine;
>          }

We could get rid of these as a whole.

> +                    recordContentTable.appendChild(this._createRow(WebInspector.UIString("Is Main Resource"), this.data.isMainResource));
> +                if (typeof this.data.statusCode === "number")

Not sure we need this one.

[Timothy Hatcher]

(In reply to comment #2)

> 1) Timer Fire events are connecting to Timer Install events;
> 2) Receive Response and Finish loading events are connecting to Send Request
> events;

I'm not sure this makes sense. I would expect a Timer container that has two children "Timer Install" and "Timer Fire". Similar for Resource loading.

> 6) new semitransparent bar was added for connected events. As example if timer
> install event got 10 ms and have timeout 100ms then timer install event will
> have solid bar for 10ms and semitransparent for 100ms + Timer Fire event
> length. The same for Send Request and Receive Response etc;

I don't think this makes sense either. I don't expect a bar to stretch the whole way. Code hasn't been running that entire time. I only expect to see bars where code actually was running. SO I would expect two bar segments seperated by empty space on one row for the collapsed container. Expending it would show the seperate peices as different rows.

> 8) Timeline panel will not be cleared at page reload. That will allow us to see
> Send Request for main resource and Event(unload);

When and how is the panel cleared now?

[Ilya Tikhonovsky]

> > 1) Timer Fire events are connecting to Timer Install events;
> > 2) Receive Response and Finish loading events are connecting to Send Request events;

> I'm not sure this makes sense. I would expect a Timer container that has two
> children "Timer Install" and "Timer Fire". Similar for Resource loading.

It is not clear for me why we need some additional element as a container for Timer Install and Timer fire events.
Right now there is clear cause and effect relationship between Timer Install and Timer Fire/Timer Remove events.

Some sites use nonrepeatable timers reinstalled each time at Timer Fire event and in that case 
we will have a third more records in the Timeline for the same amount of information.

> > 6) new semitransparent bar was added for connected events. As example if timer
> > install event got 10 ms and have timeout 100ms then timer install event will
> > have solid bar for 10ms and semitransparent for 100ms + Timer Fire event
> > length. The same for Send Request and Receive Response etc;
> 
> I don't think this makes sense either. I don't expect a bar to stretch the
> whole way. Code hasn't been running that entire time. I only expect to see bars
> where code actually was running. SO I would expect two bar segments seperated
> by empty space on one row for the collapsed container. Expending it would show
> the seperate peices as different rows.

We can have two bars on the same row but I think we need something for linking these bars together.
In the other case it will be not clear for users if the second bar is filtered out by Timeline Overview.
I think we can use some gray semitransparent background bar for that.

The last idea about second bar was to have aggregated information about all the children events.
Like: <RecordTime in ms>--gray bar b.g.--<% of loading|% of rendering|% of scripting>---gray bar b.g.------->

> 
> > 8) Timeline panel will not be cleared at page reload. That will allow us to see
> > Send Request for main resource and Event(unload);
> 
> When and how is the panel cleared now?

Only by Clear button.

[Ilya Tikhonovsky]

Created attachment 50898 [details]
[Patch

[Ilya Tikhonovsky]

Created attachment 50899 [details]
[Patch] second revision.

The patch with Timeline improvements
0) rebaselined;
1) _topLevelRecord was renamed to _rootRecord;
2) willResourceReceiveData didResourceReceiveData was added for better grouping of inner events;
3) the size of parent bars is adjusted a bit.

[Pavel Feldman]

Comment on attachment 50899 [details]
[Patch] second revision.

> +
> +    reset: function()
> +    {
> +    },
> +

Why adding this?

[Pavel Feldman]

[+knorton] FYI: This is likely to break speed tracer.

[Pavel Feldman]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/inspector/timeline-enum-stability-expected.txt
	M	LayoutTests/inspector/timeline-network-resource-expected.txt
	M	LayoutTests/inspector/timeline-test.js
	M	WebCore/ChangeLog
	M	WebCore/inspector/InspectorController.cpp
	M	WebCore/inspector/InspectorTimelineAgent.cpp
	M	WebCore/inspector/InspectorTimelineAgent.h
	M	WebCore/inspector/TimelineRecordFactory.cpp
	M	WebCore/inspector/TimelineRecordFactory.h
	M	WebCore/inspector/front-end/TimelineAgent.js
	M	WebCore/inspector/front-end/TimelinePanel.js
	M	WebCore/inspector/front-end/inspector.css
	M	WebCore/loader/ResourceLoader.cpp
Committed r56108