[DevTools] Extracted EventListenersTreeOutline

[DevTools] Extracted EventListenersTreeOutline

EventListenersTreeOutline is extracted from EventListenersSidebarPane.js. It will be reused in Sources Panel for global objects event listeners.

BUG=469159, 483355, 478677, 491519
R=pfeldman@chromium.org

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

[kozy, 2015-05-21 13:32:43]

ptal

[pfeldman, 2015-05-21 21:22:40]

Could you refactor it in two steps so that I could review the changes using
side-by-side diff?

[kozy, 2015-05-22 10:54:11]

On 2015/05/21 21:22:40, pfeldman_slow wrote:
> Could you refactor it in two steps so that I could review the changes using
> side-by-side diff?

I've split it into two CLs - this one and
https://codereview.chromium.org/1154703005/ .
Please take a look.

[pfeldman, 2015-05-22 16:11:23]

Thanks!

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/components/EventListenersTreeOutline.js (right):

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:10:
WebInspector.EventListenersTreeOutline = function(element, objectGroup)
The name is unfortunate since this is not a tree outline.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:14:
this._treeOutline = new TreeOutline(true);
Please use TreeOutlineInShadow

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:16:
this._treeOutline.element.classList.add("event-listener-tree",
"outline-disclosure", "monospace");
You would not need the outline-disclosure

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:18:
this._treeItemMap = new Map();
Annotate the type please.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:20:
this._lastRuntimeAgents = new Set();
Ditto. Also, is this a leak?

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:42:
_getTreeElementForType: function(type)
no get prefixes in Blink, but in this case getOrCreate..

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:49: if
(this._isEmpty) {
You could get child count from root node instead, but you don't need it.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:52:
this._element.appendChild(this._treeOutline.element);
No need to do this lazily, just remove the placeholder unconditionally. It is
noop if it is removed.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:93:
node.resolveToObject(this._objectGroup, objectCallback.bind(this));
Reverse the order of functions and calls so that they went top -> bottom

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:117:
WebInspector.EventListenersTreeElement = function(type, linkifier)
You don't seem to override anything, don't inherit.

[kozy, 2015-05-22 17:43:44]

All done. Please take a look!

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
File Source/devtools/front_end/components/EventListenersTreeOutline.js (right):

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:10:
WebInspector.EventListenersTreeOutline = function(element, objectGroup)
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> The name is unfortunate since this is not a tree outline.

Renamed to EventListenersView

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:14:
this._treeOutline = new TreeOutline(true);
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> Please use TreeOutlineInShadow

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:16:
this._treeOutline.element.classList.add("event-listener-tree",
"outline-disclosure", "monospace");
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> You would not need the outline-disclosure

Removed.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:18:
this._treeItemMap = new Map();
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> Annotate the type please.

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:20:
this._lastRuntimeAgents = new Set();
On 2015/05/22 16:11:22, pfeldman_slow wrote:
> Ditto. Also, is this a leak?

releaseObjectGroup logic is shifted to EventListenerssSidebarPane.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:42:
_getTreeElementForType: function(type)
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> no get prefixes in Blink, but in this case getOrCreate..

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:49: if
(this._isEmpty) {
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> You could get child count from root node instead, but you don't need it.

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:52:
this._element.appendChild(this._treeOutline.element);
On 2015/05/22 16:11:22, pfeldman_slow wrote:
> No need to do this lazily, just remove the placeholder unconditionally. It is
> noop if it is removed.

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:93:
node.resolveToObject(this._objectGroup, objectCallback.bind(this));
On 2015/05/22 16:11:23, pfeldman_slow wrote:
> Reverse the order of functions and calls so that they went top -> bottom

Done.

https://codereview.chromium.org/1144953005/diff/20001/Source/devtools/front_e...
Source/devtools/front_end/components/EventListenersTreeOutline.js:117:
WebInspector.EventListenersTreeElement = function(type, linkifier)
On 2015/05/22 16:11:22, pfeldman_slow wrote:
> You don't seem to override anything, don't inherit.

I use this object in line 55: this._treeOutline.appendChild(treeItem);
appendChild function signature contains TreeElement.

[pfeldman, 2015-05-25 12:08:12]

lgtm

[kozy, 2015-05-25 12:08:56]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2015-05-25 12:09:08]

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

[commit-bot: I haz the power, 2015-05-25 13:22:47]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195862