[invalidations] Convert invalidations to use TreeOutline

[invalidations] Convert invalidations to use TreeOutline

This patch fixes the arrows next to invalidation groups which were
broken by reimplementing the invalidations group UI on top of
TreeOutlineInShadow.

Preview the new look at: http://pr.gg/invalidationarrows2.png

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

[pdr., 2015-03-30 01:14:35]

pdr@chromium.org changed reviewers:
+ aandrey@chromium.org, kouhei@chromium.org

[caseq, 2015-03-30 17:38:27]

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

[caseq, 2015-03-30 17:38:28]

https://codereview.chromium.org/1048663002/diff/1/Source/devtools/front_end/t...
File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):

https://codereview.chromium.org/1048663002/diff/1/Source/devtools/front_end/t...
Source/devtools/front_end/timeline/TimelineUIUtils.js:804:
header.createChild("div", "timeline-expandable-arrow");
Did you consider using TreeOutline here?

[pdr., 2015-03-31 04:42:36]

On 2015/03/30 at 17:38:28, caseq wrote:
>
https://codereview.chromium.org/1048663002/diff/1/Source/devtools/front_end/t...
> File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):
> 
>
https://codereview.chromium.org/1048663002/diff/1/Source/devtools/front_end/t...
> Source/devtools/front_end/timeline/TimelineUIUtils.js:804:
header.createChild("div", "timeline-expandable-arrow");
> Did you consider using TreeOutline here?

Thanks for the tip. I think TreeOutline is the way to go but I just didn't get
the time to finish this today.

[pfeldman, 2015-03-31 05:48:30]

pfeldman@chromium.org changed reviewers:
+ pfeldman@chromium.org

[pfeldman, 2015-03-31 05:48:30]

TreeOutlineInShadow is better

[pdr., 2015-04-04 05:59:52]

TreeOutline is cool. PTAL

[pfeldman, 2015-04-07 15:02:09]

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:819:
treeElement.isEventWithinDisclosureTriangle = function() {return true;}
you should not need this.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:832: treeElement.onexpand
= onExpand.bind(null, treeElement, invalidations);
We try not to do this. You should extend TreeElement instead.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:840: if
(treeElement.childCount() >= 1)
You should override onpopulate which is going to be called exactly once.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:893:
invalidationNode.onclick = function(event) {
add named click listener please.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/timelinePanel.css (right):

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/timelinePanel.css:866: .invalidations-tree {
You should instead create a small dedicated css file that you would inject via
treeoutlineinshadow's registerRequiredCSS

[pdr., 2015-04-09 06:14:49]

Thanks for the review. PTAL

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:819:
treeElement.isEventWithinDisclosureTriangle = function() {return true;}
On 2015/04/07 at 15:02:09, pfeldman wrote:
> you should not need this.

Fixed. "toggleOnClick = true" was what I was looking for.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:832: treeElement.onexpand
= onExpand.bind(null, treeElement, invalidations);
On 2015/04/07 at 15:02:09, pfeldman wrote:
> We try not to do this. You should extend TreeElement instead.

Done. This is much cleaner but unfortunately makes this patch difficult to
review.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:840: if
(treeElement.childCount() >= 1)
On 2015/04/07 at 15:02:09, pfeldman wrote:
> You should override onpopulate which is going to be called exactly once.

Done.

https://codereview.chromium.org/1048663002/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:893:
invalidationNode.onclick = function(event) {
On 2015/04/07 at 15:02:09, pfeldman wrote:
> add named click listener please.

Done, and switched to "consume".

[pfeldman, 2015-04-09 10:15:52]

lgtm % nits

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:760:
invalidationsTreeOutline.element.classList.add("timeline-details-view-row-value");
classList.add(a, b, c) would work.

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:892: if
(invalidation.changedId) {
All one-liners? Drop the {}!

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:946: function
consumeEvent(event)
You don't seem to need to preventDefault, so you could just use "consumeEvent"
symbol from DOMExtensions.js

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/invalidationsTree.css (right):

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/invalidationsTree.css:12: .children
li::before {
So you are making it always expanded? I guess we should enrich treeoutline with
this capability. Mind inserting a FIXME here?

[pdr., 2015-04-09 18:20:33]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-04-09 18:20:34]

Thank you for the reviews. So codes. A++. Much learn. Super

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/TimelineUIUtils.js (right):

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:760:
invalidationsTreeOutline.element.classList.add("timeline-details-view-row-value");
On 2015/04/09 at 10:15:52, pfeldman wrote:
> classList.add(a, b, c) would work.

Done.

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:892: if
(invalidation.changedId) {
On 2015/04/09 at 10:15:52, pfeldman wrote:
> All one-liners? Drop the {}!

Dropped. -20bytes!

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/TimelineUIUtils.js:946: function
consumeEvent(event)
On 2015/04/09 at 10:15:52, pfeldman wrote:
> You don't seem to need to preventDefault, so you could just use "consumeEvent"
symbol from DOMExtensions.js

Oh my.. Didn't realize this already existed.

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
File Source/devtools/front_end/timeline/invalidationsTree.css (right):

https://codereview.chromium.org/1048663002/diff/40001/Source/devtools/front_e...
Source/devtools/front_end/timeline/invalidationsTree.css:12: .children
li::before {
On 2015/04/09 at 10:15:52, pfeldman wrote:
> So you are making it always expanded? I guess we should enrich treeoutline
with this capability. Mind inserting a FIXME here?

Yep! Done. crbug.com/475618 is tracking this. Should be pretty easy to
implement.

[pdr., 2015-04-09 18:20:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1048663002/#ps60001
(title: "Update per reviewer comments")

[commit-bot: I haz the power, 2015-04-09 18:21:18]

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

[commit-bot: I haz the power, 2015-04-09 22:52:24]

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

[commit-bot: I haz the power, 2015-04-09 22:52:25]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/51189)

[pdr., 2015-04-10 00:56:35]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-04-10 00:56:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/1048663002/#ps80001
(title: "Fix failing test to to shadowRoot.getElementsByClassName going away")

[commit-bot: I haz the power, 2015-04-10 00:57:56]

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

[commit-bot: I haz the power, 2015-04-10 05:54:45]

Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=193514