Support warning decorations in Timeline flame chart based on trace events

Support warning decorations in Timeline flame chart based on trace events

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

[caseq, 2014-05-07 15:44:32]

https://codereview.chromium.org/270553005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:196: if (event.name ===
WebInspector.TimelineModel.RecordType.EvaluateScript || event.name ===
WebInspector.TimelineModel.RecordType.FunctionCall)
Looks like this belongs to a higher level than tracing model. Can we do this
elsewhere?

[yurys, 2014-05-08 07:46:45]

https://codereview.chromium.org/270553005/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:196: if (event.name ===
WebInspector.TimelineModel.RecordType.EvaluateScript || event.name ===
WebInspector.TimelineModel.RecordType.FunctionCall)
On 2014/05/07 15:44:33, caseq wrote:
> Looks like this belongs to a higher level than tracing model. Can we do this
> elsewhere?

Done. Extracted this code into TracingModel.EventBindings which will contain
derived data for the tracing model similar to InterRecordBingings in current
timeline implementation.

[caseq, 2014-05-08 09:03:53]

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:204: this._eventToWarning =
new Map();
remove?

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:327: for (var i = 0, l =
events.length; i < l; i++) {
Lowercase l looks bad as identifier, let's use just "length"?

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:329: if (currentScriptEvent
&& event.startTime > currentScriptEvent.endTime)
Are script events never nested?

[yurys, 2014-05-08 09:47:30]

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:204: this._eventToWarning =
new Map();
On 2014/05/08 09:03:53, caseq wrote:
> remove?

Done.

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:327: for (var i = 0, l =
events.length; i < l; i++) {
On 2014/05/08 09:03:53, caseq wrote:
> Lowercase l looks bad as identifier, let's use just "length"?

Done.

https://codereview.chromium.org/270553005/diff/50001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:329: if (currentScriptEvent
&& event.startTime > currentScriptEvent.endTime)
On 2014/05/08 09:03:53, caseq wrote:
> Are script events never nested?

They are but we just need to know if there is at least one on the stack so it is
enough to keep only the the outermost one.

[caseq, 2014-05-08 09:58:18]

lgtm

https://codereview.chromium.org/270553005/diff/70001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/70001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:307:
WebInspector.TracingModel.EventBindings = function(model) {
{ => next line.

[yurys, 2014-05-08 10:49:27]

https://codereview.chromium.org/270553005/diff/70001/Source/devtools/front_en...
File Source/devtools/front_end/timeline/TracingModel.js (right):

https://codereview.chromium.org/270553005/diff/70001/Source/devtools/front_en...
Source/devtools/front_end/timeline/TracingModel.js:307:
WebInspector.TracingModel.EventBindings = function(model) {
On 2014/05/08 09:58:18, caseq wrote:
> { => next line.

Done.

[yurys, 2014-05-08 14:35:58]

The CQ bit was checked by yurys@chromium.org

[commit-bot: I haz the power, 2014-05-08 14:36:42]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/yurys@chromium.org/270553005/90001

[commit-bot: I haz the power, 2014-05-08 15:42:36]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/6955)
  mac_blink_compile_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_compile_dbg/bu...)
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/6197)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/6665)

[commit-bot: I haz the power, 2014-05-08 16:49:06]

Change committed as 173654