* Open the inspector on any page and select an element that doesn't yet have any event listeners. * Open console and type: `$0.addEventListener("click", () => {});` * Flip back to inspector Expected: There is an event bubble next to the node Actual: There is no event bubble. If you close and reopen the inspector it will be there

Uses the addListenerChangeListener from Bug 524674 to send over mutations when events change for a known target

The debugger events pane will need this, too.

This should be easy once Bug 524674 lands

(In reply to Panos Astithas [:past] from comment #2) > The debugger events pane will need this, too. It will get updated already in between steps in the debugger, but it won't get updated immediately if the listener was added / removed in the REPL while paused. That is a smaller usecase, but it will be possible with this API so we may as well support it.

My main concern was the case where the events pane is visible, but the page is running normally (i.e. no debugging currently happens). Will the event list be updated when new event listeners are added?

Inspector actor now uses EventListenerService:addListenerChangeListener to propagate event listener changes as mutations with type 'events'. markupview will now trigger an update when receiving a mutation of type 'event', and the event bubble display will be updated Review commit: https://reviewboard.mozilla.org/r/29739/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/29739/

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29739/diff/1-2/

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins https://reviewboard.mozilla.org/r/29739/#review26585 LGTM ::: devtools/client/markupview/test/helper_events_test_runner.js:29 (Diff revision 2) > - * Selector pointing at the node to be inspected > + * node description object Can you expand on these docs with the possible keys for this object? ::: devtools/server/actors/inspector.js:1360 (Diff revision 2) > + * @param Array array This isn't an actual Array, looks like an nsISimpleEnumerator. Param name could be updated also to maybe just `changes` ::: devtools/server/actors/inspector.js:1445 (Diff revision 2) > + eventListenerService.removeListenerChangeListener(this._onEventListenerChange); Nit: 80 chars ::: devtools/server/tests/mochitest/test_inspector-mutations-events.html:70 (Diff revision 2) > + is(mutations[0].hasEventListeners, true, "mutation target should have event listeners"); May as well check that this value is correct on the front also ::: devtools/server/tests/mochitest/test_inspector-mutations-events.html:105 (Diff revision 2) > + is(mutations[0].hasEventListeners, true, "mutation target should have event listeners"); We could also check that the mutation target is eventFront1.actorID (and eventFront2 for the next one).

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29739/diff/2-3/

New try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=c963ab2efa6c&selectedJob=15139631 Some intermittent or unrelated failures.

this failed to apply: applying 78bc70b54a7f unable to find 'devtools/client/markupview/markup-view.js' for patching 3 out of 3 hunks FAILED -- saving rejects to file devtools/client/markupview/markup-view.js.rej unable to find 'devtools/client/markupview/test/browser_markupview_events.js' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/browser_markupview_events.js.rej unable to find 'devtools/client/markupview/test/doc_markup_events.html' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/doc_markup_events.html.rej unable to find 'devtools/client/markupview/test/helper_events_test_runner.js' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/helper_events_test_runner.js.rej

Failed to apply on top of https://hg.mozilla.org/integration/fx-team/rev/9c6a36040f06 (Bug 1237335). Just rebased my changes but 9c6a36040f06 was backed out in the meantime. New try push https://treeherder.mozilla.org/#/jobs?repo=try&revision=b546c5c658ad

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29739/diff/3-4/

smaug: we started using EventListenerService:addListenerChangeListener and there are intermittent failures which seem linked to it. 15:22:38 WARNING - PROCESS-CRASH | devtools/client/inspector/test/browser_inspector_search-01.js | application crashed [@ xpc::IsInContentXBLScope(JSObject*)] [... later in the stacktrace] 15:22:38 INFO - 29 XUL!mozilla::EventListenerService::NotifyPendingChanges() [EventListenerService.cpp:83469fb2d102 : 399 + 0xc] 15:22:38 INFO - rbp = 0x00007fff5fbfce10 rsp = 0x00007fff5fbfcdd0 15:22:38 INFO - rip = 0x0000000102a6dedf 15:22:38 INFO - Found by: previous frame's frame pointer (full stacktrace is at http://archive.mozilla.org/pub/firefox/tinderbox-builds/fx-team-macosx64/1452208549/fx-team_snowleopard_test-mochitest-devtools-chrome-7-bm108-tests1-macosx-build166.txt.gz relevant part of the stacktrace attached to the bug) Do you have any idea what might be wrong here ?

@KWierso : feel free to back this out if it occurs too frequently

@philor: this bug is most likely the root cause for Bug 1237797. Could you back it out while we are investigating the issue? Sorry about that.

Backed out in https://hg.mozilla.org/mozilla-central/rev/d4213241bb79 and merged around.

Looks like a null pointer crash in webidl bindings. WrapNativeParent calls WrapNativeParentHelper<T>::Wrap and that seems to return null, which is then passed to IsInContentXBLScope. As far as I see, this is a regression from bug 825392. http://hg.mozilla.org/mozilla-central/annotate/1424cdfc075d/dom/bindings/BindingUtils.h#l1601 assumes Wrap never fails. I think we should just change if (!useXBLScope) { few lines above to if (!useXBLScope || !parent) { Generated code does have null check for the return value of WrapNativeParent. Based on the stack trace, the patch should fix the crash. Whether or not there are other issue is different thing. Something is accessing a xul element rather late. Does something perhaps forget to remove event listener change listener? Julian, could you try the patch?

Thanks! We cannot reproduce the issue locally, so here is a try push with the patch applied : https://treeherder.mozilla.org/#/jobs?repo=try&revision=b66baa5324ae

The try push failed on a similar issue, this time in nsINode::WrapObject(JSContext*, JS::Handle<JSObject*>) which also relies on Wrap. 12:04:24 INFO - Thread 0 (crashed) 12:04:24 INFO - 0 libxul.so!xpc::IsInContentXBLScope(JSObject*) [jsfriendapi.h:b66baa5324ae : 674 + 0x0] 12:04:24 INFO - rax = 0x00007ff507932801 rdx = 0x00007fffbf0c45a0 12:04:24 INFO - rcx = 0x0000000000000000 rbx = 0x0000000000000001 12:04:24 INFO - rsi = 0x0000000000200020 rdi = 0x0000000000000000 12:04:24 INFO - rbp = 0x00007fffbf0c4540 rsp = 0x00007fffbf0c44e0 12:04:24 INFO - r8 = 0x0000000000000001 r9 = 0x00007ff512da2ad0 12:04:24 INFO - r10 = 0x00007fffbf0c42c8 r11 = 0x00007ff512da2ad0 12:04:24 INFO - r12 = 0x00007ff4ce1f2ee0 r13 = 0x00007fffbf0c4598 12:04:24 INFO - r14 = 0x0000000000000000 r15 = 0x00007ff4f3bfe8b0 12:04:24 INFO - rip = 0x00007ff50e02c725 12:04:24 INFO - Found by: given as instruction pointer in context 12:04:24 INFO - 1 libxul.so!nsINode::WrapObject(JSContext*, JS::Handle<JSObject*>) [nsINode.cpp:b66baa5324ae : 2782 + 0x9] 12:04:24 INFO - rbx = 0x0000000000000001 rbp = 0x00007fffbf0c4540 12:04:24 INFO - rsp = 0x00007fffbf0c44f0 r12 = 0x00007ff4ce1f2ee0 12:04:24 INFO - r13 = 0x00007fffbf0c4598 r14 = 0x0000000000000000 12:04:24 INFO - r15 = 0x00007ff4f3bfe8b0 rip = 0x00007ff50e54e6c9 12:04:24 INFO - Found by: call frame info 12:04:24 INFO - 2 libxul.so!mozilla::dom::Element::WrapObject(JSContext*, JS::Handle<JSObject*>) [Element.cpp:b66baa5324ae : 432 + 0x16] 12:04:24 INFO - rbx = 0x00007ff4ce1f2ee0 rbp = 0x00007fffbf0c4620 12:04:24 INFO - rsp = 0x00007fffbf0c4550 r12 = 0x00007fffbf0c490c 12:04:24 INFO - r13 = 0x0000000000000000 r14 = 0x0000000000000000 12:04:24 INFO - r15 = 0x00007ff4f3bfe8b0 rip = 0x00007ff50e488ae1 12:04:24 INFO - Found by: call frame info 12:04:24 INFO - 3 libxul.so!XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, XPCNativeInterface**, bool, nsresult*) [XPCConvert.cpp:b66baa5324ae : 785 + 0x30] 12:04:24 INFO - rbx = 0x0000000000000000 rbp = 0x00007fffbf0c47a0 12:04:24 INFO - rsp = 0x00007fffbf0c4630 r12 = 0x00007fffbf0c490c 12:04:24 INFO - r13 = 0x00007ff4ce1f2ee8 r14 = 0x0000000000000000 12:04:24 INFO - r15 = 0x00007ff4f3bfe8b0 rip = 0x00007ff50e04d635 12:04:24 INFO - Found by: call frame info 12:04:24 INFO - 4 libxul.so!XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, nsresult*) [XPCConvert.cpp:b66baa5324ae : 342 + 0x25] 12:04:24 INFO - rbx = 0x00007ff4ce1f2ee0 rbp = 0x00007fffbf0c48c0 12:04:24 INFO - rsp = 0x00007fffbf0c47b0 r12 = 0x00007fffbf0c490c 12:04:24 INFO - r13 = 0x00007fffbf0c4920 r14 = 0x00007fffbf0c4907 12:04:24 INFO - r15 = 0x0000000000000000 rip = 0x00007ff50e04db73 12:04:24 INFO - Found by: call frame info (full log at http://archive.mozilla.org/pub/firefox/try-builds/jdescottes@mozilla.com-b66baa5324ae2fe2b40e73153908b2ff557f358b/try-linux64-debug/try_ubuntu64_vm-debug_test-mochitest-devtools-chrome-5-bm122-tests1-linux64-build225.txt.gz) I will check if the event listener change listener is properly removed.

smaug: new stack trace in Comment 24 if you want to have a look

Julian, want to try this one then. Just added a null check !obj

These crashes do hint that event listener change listener is called easily with event targets which are from a scope which is already gone. Which is expected given that the callback is async.

@smaug : thanks! new try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=da6aba6da689

Some builds failed on the previous try push. Triggered another one : https://treeherder.mozilla.org/#/jobs?repo=try&revision=c03e1960ae4d , no errors. Relaunched another try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=49d14f3b869e

Cancelled https://treeherder.mozilla.org/#/jobs?repo=try&revision=49d14f3b869e (triggered from the wrong changeset) New try push at : https://treeherder.mozilla.org/#/jobs?repo=try&revision=67366b321333

Smaug: your last patch fixes the issue. How do we proceed to land this?

the patch needs a review. Once it has r+, feel free to land with other patches.

Comment on attachment 8705861 [details] [diff] [review] v2 for the crash Review of attachment 8705861 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/base/nsINode.cpp @@ +2780,5 @@ > > JS::Rooted<JSObject*> obj(aCx, WrapNode(aCx, aGivenProto)); > MOZ_ASSERT_IF(ChromeOnlyAccess(), > + !obj || xpc::IsInContentXBLScope(obj) || > + !xpc::UseContentXBLScope(js::GetObjectCompartment(obj))); Nit: I think it would be clearer to include the null check in the "IF" part of the assert. So: MOZ_ASSERT_IF(obj && ChromeOnlyAccess(), xpc::IsInContentXBLScope(obj) || xpc::UseContentXBLScope(js::GetObjectCompartment(obj))); ::: dom/bindings/BindingUtils.h @@ +1593,5 @@ > return JS::CurrentGlobalOrNull(cx); > } > > JSObject* parent = WrapNativeParentHelper<T>::Wrap(cx, p, cache); > + if (!useXBLScope || !parent) { Nit: I think it would be clearer to switch the order of these two checks. So: if (!parent || !useXBLScope) {

Comment on attachment 8706500 [details] [diff] [review] +comments Carrying over the R+ from smaug's last patch

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins https://reviewboard.mozilla.org/r/29739/#review27229

Try push with the latest patch from smaug applied : https://treeherder.mozilla.org/#/jobs?repo=try&revision=89860813412c&selectedJob=15329672

Last try push was successful, adding checkin-needed for both patches

this failed to apply: applying 4451dc137869 unable to find 'devtools/client/markupview/markup-view.js' for patching 3 out of 3 hunks FAILED -- saving rejects to file devtools/client/markupview/markup-view.js.rej unable to find 'devtools/client/markupview/test/browser_markupview_events.js' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/browser_markupview_events.js.rej unable to find 'devtools/client/markupview/test/doc_markup_events.html' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/doc_markup_events.html.rej unable to find 'devtools/client/markupview/test/helper_events_test_runner.js' for patching 1 out of 1 hunks FAILED -- saving rejects to file devtools/client/markupview/test/helper_events_test_runner.js.rej file devtools/server/tests/mochitest/test_inspector-mutations-events.html already exists 1 out of 1 hunks FAILED -- saving rejects to file devtools/server/tests/mochitest/test_inspector-mutations-events.html.rej patch failed to apply abort: fix up the merge and run hg transplant --continue could you take a look, thanks!

Comment on attachment 8704776 [details] MozReview Request: Bug 1157469 - use EventListenerService to update markupview event bubbles;r=bgrins Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29739/diff/4-5/

I reapplied the backed-out changeset locally using `hg graft -f d933300edeed`, which merged without conflicts. This is what I did yesterday as well for my try push, so I don't think a new try push is needed. I uploaded this new revision to mozreview. Could you try with the latest patch uploaded to reviewboard ? (131c9e4a65750af75a9fef8d3e59b755a18bf296)

https://hg.mozilla.org/integration/fx-team/rev/3791e857a33f https://hg.mozilla.org/integration/fx-team/rev/dd3a10e8fcce

https://hg.mozilla.org/mozilla-central/rev/3791e857a33f https://hg.mozilla.org/mozilla-central/rev/dd3a10e8fcce

[bugday-20160323] Status: RESOLVED,FIXED -> VERIFIED Comments: Test Successful. Component: Name Firefox Version 46.0b4 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: There is an event bubble next to the node Actual Results: As expected