Bug 37820

Summary:

Web Inspector: Aggregated time stats should be visible in Timeline record Popup

Product:

WebKit

Reporter:

Ilya Tikhonovsky <loislo>

Component:

Web Inspector (Deprecated)

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

pfeldman, timothy, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
screenshot of Popup with aggregated time info	none
[patch] initial version.	none
screenshot of Popup with aggregated time info and with self\|cpu\|total bars	none
[patch] second iteration.	loislo: commit-queue-
[patch] Second iteration.	pfeldman: review+

Ilya Tikhonovsky

Reported 2010-04-19 13:59:07 PDT

Created attachment 53716 [details] screenshot of Popup with aggregated time info %SUBJ%

Attachments
screenshot of Popup with aggregated time info (27.35 KB, image/png) 2010-04-19 13:59 PDT, Ilya Tikhonovsky	no flags	Details
[patch] initial version. (7.10 KB, patch) 2010-04-19 14:15 PDT, Ilya Tikhonovsky	no flags	Details Formatted Diff Diff
screenshot of Popup with aggregated time info and with self\|cpu\|total bars (14.15 KB, image/png) 2010-04-20 08:00 PDT, Ilya Tikhonovsky	no flags	Details
[patch] second iteration. (14.08 KB, patch) 2010-04-20 14:21 PDT, Ilya Tikhonovsky	loislo: commit-queue-	Details Formatted Diff Diff
[patch] Second iteration. (16.45 KB, patch) 2010-04-23 08:11 PDT, Ilya Tikhonovsky	pfeldman: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Ilya Tikhonovsky

Comment 1 2010-04-19 14:15:02 PDT

Created attachment 53721 [details] [patch] initial version.

Pavel Feldman

Comment 2 2010-04-19 14:41:13 PDT

Comment on attachment 53721 [details] [patch] initial version. I don't think that popover is informative enough. You might want to do a little pie chart or use legend with corresponding colors instead.

Ilya Tikhonovsky

Comment 3 2010-04-20 08:00:57 PDT

Created attachment 53819 [details] screenshot of Popup with aggregated time info and with self|cpu|total bars

Timothy Hatcher

Comment 4 2010-04-20 11:33:08 PDT

I really like the second version.

Timothy Hatcher

Comment 5 2010-04-20 11:33:29 PDT

Maybe we should hide anything with 0?

Ilya Tikhonovsky

Comment 6 2010-04-20 14:21:17 PDT

Created attachment 53880 [details] [patch] second iteration. has binary changes. 1) Legend boxes was added to the Aggregated Time row data in Popup; 2) three bars with different opacity are used for one record. Solid is for Self Time, with opacity 0.6 for aggregated cpu time of the record and it's children and with opacity 0.2 for duration; 3) Duration property is changed. Now it is time between startTime and lastChildEndTime (was time between start and end of real event); 4) Title of popup is fixed.

Ilya Tikhonovsky

Comment 7 2010-04-23 08:11:44 PDT

Created attachment 54160 [details] [patch] Second iteration. pushGCEvents crash was fixed. aggregated stats propagation to the parents was fixed. incorrect calculation of visible records count was fixed.

Pavel Feldman

Comment 8 2010-04-23 08:24:02 PDT

Comment on attachment 54160 [details] [patch] Second iteration. r+ with comments. please address them prior to landing. WebCore/inspector/InspectorTimelineAgent.cpp:59 + if (m_gcEvents.size()) { Replace with guard expression? WebCore/inspector/front-end/TimelinePanel.js:1005 + _calculateAggregatedStats: function(categories) It would be great to remove the 0.00001 hack and to do as much of this as possible lazily (i.e. only calculate self time on record add).

Ilya Tikhonovsky

Comment 9 2010-04-26 06:39:31 PDT

Committed r58242: <http://trac.webkit.org/changeset/58242>

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