Skip to content

Commit

Permalink
DevTools: make all timeline trace events use 0-based line numbers
Browse files Browse the repository at this point in the history
Review-Url: https://codereview.chromium.org/2169153002
Cr-Commit-Position: refs/heads/master@{#407021}
  • Loading branch information
a1ph authored and Commit bot committed Jul 22, 2016
1 parent 5db55e4 commit 4c68e86
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 87 deletions.
12 changes: 0 additions & 12 deletions front_end/components/Linkifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,18 +223,6 @@ WebInspector.Linkifier.prototype = {
return this.maybeLinkifyScriptLocation(target, callFrame.scriptId, callFrame.url, callFrame.lineNumber, callFrame.columnNumber, classes);
},

/**
* @param {?WebInspector.Target} target
* @param {!RuntimeAgent.CallFrame} callFrame
* @param {string=} classes
* @return {?Element}
*/
maybeLinkifyConsoleCallFrameForTracing: function(target, callFrame, classes)
{
// TODO(kozyatinskiy): remove this when tracing will migrate to 0-based lineNumber and columnNumber.
return this.linkifyScriptLocation(target, callFrame.scriptId, callFrame.url, callFrame.lineNumber - 1, callFrame.columnNumber - 1, classes);
},

/**
* @param {!WebInspector.Target} target
* @param {!RuntimeAgent.StackTrace} stackTrace
Expand Down
21 changes: 9 additions & 12 deletions front_end/profiler/HeapProfileView.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,18 +221,15 @@ WebInspector.SamplingHeapProfileHeader.prototype = {
*/
WebInspector.SamplingHeapProfileNode = function(node)
{
if (node.callFrame) {
WebInspector.ProfileNode.call(this, node.callFrame);
} else {
// Backward compatibility for old SamplingHeapProfileNode format.
var frame = /** @type {!RuntimeAgent.CallFrame} */(node);
WebInspector.ProfileNode.call(this, {
functionName: frame.functionName,
scriptId: frame.scriptId, url: frame.url,
lineNumber: frame.lineNumber - 1,
columnNumber: frame.columnNumber - 1
});
}
var callFrame = node.callFrame || /** @type {!RuntimeAgent.CallFrame} */ ({
// Backward compatibility for old CpuProfileNode format.
functionName: node["functionName"],
scriptId: node["scriptId"],
url: node["url"],
lineNumber: node["lineNumber"] - 1,
columnNumber: node["columnNumber"] - 1
});
WebInspector.ProfileNode.call(this, callFrame);
this.self = node.selfSize;
}

Expand Down
33 changes: 15 additions & 18 deletions front_end/sdk/CPUProfileDataModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,24 @@
/**
* @constructor
* @extends {WebInspector.ProfileNode}
* @param {!ProfilerAgent.CPUProfileNode} sourceNode
* @param {!ProfilerAgent.CPUProfileNode} node
* @param {number} sampleTime
*/
WebInspector.CPUProfileNode = function(sourceNode, sampleTime)
WebInspector.CPUProfileNode = function(node, sampleTime)
{
if (sourceNode.callFrame) {
WebInspector.ProfileNode.call(this, sourceNode.callFrame);
} else {
// Backward compatibility for old CPUProfileNode format.
var frame = /** @type {!RuntimeAgent.CallFrame} */(sourceNode);
WebInspector.ProfileNode.call(this, {
functionName: frame.functionName,
scriptId: frame.scriptId, url: frame.url,
lineNumber: frame.lineNumber - 1,
columnNumber: frame.columnNumber - 1
});
}
this.id = sourceNode.id;
this.self = sourceNode.hitCount * sampleTime;
this.positionTicks = sourceNode.positionTicks;
this.deoptReason = sourceNode.deoptReason;
var callFrame = node.callFrame || /** @type {!RuntimeAgent.CallFrame} */ ({
// Backward compatibility for old SamplingHeapProfileNode format.
functionName: node["functionName"],
scriptId: node["scriptId"],
url: node["url"],
lineNumber: node["lineNumber"] - 1,
columnNumber: node["columnNumber"] - 1
});
WebInspector.ProfileNode.call(this, callFrame);
this.id = node.id;
this.self = node.hitCount * sampleTime;
this.positionTicks = node.positionTicks;
this.deoptReason = node.deoptReason;
}

WebInspector.CPUProfileNode.prototype = {
Expand Down
2 changes: 1 addition & 1 deletion front_end/sdk/TracingModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ WebInspector.TracingModel.Event = function(categories, name, phase, startTime, t
this.warning = null;
/** @type {?WebInspector.TracingModel.Event} */
this.initiator = null;
/** @type {?Array.<!RuntimeAgent.CallFrame>} */
/** @type {?Array<!RuntimeAgent.CallFrame>} */
this.stackTrace = null;
/** @type {?Element} */
this.previewElement = null;
Expand Down
2 changes: 1 addition & 1 deletion front_end/sources/CallStackSidebarPane.js
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ WebInspector.CallStackSidebarPane.prototype = {
},

/**
* @param {!Array.<!RuntimeAgent.CallFrame>} callFrames
* @param {!Array<!RuntimeAgent.CallFrame>} callFrames
* @param {!WebInspector.UIList.Item} asyncCallFrameItem
* @return {!Array<!WebInspector.CallStackSidebarPane.CallFrame>}
*/
Expand Down
4 changes: 1 addition & 3 deletions front_end/timeline/TimelineTreeView.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ WebInspector.TimelineTreeView.prototype = {
var frame = WebInspector.TimelineProfileTree.eventStackFrame(event);
if (!frame)
return null;
return event.name === WebInspector.TimelineModel.RecordType.JSFrame
? this._linkifier.maybeLinkifyConsoleCallFrame(target, frame)
: this._linkifier.maybeLinkifyConsoleCallFrameForTracing(target, frame);
return this._linkifier.maybeLinkifyConsoleCallFrame(target, frame);
},

/**
Expand Down
48 changes: 21 additions & 27 deletions front_end/timeline/TimelineUIUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ WebInspector.TimelineUIUtils.buildDetailsTextForTraceEvent = function(event, tar
break;
case recordType.FunctionCall:
// Omit internally generated script names.
if (eventData && eventData["scriptName"])
detailsText = linkifyLocationAsText(eventData["scriptId"], eventData["scriptLine"], 0);
if (eventData)
detailsText = linkifyLocationAsText(eventData["scriptId"], eventData["lineNumber"], 0);
break;
case recordType.JSFrame:
detailsText = WebInspector.beautifyFunctionName(eventData["functionName"]);
Expand All @@ -429,7 +429,7 @@ WebInspector.TimelineUIUtils.buildDetailsTextForTraceEvent = function(event, tar
case recordType.EvaluateScript:
var url = eventData["url"];
if (url)
detailsText = WebInspector.displayNameForURL(url) + ":" + eventData["lineNumber"];
detailsText = WebInspector.displayNameForURL(url) + ":" + (eventData["lineNumber"] + 1);
break;
case recordType.ParseScriptOnBackground:
case recordType.XHRReadyStateChange:
Expand Down Expand Up @@ -491,14 +491,15 @@ WebInspector.TimelineUIUtils.buildDetailsTextForTraceEvent = function(event, tar
/**
* @param {string} scriptId
* @param {number} lineNumber
* @param {number=} columnNumber
* @param {number} columnNumber
* @return {?string}
*/
function linkifyLocationAsText(scriptId, lineNumber, columnNumber)
{
// FIXME(62725): stack trace line/column numbers are one-based.
var debuggerModel = WebInspector.DebuggerModel.fromTarget(target);
var rawLocation = target && !target.isDetached() && scriptId && debuggerModel ? debuggerModel.createRawLocationByScriptId(scriptId, lineNumber - 1, (columnNumber || 1) - 1) : null;
if (!target || target.isDetached() || !scriptId || !debuggerModel)
return null;
var rawLocation = debuggerModel.createRawLocationByScriptId(scriptId, lineNumber, columnNumber);
if (!rawLocation)
return null;
var uiLocation = WebInspector.debuggerWorkspaceBinding.rawLocationToUILocation(rawLocation);
Expand All @@ -511,8 +512,10 @@ WebInspector.TimelineUIUtils.buildDetailsTextForTraceEvent = function(event, tar
function linkifyTopCallFrameAsText()
{
var frame = WebInspector.TimelineUIUtils.topStackFrame(event);
var text = frame ? linkifyLocationAsText(frame.scriptId, frame.lineNumber, frame.columnNumber) : null;
if (frame && !text) {
if (!frame)
return null;
var text = linkifyLocationAsText(frame.scriptId, frame.lineNumber, frame.columnNumber);
if (!text) {
text = frame.url;
if (typeof frame.lineNumber === "number")
text += ":" + (frame.lineNumber + 1);
Expand Down Expand Up @@ -565,8 +568,6 @@ WebInspector.TimelineUIUtils.buildDetailsNodeForTraceEvent = function(event, tar
details = WebInspector.linkifyResourceAsNode(event.url);
break;
case recordType.FunctionCall:
details = linkifyLocation(eventData["scriptId"], eventData["scriptName"], eventData["scriptLine"] - 1, 0);
break;
case recordType.JSFrame:
details = createElement("span");
details.createTextChild(WebInspector.beautifyFunctionName(eventData["functionName"]));
Expand All @@ -580,7 +581,7 @@ WebInspector.TimelineUIUtils.buildDetailsNodeForTraceEvent = function(event, tar
case recordType.EvaluateScript:
var url = eventData["url"];
if (url)
details = linkifyLocation("", url, eventData["lineNumber"] - 1, 0);
details = linkifyLocation("", url, eventData["lineNumber"], 0);
break;
case recordType.ParseScriptOnBackground:
var url = eventData["url"];
Expand Down Expand Up @@ -617,7 +618,7 @@ WebInspector.TimelineUIUtils.buildDetailsNodeForTraceEvent = function(event, tar
function linkifyTopCallFrame()
{
var frame = WebInspector.TimelineUIUtils.topStackFrame(event);
return frame ? linkifier.maybeLinkifyConsoleCallFrameForTracing(target, frame, "timeline-details") : null;
return frame ? linkifier.maybeLinkifyConsoleCallFrame(target, frame, "timeline-details") : null;
}
}

Expand Down Expand Up @@ -720,6 +721,7 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
contentHelper.appendTextRow(WebInspector.UIString("Collected"), Number.bytesToString(delta));
break;
case recordTypes.JSFrame:
case recordTypes.FunctionCall:
var detailsNode = WebInspector.TimelineUIUtils.buildDetailsNodeForTraceEvent(event, model.targetByEvent(event), linkifier);
if (detailsNode)
contentHelper.appendElementRow(WebInspector.UIString("Function"), detailsNode);
Expand All @@ -736,12 +738,6 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
case recordTypes.FireAnimationFrame:
contentHelper.appendTextRow(WebInspector.UIString("Callback ID"), eventData["id"]);
break;
case recordTypes.FunctionCall:
if (typeof eventData["functionName"] === "string")
contentHelper.appendTextRow(WebInspector.UIString("Function"), WebInspector.beautifyFunctionName(eventData["functionName"]));
if (eventData["scriptName"])
contentHelper.appendLocationRow(WebInspector.UIString("Location"), eventData["scriptName"], eventData["scriptLine"]);
break;
case recordTypes.ResourceSendRequest:
case recordTypes.ResourceReceiveResponse:
case recordTypes.ResourceReceivedData:
Expand Down Expand Up @@ -829,8 +825,8 @@ WebInspector.TimelineUIUtils._buildTraceEventDetailsSynchronously = function(eve
case recordTypes.ParseHTML:
var beginData = event.args["beginData"];
var url = beginData["url"];
var startLine = beginData["startLine"] + 1;
var endLine = event.args["endData"] ? event.args["endData"]["endLine"] + 1 : 0;
var startLine = beginData["startLine"] - 1;
var endLine = event.args["endData"] ? event.args["endData"]["endLine"] - 1 : undefined;
if (url)
contentHelper.appendLocationRange(WebInspector.UIString("Range"), url, startLine, endLine);
break;
Expand Down Expand Up @@ -994,7 +990,7 @@ WebInspector.TimelineUIUtils.buildNetworkRequestDetails = function(request, mode
var sendRequest = request.children[0];
var topFrame = WebInspector.TimelineUIUtils.topStackFrame(sendRequest);
if (topFrame) {
var link = linkifier.maybeLinkifyConsoleCallFrameForTracing(target, topFrame);
var link = linkifier.maybeLinkifyConsoleCallFrame(target, topFrame);
if (link)
contentHelper.appendElementRow(title, link);
} else if (sendRequest.initiator) {
Expand Down Expand Up @@ -1245,7 +1241,7 @@ WebInspector.TimelineUIUtils.InvalidationsGroupElement.prototype = {
title.createTextChild(WebInspector.UIString(". "));
var stack = title.createChild("span", "monospace");
stack.createChild("span").textContent = WebInspector.beautifyFunctionName(topFrame.functionName);
var link = this._contentHelper.linkifier().maybeLinkifyConsoleCallFrameForTracing(target, topFrame);
var link = this._contentHelper.linkifier().maybeLinkifyConsoleCallFrame(target, topFrame);
if (link) {
stack.createChild("span").textContent = " @ ";
stack.createChild("span").appendChild(link);
Expand Down Expand Up @@ -2057,9 +2053,7 @@ WebInspector.TimelineDetailsContentHelper.prototype = {
{
if (!this._linkifier || !this._target)
return;
if (startColumn)
--startColumn;
var link = this._linkifier.maybeLinkifyScriptLocation(this._target, null, url, startLine - 1, startColumn);
var link = this._linkifier.maybeLinkifyScriptLocation(this._target, null, url, startLine, startColumn);
if (!link)
return;
this.appendElementRow(title, link);
Expand All @@ -2076,11 +2070,11 @@ WebInspector.TimelineDetailsContentHelper.prototype = {
if (!this._linkifier || !this._target)
return;
var locationContent = createElement("span");
var link = this._linkifier.maybeLinkifyScriptLocation(this._target, null, url, startLine - 1);
var link = this._linkifier.maybeLinkifyScriptLocation(this._target, null, url, startLine);
if (!link)
return;
locationContent.appendChild(link);
locationContent.createTextChild(String.sprintf(" [%s\u2026%s]", startLine, endLine || ""));
locationContent.createTextChild(String.sprintf(" [%s\u2026%s]", startLine + 1, endLine + 1 || ""));
this.appendElementRow(title, locationContent);
},

Expand Down
45 changes: 32 additions & 13 deletions front_end/timeline_model/TimelineModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -831,8 +831,16 @@ WebInspector.TimelineModel.prototype = {
this._currentScriptEvent = null;

var eventData = event.args["data"] || event.args["beginData"] || {};
if (eventData && eventData["stackTrace"])
if (eventData["stackTrace"])
event.stackTrace = eventData["stackTrace"];
if (event.stackTrace && event.name !== recordTypes.JSSample) {
// TraceEvents come with 1-based line & column numbers. The frontend code
// requires 0-based ones. Adjust the values.
for (var i = 0; i < event.stackTrace.length; ++i) {
--event.stackTrace[i].lineNumber;
--event.stackTrace[i].columnNumber;
}
}

if (eventStack.length && eventStack.peekLast().name === recordTypes.EventDispatch)
eventStack.peekLast().hasChildren = true;
Expand All @@ -842,12 +850,12 @@ WebInspector.TimelineModel.prototype = {
switch (event.name) {
case recordTypes.ResourceSendRequest:
case recordTypes.WebSocketCreate:
event.url = event.args["data"]["url"];
event.url = eventData["url"];
event.initiator = eventStack.peekLast() || null;
break;

case recordTypes.ScheduleStyleRecalculation:
this._lastScheduleStyleRecalculation[event.args["data"]["frame"]] = event;
this._lastScheduleStyleRecalculation[eventData["frame"]] = event;
break;

case recordTypes.UpdateLayoutTree:
Expand All @@ -874,7 +882,7 @@ WebInspector.TimelineModel.prototype = {
// Consider style recalculation as a reason for layout invalidation,
// but only if we had no earlier layout invalidation records.
var layoutInitator = event;
var frameId = event.args["data"]["frame"];
var frameId = eventData["frame"];
if (!this._layoutInvalidate[frameId] && this._lastRecalculateStylesEvent && this._lastRecalculateStylesEvent.endTime > event.startTime)
layoutInitator = this._lastRecalculateStylesEvent.initiator;
this._layoutInvalidate[frameId] = layoutInitator;
Expand All @@ -894,8 +902,19 @@ WebInspector.TimelineModel.prototype = {
event.warning = WebInspector.TimelineModel.WarningType.ForcedLayout;
break;

case recordTypes.EvaluateScript:
case recordTypes.FunctionCall:
// Compatibility with old format.
if (typeof eventData["scriptName"] === "string")
eventData["url"] = eventData["scriptName"];
if (typeof eventData["scriptLine"] === "number")
eventData["lineNumber"] = eventData["scriptLine"];
// Fallthrough.
case recordTypes.EvaluateScript:
case recordTypes.CompileScript:
if (typeof eventData["lineNumber"] === "number")
--eventData["lineNumber"];
if (typeof eventData["columnNumber"] === "number")
--eventData["columnNumber"];
if (!this._currentScriptEvent)
this._currentScriptEvent = event;
break;
Expand All @@ -906,12 +925,12 @@ WebInspector.TimelineModel.prototype = {

case recordTypes.Paint:
this._invalidationTracker.didPaint(event);
event.highlightQuad = event.args["data"]["clip"];
event.backendNodeId = event.args["data"]["nodeId"];
event.highlightQuad = eventData["clip"];
event.backendNodeId = eventData["nodeId"];
// Only keep layer paint events, skip paints for subframes that get painted to the same layer as parent.
if (!event.args["data"]["layerId"])
if (!eventData["layerId"])
break;
var layerId = event.args["data"]["layerId"];
var layerId = eventData["layerId"];
this._lastPaintForLayer[layerId] = event;
break;

Expand All @@ -926,12 +945,12 @@ WebInspector.TimelineModel.prototype = {
break;

case recordTypes.ScrollLayer:
event.backendNodeId = event.args["data"]["nodeId"];
event.backendNodeId = eventData["nodeId"];
break;

case recordTypes.PaintImage:
event.backendNodeId = event.args["data"]["nodeId"];
event.url = event.args["data"]["url"];
event.backendNodeId = eventData["nodeId"];
event.url = eventData["url"];
break;

case recordTypes.DecodeImage:
Expand Down Expand Up @@ -1447,7 +1466,7 @@ WebInspector.InvalidationTrackingEvent = function(event)
this.cause.reason = "Layout forced";
}

/** @typedef {{reason: string, stackTrace: ?Array.<!RuntimeAgent.CallFrame>}} */
/** @typedef {{reason: string, stackTrace: ?Array<!RuntimeAgent.CallFrame>}} */
WebInspector.InvalidationCause;

/**
Expand Down

0 comments on commit 4c68e86

Please sign in to comment.