DevTools: Add grouping by Activity to Timeline treeview.

DevTools: Add grouping by Activity to Timeline treeview.

Allows a user to compare time spent in Painting vs Image Decode, etc.

BUG=614935
Committed: https://crrev.com/2baa7280750510db8208cfc0c250a24bb1a88ea8
Cr-Commit-Position: refs/heads/master@{#399131}

[paulirish, 2016-05-26 06:41:16]

paulirish@chromium.org changed reviewers:
+ alph@chromium.org

[alph, 2016-05-26 07:23:21]

lgtm

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js
(right):

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js:340:
case WebInspector.TimelineAggregator.GroupBy.EventName: return node =>
node.event.name;
Can event be null? There's a check on the next line.

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js
(right):

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:509: }
missing ;

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:513:
WebInspector.UIString("Execute Script") :
WebInspector.TimelineUIUtils.eventTitle(node.event);
Strange name. Maybe "JavaScript"?

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:518: }
missing ;

https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:529: }
ditto

[paulirish, 2016-05-26 22:27:46]

On 2016/05/26 at 07:23:21, alph wrote:
> lgtm
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> File
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js
(right):
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
>
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js:340:
case WebInspector.TimelineAggregator.GroupBy.EventName: return node =>
node.event.name;
> Can event be null? There's a check on the next line.

Better to be safe. Added.
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> File third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js
(right):
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:509:
}
> missing ;

done

> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:513:
WebInspector.UIString("Execute Script") :
WebInspector.TimelineUIUtils.eventTitle(node.event);
> Strange name. Maybe "JavaScript"?

Sounds good. changed.
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:518:
}
> missing ;

done
> 
>
https://codereview.chromium.org/2010943002/diff/1/third_party/WebKit/Source/d...
> third_party/WebKit/Source/devtools/front_end/timeline/TimelineTreeView.js:529:
}
> ditto
done

[paulirish, 2016-05-26 22:28:05]

The CQ bit was checked by paulirish@chromium.org

[paulirish, 2016-05-26 22:28:06]

The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2010943002/#ps20001
(title: "rename to JavaScript. semicolons.")

[commit-bot: I haz the power, 2016-05-26 22:28:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2010943002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2010943002/20001

[paulirish, 2016-05-26 23:25:15]

The CQ bit was unchecked by paulirish@chromium.org

[paulirish, 2016-05-26 23:48:48]

paulirish@chromium.org changed reviewers:
+ caseq@chromium.org

[paulirish, 2016-05-26 23:48:49]

caseq caught that I had duplicate events.

Changed the approach to tweak our categoryMapper to carry over both title &
category.

@caseq: ptal

[caseq, 2016-05-26 23:53:42]

https://codereview.chromium.org/2010943002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js
(right):

https://codereview.chromium.org/2010943002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js:221:
* @param {{title: string, category: !WebInspector.TimelineCategory}}
eventStyleMapper
wrong annotation, this is supposed to be a function, right?

[paulirish, 2016-05-27 00:53:16]

On 2016/05/26 at 23:53:42, caseq wrote:
>
https://codereview.chromium.org/2010943002/diff/40001/third_party/WebKit/Sour...
> File
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js
(right):
> 
>
https://codereview.chromium.org/2010943002/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/devtools/front_end/timeline/TimelineProfileTree.js:221:
* @param {{title: string, category: !WebInspector.TimelineCategory}}
eventStyleMapper
> wrong annotation, this is supposed to be a function, right?

ayup! fixed.

[paulirish, 2016-05-27 06:04:22]

The CQ bit was checked by paulirish@chromium.org

[paulirish, 2016-05-27 06:04:22]

The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2010943002/#ps60001
(title: "fix annotation")

[commit-bot: I haz the power, 2016-05-27 06:04:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2010943002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2010943002/60001

[commit-bot: I haz the power, 2016-05-27 07:48:57]

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

[commit-bot: I haz the power, 2016-05-27 07:48:58]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[paulirish, 2016-05-28 01:15:26]

alph can you check the latest? 

Added an extra check in TimelineUIUtils so we dont undefined.startsWith() with
our test data.
Also nuked the experiment enabling in the test, as it isn't neccessary any
longer.

[paulirish, 2016-06-10 05:49:33]

The CQ bit was checked by paulirish@chromium.org

[paulirish, 2016-06-10 05:49:34]

The patchset sent to the CQ was uploaded after l-g-t-m from alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2010943002/#ps100001
(title: "layout test got resultz")

[commit-bot: I haz the power, 2016-06-10 05:49:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2010943002/100001

[commit-bot: I haz the power, 2016-06-10 07:24:35]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-06-10 07:24:41]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-10 07:27:31]

Description was changed from

==========
DevTools: Add grouping by Activity to Timeline treeview.

Allows a user to compare time spent in Painting vs Image Decode, etc.

BUG=614935
==========

to

==========
DevTools: Add grouping by Activity to Timeline treeview.

Allows a user to compare time spent in Painting vs Image Decode, etc.

BUG=614935

Committed: https://crrev.com/2baa7280750510db8208cfc0c250a24bb1a88ea8
Cr-Commit-Position: refs/heads/master@{#399131}
==========

[commit-bot: I haz the power, 2016-06-10 07:27:33]

Patchset 6 (id:??) landed as
https://crrev.com/2baa7280750510db8208cfc0c250a24bb1a88ea8
Cr-Commit-Position: refs/heads/master@{#399131}

[caseq, 2016-06-15 10:04:24]

https://codereview.chromium.org/2010943002/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/timeline_model/TimelineProfileTree.js
(right):

https://codereview.chromium.org/2010943002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline_model/TimelineProfileTree.js:221:
* @param {function(!WebInspector.TracingModel.Event):{title: string, category:
!WebInspector.TimelineCategory}} eventStyleMapper
You can't do this -- category is a UI level notion, so this doesn't compile.

[caseq, 2016-06-15 10:28:10]

https://codereview.chromium.org/2010943002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js
(right):

https://codereview.chromium.org/2010943002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/timeline/TimelineUIUtils.js:316:
return frame.scriptId !== "0" && frame.url && !frame.url.startsWith("native ");
Also, why this check? Does it work well with eval()?