[DevTools] Added window listeners to EventListenersSidebar (issue 1154703005 by kozyatinskiy@chromium.org)

0 views
Skip to first unread message

kozyat...@chromium.org

unread,
May 22, 2015, 6:52:48 AM5/22/15
to pfel...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
Reviewers: pfeldman_slow,

Message:
ptal.

Description:
[DevTools] Added window listeners to EventListenersSidebar

If filter is setted to "All Nodes" then listeners from curent execution
context's window will be shown in Event Listeners Sidebar in Elements Panel.

BUG=469159
R=pfel...@chromium.org

Please review this at https://codereview.chromium.org/1154703005/

Base URL:
https://chromium.googlesource.com/chromium/blink.git@extract-event-listeners-tree-outline

Affected files (+129, -1 lines):
M LayoutTests/inspector/elements/event-listener-sidebar-expected.txt
M Source/devtools/front_end/components/EventListenersTreeOutline.js
M Source/devtools/front_end/elements/EventListenersSidebarPane.js


pfel...@chromium.org

unread,
May 25, 2015, 9:18:43 AM5/25/15
to kozyat...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js
File Source/devtools/front_end/components/EventListenersView.js (right):

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode75
Source/devtools/front_end/components/EventListenersView.js:75: * @return
{!Promise}
.<undefined>

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode96
Source/devtools/front_end/components/EventListenersView.js:96:
object.getEventListeners(listenersCallback.bind(this));
Lets convert this one into promise.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode284
Source/devtools/front_end/components/EventListenersView.js:284:
title.textContent = object.description;
It might be that
WebInspector.ObjectPropertiesSection.valueTextForFunctionDescription
would work better for you.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js
File Source/devtools/front_end/elements/EventListenersSidebarPane.js
(right):

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode96
Source/devtools/front_end/elements/EventListenersSidebarPane.js:96:
this._getWindowObjectInNodeContext(node).then(windowObjectCallback.bind(this));
_windowObjectInNodeContext

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode124
Source/devtools/front_end/elements/EventListenersSidebarPane.js:124:
_getWindowObjectInNodeContext: function(node)
ditto

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode132
Source/devtools/front_end/elements/EventListenersSidebarPane.js:132:
function getWindowObject(fulfill, reject)
ditto

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode145
Source/devtools/front_end/elements/EventListenersSidebarPane.js:145:
context.evaluate("window",
WebInspector.EventListenersSidebarPane._objectGroupName, false, true,
false, false, fulfill);
self

https://codereview.chromium.org/1154703005/

kozyat...@chromium.org

unread,
May 25, 2015, 10:55:24 AM5/25/15
to pfel...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
All done. Please take a look!


https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js
File Source/devtools/front_end/components/EventListenersView.js (right):

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode75
Source/devtools/front_end/components/EventListenersView.js:75: * @return
{!Promise}
On 2015/05/25 13:18:43, pfeldman wrote:
> .<undefined>

Done.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode96
Source/devtools/front_end/components/EventListenersView.js:96:
object.getEventListeners(listenersCallback.bind(this));
On 2015/05/25 13:18:43, pfeldman wrote:
> Lets convert this one into promise.

Done.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/components/EventListenersView.js#newcode284
Source/devtools/front_end/components/EventListenersView.js:284:
title.textContent = object.description;
On 2015/05/25 13:18:43, pfeldman wrote:
> It might be that
> WebInspector.ObjectPropertiesSection.valueTextForFunctionDescription
would work
> better for you.

I've added Revealer onclick listener to
WebInspector.ObjectPropertiesSection.createValueElement for nodes and
reused it here. Because of this I've deleted all dom node specific code
from EventListenersView.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js
File Source/devtools/front_end/elements/EventListenersSidebarPane.js
(right):

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode96
Source/devtools/front_end/elements/EventListenersSidebarPane.js:96:
this._getWindowObjectInNodeContext(node).then(windowObjectCallback.bind(this));
On 2015/05/25 13:18:43, pfeldman wrote:
> _windowObjectInNodeContext

Done.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode124
Source/devtools/front_end/elements/EventListenersSidebarPane.js:124:
_getWindowObjectInNodeContext: function(node)
On 2015/05/25 13:18:43, pfeldman wrote:
> ditto

Done.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode132
Source/devtools/front_end/elements/EventListenersSidebarPane.js:132:
function getWindowObject(fulfill, reject)
On 2015/05/25 13:18:43, pfeldman wrote:
> ditto

Done.

https://codereview.chromium.org/1154703005/diff/20001/Source/devtools/front_end/elements/EventListenersSidebarPane.js#newcode145
Source/devtools/front_end/elements/EventListenersSidebarPane.js:145:
context.evaluate("window",
WebInspector.EventListenersSidebarPane._objectGroupName, false, true,
false, false, fulfill);
On 2015/05/25 13:18:43, pfeldman wrote:
> self

Done.

https://codereview.chromium.org/1154703005/

pfel...@chromium.org

unread,
May 25, 2015, 12:25:17 PM5/25/15
to kozyat...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/EventListenersView.js
File Source/devtools/front_end/components/EventListenersView.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/EventListenersView.js#newcode63
Source/devtools/front_end/components/EventListenersView.js:63: return
object.getEventListeners().then(this._addObjectEventListeners.bind(this,
object));
eventListeners()

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/ObjectPropertiesSection.js
File Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/ObjectPropertiesSection.js#newcode1150
Source/devtools/front_end/components/ObjectPropertiesSection.js:1150:
valueElement.addEventListener("click",
WebInspector.Revealer.reveal.bind(WebInspector.Revealer, value,
undefined), false);
Make sure you consume the click.

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/DOMModel.js#newcode949
Source/devtools/front_end/sdk/DOMModel.js:949: return new
Promise(resolveToObject.bind(this));
return new
Promise(this._wrapWithPromise(this.resolveToObject.bind(this)));

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/RemoteObject.js
File Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/RemoteObject.js#newcode431
Source/devtools/front_end/sdk/RemoteObject.js:431:
this.target().domdebuggerAgent().getEventListeners(this._objectId,
mycallback.bind(this));
move to line 410

https://codereview.chromium.org/1154703005/

pfel...@chromium.org

unread,
May 25, 2015, 12:29:01 PM5/25/15
to kozyat...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
lgtm



https://codereview.chromium.org/1154703005/

kozyat...@chromium.org

unread,
May 25, 2015, 2:03:31 PM5/25/15
to pfel...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/EventListenersView.js
File Source/devtools/front_end/components/EventListenersView.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/EventListenersView.js#newcode63
Source/devtools/front_end/components/EventListenersView.js:63: return
object.getEventListeners().then(this._addObjectEventListeners.bind(this,
object));
On 2015/05/25 16:25:16, pfeldman wrote:
> eventListeners()

Done.

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/ObjectPropertiesSection.js
File Source/devtools/front_end/components/ObjectPropertiesSection.js
(right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/components/ObjectPropertiesSection.js#newcode1150
Source/devtools/front_end/components/ObjectPropertiesSection.js:1150:
valueElement.addEventListener("click",
WebInspector.Revealer.reveal.bind(WebInspector.Revealer, value,
undefined), false);
On 2015/05/25 16:25:16, pfeldman wrote:
> Make sure you consume the click.

Done.

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/DOMModel.js
File Source/devtools/front_end/sdk/DOMModel.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/DOMModel.js#newcode949
Source/devtools/front_end/sdk/DOMModel.js:949: return new
Promise(resolveToObject.bind(this));
On 2015/05/25 16:25:16, pfeldman wrote:
> return new
Promise(this._wrapWithPromise(this.resolveToObject.bind(this)));

Acknowledged.

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/RemoteObject.js
File Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1154703005/diff/80001/Source/devtools/front_end/sdk/RemoteObject.js#newcode431
Source/devtools/front_end/sdk/RemoteObject.js:431:
this.target().domdebuggerAgent().getEventListeners(this._objectId,
mycallback.bind(this));
On 2015/05/25 16:25:16, pfeldman wrote:
> move to line 410

Done.

https://codereview.chromium.org/1154703005/

'I haz the power - commit-bot' via codereview.chromium.org

unread,
May 25, 2015, 2:03:43 PM5/25/15
to kozyat...@chromium.org, pfel...@chromium.org, commi...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1154703005/100001


https://codereview.chromium.org/1154703005/

'I haz the power - commit-bot' via codereview.chromium.org

unread,
May 25, 2015, 2:12:43 PM5/25/15
to kozyat...@chromium.org, pfel...@chromium.org, commi...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1154703005/120001


https://codereview.chromium.org/1154703005/

'I haz the power - commit-bot' via codereview.chromium.org

unread,
May 25, 2015, 4:28:02 PM5/25/15
to kozyat...@chromium.org, pfel...@chromium.org, commi...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
Try jobs failed on following builders:
linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/63075)

https://codereview.chromium.org/1154703005/

'I haz the power - commit-bot' via codereview.chromium.org

unread,
May 25, 2015, 4:33:41 PM5/25/15
to kozyat...@chromium.org, pfel...@chromium.org, commi...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1154703005/140001


https://codereview.chromium.org/1154703005/

'I haz the power - commit-bot' via codereview.chromium.org

unread,
May 25, 2015, 5:45:45 PM5/25/15
to kozyat...@chromium.org, pfel...@chromium.org, commi...@chromium.org, blink-...@chromium.org, caseq...@chromium.org, yurys...@chromium.org, lushnik...@chromium.org, pfeldma...@chromium.org, apavlo...@chromium.org, devtools...@chromium.org, sergey...@chromium.org, kozyatins...@chromium.org
Committed patchset #7 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=195875

https://codereview.chromium.org/1154703005/
Reply all
Reply to author
Forward
0 new messages