Correctly show self time in trace event details pie chart

Source/devtools/front_end/timeline/TimelineUIUtils.js

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  * Copyright (C) 2012 Intel Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
@@ skipping 720 matching lines @@
  * @param {?WebInspector.DOMNode} relatedNode
  * @param {boolean} loadedFromFile
  * @param {?WebInspector.TimelineTraceEventBindings} bindings
  * @param {!WebInspector.Target} target
  * @return {!DocumentFragment}
  */
 WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(event, model, linkifier, imagePreviewElement, relatedNode, loadedFromFile, bindings, target)
 {
     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;
 
     // The messages may vary per event.name;
     var callSiteStackTraceLabel;
     var callStackLabel;
     var relatedNodeLabel;
 
     var contentHelper = new WebInspector.TimelineDetailsContentHelper(target, linkifier, true);
     contentHelper.appendTextRow(WebInspector.UIString("Self Time"), Number.millisToString(event.selfTime / 1000, true));
@@ skipping 129 matching lines @@
         div.textContent = warning;
         contentHelper.appendElementRow(WebInspector.UIString("Warning"), div);
     }
     fragment.appendChild(contentHelper.element);
     return fragment;
 }
 
 /**
  * @param {!WebInspector.TracingModel} model
  * @param {!WebInspector.TracingModel.Event} event
- * @return {!Object}
+ * @return {!{ aggregatedStats: !Object, hasChildren: boolean }}
  */
 WebInspector.TimelineUIUtils._aggregatedStatsForTraceEvent = function(model, event)
 {
     var events = model.inspectedTargetEvents();
     /**
      * @param {number} startTime
      * @param {!WebInspector.TracingModel.Event} e
      * @return {number}
      */
     function eventComparator(startTime, e)
     {
         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 };
 }
 
 /**
  * @param {!Array.<number>} quad
  * @return {number}
  */
 WebInspector.TimelineUIUtils._quadHeight = function(quad)
 {
     return Math.round(Math.sqrt(Math.pow(quad[0] - quad[6], 2) + Math.pow(quad[1] - quad[7], 2)));
 }
@@ skipping 449 matching lines @@
         for (var i = 0; i < stackTrace.length; ++i) {
             var stackFrame = stackTrace[i];
             var row = stackTraceElement.createChild("div");
             row.createTextChild(stackFrame.functionName || WebInspector.UIString("(anonymous function)"));
             row.createTextChild(" @ ");
             var urlElement = this._linkifier.linkifyLocation(this._target, stackFrame.url, stackFrame.lineNumber - 1);
             row.appendChild(urlElement);
         }
     }
 }