Modify devtools to show passive event listeners.

Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756
Committed: https://crrev.com/ed3f93d1f6149e0861bd9df3dedabc84f2d28d33
Cr-Commit-Position: refs/heads/master@{#386281}

[dtapuska, 2016-03-30 17:36:32]

Description was changed from

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=489802
==========

to

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=489802
==========

[dtapuska, 2016-03-30 17:36:32]

dtapuska@chromium.org changed reviewers:
+ dgozman@chromium.org

[dtapuska, 2016-03-30 17:38:28]

On 2016/03/30 17:36:39, dtapuska wrote:

I'm no JS developer so please tell me if I did something wrong.

[pfeldman, 2016-03-30 17:43:04]

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

[pfeldman, 2016-03-30 17:43:05]

Overall looks good. Some nits below. This also needs a test.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js
(right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:69:
function addDispatchFilterOption(name, value) {
This function needs JSDoc annotations. { should go the next line.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:77:
dispatchFilter.setMaxWidth(200);
For UI changes, please attach the screenshot to the bug / codereview.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:120:
_onDispatchFilterTypeChanged: function(event)
JSDoc missing

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:608: function
eventListenerOptions() {
{ goes next line.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:609: if
(passive) {
drop {}

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:612: return
useCapture;
JSDoc return type. Can we always use the {"capture":} mode to make branches
return same type?

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1399:
function eventListenerOptions(capture, passive) {
ditto

[dtapuska, 2016-03-30 17:47:16]

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:612: return
useCapture;
On 2016/03/30 17:43:04, pfeldman wrote:
> JSDoc return type. Can we always use the {"capture":} mode to make branches
> return same type?

We might temporarily disable passive event listeners in stable due to some other
vendor feelings on the API that got into the DOM spec. So I didn't want to use
the new remove format without some kind of check.

[pfeldman, 2016-03-30 17:52:33]

> We might temporarily disable passive event listeners in stable due to some
other
> vendor feelings on the API that got into the DOM spec. So I didn't want to use
> the new remove format without some kind of check.

I see. What is the status of this then, should we consider DevTools support when
it settles?

[dtapuska, 2016-03-30 20:59:48]

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js
(right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:69:
function addDispatchFilterOption(name, value) {
On 2016/03/30 17:43:04, pfeldman wrote:
> This function needs JSDoc annotations. { should go the next line.

Done.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:77:
dispatchFilter.setMaxWidth(200);
On 2016/03/30 17:43:04, pfeldman wrote:
> For UI changes, please attach the screenshot to the bug / codereview.

Done.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/elements/EventListenersWidget.js:120:
_onDispatchFilterTypeChanged: function(event)
On 2016/03/30 17:43:04, pfeldman wrote:
> JSDoc missing

Done.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:608: function
eventListenerOptions() {
On 2016/03/30 17:43:05, pfeldman wrote:
> { goes next line.

Done.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:609: if
(passive) {
On 2016/03/30 17:43:04, pfeldman wrote:
> drop {}

Done.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:612: return
useCapture;
On 2016/03/30 17:43:04, pfeldman wrote:
> JSDoc return type. Can we always use the {"capture":} mode to make branches
> return same type?

If I JSDoc this it fails passing it into the removeEventListener because the
closure compiler hasn't been updated yet for the new DOM API.

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js
(right):

https://codereview.chromium.org/1845493004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/v8_inspector/InjectedScriptSource.js:1399:
function eventListenerOptions(capture, passive) {
On 2016/03/30 17:43:05, pfeldman wrote:
> ditto

Likewise; if I JSDoc this it will fail.

[dtapuska, 2016-03-30 21:01:19]

On 2016/03/30 17:52:33, pfeldman wrote:
> > We might temporarily disable passive event listeners in stable due to some
> other
> > vendor feelings on the API that got into the DOM spec. So I didn't want to
use
> > the new remove format without some kind of check.
> 
> I see. What is the status of this then, should we consider DevTools support
when
> it settles?

There hasn't been a decision either way yet. The two options are we go with
a 4 arg approach or stick with the 3 argument approach.

I don't know if we should block the devtools work on that. If there isn't
a decision and it is right before branch point we might pull it; or we
could also branch and change the API in a merge.

[pfeldman, 2016-03-30 21:11:15]

> I don't know if we should block the devtools work on that. If there isn't
> a decision and it is right before branch point we might pull it; or we
> could also branch and change the API in a merge.

Lets not be in a hurry with this then? We can wait for the final decision and
land support afterwards.

[dtapuska, 2016-03-30 21:14:41]

On 2016/03/30 21:11:15, pfeldman wrote:
> > I don't know if we should block the devtools work on that. If there isn't
> > a decision and it is right before branch point we might pull it; or we
> > could also branch and change the API in a merge.
> 
> Lets not be in a hurry with this then? We can wait for the final decision and
> land support afterwards.

I've changed the title to have [On Hold]: will probably have this change sit
around for a week or two until it gets finalized.

[pfeldman, 2016-03-30 21:20:06]

the code looks good, pending the spec change.

https://codereview.chromium.org/1845493004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1845493004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:608: //
TODO(dtapuska): Remove this one closure compiler is updated
We'd have to patch the externs once the spec finalizes, so this would compile.

[dtapuska, 2016-04-08 13:42:13]

Description was changed from

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=489802
==========

to

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756
==========

[dtapuska, 2016-04-08 13:42:37]

PTAL today as I'd like to get this into 51.

https://codereview.chromium.org/1845493004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/1845493004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:608: //
TODO(dtapuska): Remove this one closure compiler is updated
On 2016/03/30 21:20:06, pfeldman wrote:
> We'd have to patch the externs once the spec finalizes, so this would compile.

The spec is finalized; we are shipping in 51.

I'm going to try to do a pull request for closure to update the externs there;
for now this compiles because I didn't use jsdoc style so it doesn't know the
return type.

[dgozman, 2016-04-09 00:23:49]

lgtm

[dtapuska, 2016-04-09 02:28:22]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2016-04-09 02:28:38]

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

[commit-bot: I haz the power, 2016-04-09 03:28:19]

Description was changed from

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756
==========

to

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756
==========

[commit-bot: I haz the power, 2016-04-09 03:28:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-09 03:29:43]

Description was changed from

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756
==========

to

==========
Modify devtools to show passive event listeners.

Add the ability to filter out what event listeners
based on dispatch type; [All, Passive, Blocking].

Add the listing of passive in the event listener list
similar to how useCapture is shown.

BUG=601756

Committed: https://crrev.com/ed3f93d1f6149e0861bd9df3dedabc84f2d28d33
Cr-Commit-Position: refs/heads/master@{#386281}
==========

[commit-bot: I haz the power, 2016-04-09 03:29:44]

Patchset 4 (id:??) landed as
https://crrev.com/ed3f93d1f6149e0861bd9df3dedabc84f2d28d33
Cr-Commit-Position: refs/heads/master@{#386281}