Timeline Trace viewer prototype

Timeline Trace viewer prototype

This adds a timeline mode for recording a trace and a very basic trace
viewer based on FlameChart.

BUG=

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170548

[pfeldman, 2014-03-26 18:32:09]

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/Settings.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/Settings.js:345: this.traceViewer =
this._createExperiment("traceViewer", "Show Trace viewer in Timeline");
"trace viewer" would add to the confusion. lets call it tracing by the name of
the underlying instrumentation.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TimelinePanel.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelinePanel.js:240: if (!this._lazyTracingModel)
We need to figure out the instrumentation vs current view problem. We might end
forcing user to stick with the view after recording.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelinePanel.js:600:
WebInspector.tracingAgent.start("", "");
For better modularity, you should introduce recordingStarted / Stopped on the
TimelineView or set a listener from your module.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TracingAgent.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:263: get name()
no getters please.

[pfeldman, 2014-03-26 20:10:05]

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TimelineTraceView.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:78:
this._mainView.setWindowTimes(startTime, endTime);
This does not work when applied.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:261: if (record instanceof
WebInspector.TraceViewFlameChartDataProvider.FakeEventForDelimiter)
instanceof is way too expensive for the case of 2M records.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:375: {r: 138, g: 113, b: 152},
See WebInspector.Color.Nicknames for how we define colors.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TracingAgent.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:253: WebInspector.TracingModel.Event =
function(payload, level)
Can we actually afford keeping both - payload and event object? Unless it does
not store children, we should not need this.

[pfeldman, 2014-03-26 20:37:11]

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TimelineTraceView.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:19: this._tracingModel =
tracingModel;
unused?

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:192: this._zeroTime =
this._model.minimumRecordTime() || 0;
This is going to be relative to the CPU clock, while you want it to be wall
time?

[pfeldman, 2014-03-27 13:09:53]

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TracingAgent.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:307: console.assert(false, "Open/close
event mismatch: " + JSON.stringify(this._payload) + " vs. " +
JSON.stringify(payload));
It is fine to complete the event you did not start since you could have started
recording with some Begin events already on stack.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:443: this._stack.push(event);
You should not put the complete event into the stack.

[caseq, 2014-03-28 17:13:41]

ptal

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/Settings.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/Settings.js:345: this.traceViewer =
this._createExperiment("traceViewer", "Show Trace viewer in Timeline");
On 2014/03/26 18:32:09, pfeldman wrote:
> "trace viewer" would add to the confusion. lets call it tracing by the name of
> the underlying instrumentation.

renamed to timelineTracingMode

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TimelinePanel.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelinePanel.js:600:
WebInspector.tracingAgent.start("", "");
On 2014/03/26 18:32:09, pfeldman wrote:
> For better modularity, you should introduce recordingStarted / Stopped on the
> TimelineView or set a listener from your module.

Done, though I named it timelineStarted/Stopped sine we've already introduced
one for the overview.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TimelineTraceView.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:19: this._tracingModel =
tracingModel;
On 2014/03/26 20:37:12, pfeldman wrote:
> unused?

used now :-)

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:78:
this._mainView.setWindowTimes(startTime, endTime);
On 2014/03/26 20:10:06, pfeldman wrote:
> This does not work when applied.

This requires https://codereview.chromium.org/212893002/

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:192: this._zeroTime =
this._model.minimumRecordTime() || 0;
On 2014/03/26 20:37:12, pfeldman wrote:
> This is going to be relative to the CPU clock, while you want it to be wall
> time?

With https://codereview.chromium.org/212893002/, this should be
monotonicallyIncreasingTime()  expressed in milliseconds, so this works
correctly.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:261: if (record instanceof
WebInspector.TraceViewFlameChartDataProvider.FakeEventForDelimiter)
On 2014/03/26 20:10:06, pfeldman wrote:
> instanceof is way too expensive for the case of 2M records.

Fixed.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TimelineTraceView.js:375: {r: 138, g: 113, b: 152},
On 2014/03/26 20:10:06, pfeldman wrote:
> See WebInspector.Color.Nicknames for how we define colors.

Changed objects to arrays, though I think we do not require use of
WebInspector.Color here.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/TracingAgent.js (right):

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:253: WebInspector.TracingModel.Event =
function(payload, level)
On 2014/03/26 20:10:06, pfeldman wrote:
> Can we actually afford keeping both - payload and event object? Unless it does
> not store children, we should not need this.

Dropped payload, we just shallow-copy properties of interest now.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:263: get name()
On 2014/03/26 18:32:09, pfeldman wrote:
> no getters please.

Gone with the above change.

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:307: console.assert(false, "Open/close
event mismatch: " + JSON.stringify(this._payload) + " vs. " +
JSON.stringify(payload));
On 2014/03/27 13:09:53, pfeldman wrote:
> It is fine to complete the event you did not start since you could have
started
> recording with some Begin events already on stack.

This check shouldn't normally hit in the case you describe (unless we have a bug
elsewhere, which proved to be the case).

https://codereview.chromium.org/212683005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:443: this._stack.push(event);
On 2014/03/27 13:09:53, pfeldman wrote:
> You should not put the complete event into the stack.

I should -- we wouldn't compute level properly otherwise.

[pfeldman, 2014-03-31 09:28:42]

lgtm

https://codereview.chromium.org/212683005/diff/50001/Source/devtools/front_en...
File Source/devtools/front_end/TracingAgent.js (right):

https://codereview.chromium.org/212683005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:163: WebInspector.TracingModel =
function()
Lets have this in its own file.

https://codereview.chromium.org/212683005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/TracingAgent.js:288: };
remove ;

[caseq, 2014-04-01 10:27:24]

The CQ bit was checked by caseq@chromium.org

[commit-bot: I haz the power, 2014-04-01 10:27:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caseq@chromium.org/212683005/70001

[commit-bot: I haz the power, 2014-04-01 10:54:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-01 10:54:08]

Try jobs failed on following builders:
  tryserver.blink on win_blink_compile_dbg

[caseq, 2014-04-01 10:58:06]

The CQ bit was checked by caseq@chromium.org

[commit-bot: I haz the power, 2014-04-01 10:58:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/caseq@chromium.org/212683005/70001

[commit-bot: I haz the power, 2014-04-01 11:24:01]

Change committed as 170548