Bug 36122 - Web Inspector: Timeline improvements next iteration
: Web Inspector: Timeline improvements next iteration
Status: RESOLVED FIXED
: WebKit
Web Inspector (Deprecated)
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-03-15 09:13 PST by
Modified: 2010-03-17 07:09 PST (History)


Attachments
Screenshot (113.60 KB, image/png)
2010-03-15 09:55 PST, Ilya Tikhonovsky
no flags Details
[Patch]: Initial version of patch. (27.37 KB, patch)
2010-03-16 01:00 PST, Ilya Tikhonovsky
pfeldman: review+
Review Patch | Details | Formatted Diff | Diff
[Patch (33.05 KB, application/octet-stream)
2010-03-17 06:25 PST, Ilya Tikhonovsky
no flags Details
[Patch] second revision. (32.91 KB, patch)
2010-03-17 06:36 PST, Ilya Tikhonovsky
pfeldman: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-03-15 09:13:30 PST
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.
------- Comment #1 From 2010-03-15 09:55:29 PST -------
Created an attachment (id=50716) [details]
Screenshot
------- Comment #2 From 2010-03-16 01:00:49 PST -------
Created an attachment (id=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.
------- Comment #3 From 2010-03-16 03:05:51 PST -------
(From update of attachment 50774 [details])
>          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.
------- Comment #4 From 2010-03-16 05:58:08 PST -------
(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?
------- Comment #5 From 2010-03-16 08:16:47 PST -------
> > 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.
------- Comment #6 From 2010-03-17 06:25:19 PST -------
Created an attachment (id=50898) [details]
[Patch
------- Comment #7 From 2010-03-17 06:36:12 PST -------
Created an attachment (id=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.
------- Comment #8 From 2010-03-17 06:40:40 PST -------
(From update of attachment 50899 [details])
> +
> +    reset: function()
> +    {
> +    },
> +

Why adding this?
------- Comment #9 From 2010-03-17 06:41:20 PST -------
[+knorton] FYI: This is likely to break speed tracer.
------- Comment #10 From 2010-03-17 07:09:05 PST -------
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