Correctly show self time in trace event details pie chart

Source/devtools/front_end/timeline/TimelineUIUtils.js

Index: Source/devtools/front_end/timeline/TimelineUIUtils.js
diff --git a/Source/devtools/front_end/timeline/TimelineUIUtils.js b/Source/devtools/front_end/timeline/TimelineUIUtils.js
index de69a605746b8ba491363105066310ffc51a1ff1..6449fcfb17fe67e898de405b10d7d2b359ae1a2c 100644
--- a/Source/devtools/front_end/timeline/TimelineUIUtils.js
+++ b/Source/devtools/front_end/timeline/TimelineUIUtils.js
@@ -738,7 +738,10 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
 {
     var fragment = document.createDocumentFragment();
     var stats = WebInspector.TimelineUIUtils._aggregatedStatsForTraceEvent(model, event);
-    fragment.appendChild(WebInspector.TimelineUIUtils.generatePieChart(stats));
+    if (stats.hasChildren)
+        fragment.appendChild(WebInspector.TimelineUIUtils.generatePieChart(stats.aggregatedStats, WebInspector.TimelineUIUtils.styleForTimelineEvent(event.name).category, event.selfTime));

[alph, 2014/05/29 10:11:16]

I'd move the fragment.appendChild out of the if.

[yurys, 2014/05/29 12:40:38]

Done.

+    else
+        fragment.appendChild(WebInspector.TimelineUIUtils.generatePieChart(stats.aggregatedStats));
 
     var recordTypes = WebInspector.TimelineModel.RecordType;
 
@@ -888,7 +891,7 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
 /**
  * @param {!WebInspector.TracingModel} model
  * @param {!WebInspector.TracingModel.Event} event
- * @return {!Object}
+ * @return {!{ aggregatedStats: !Object, hasChildren: boolean }}
  */
 WebInspector.TimelineUIUtils._aggregatedStatsForTraceEvent = function(model, event)
 {
@@ -903,20 +906,23 @@ WebInspector.TimelineUIUtils._aggregatedStatsForTraceEvent = function(model, eve
         return startTime - e.startTime;
     }
     var index = events.binaryIndexOf(event.startTime, eventComparator);

[alph, 2014/05/29 12:59:28]

nit: you can move it inside "if (endTime)"

+    var hasChildren = false;
     var aggregatedStats = {};
     var endTime = event.endTime;
-    if (!endTime)
-        return aggregatedStats;
-    for (; index < events.length; index++) {
-        var nextEvent = events[index];
-        if (nextEvent.startTime > endTime)
-            break;
-        if (!nextEvent.selfTime)
-            continue;
-        var category = WebInspector.TimelineUIUtils.styleForTimelineEvent(nextEvent.name).category.name;
-        aggregatedStats[category] = (aggregatedStats[category] || 0) + nextEvent.selfTime;
+    if (endTime) {
+        for (var i = index; i < events.length; i++) {
+            var nextEvent = events[i];
+            if (nextEvent.startTime > endTime)

[alph, 2014/05/29 10:11:16]

nit: >=
or even better compare to nextEvent.endTime

[yurys, 2014/05/29 12:40:38]

Done.

+                break;
+            if (!nextEvent.selfTime)
+                continue;
+            if (i > index)

[alph, 2014/05/29 10:11:16]

you can hoist the check out of the loop

[yurys, 2014/05/29 12:40:38]

This wouldn't work as we can have only instant event children in which case
hasChildren should be false.

+                hasChildren = true;
+            var category = WebInspector.TimelineUIUtils.styleForTimelineEvent(nextEvent.name).category.name;
+            aggregatedStats[category] = (aggregatedStats[category] || 0) + nextEvent.selfTime;
+        }
     }
-    return aggregatedStats;
+    return { aggregatedStats: aggregatedStats, hasChildren: hasChildren };
 }
 
 /**