Failing tests: devtools/server/tests/mochitest/test_connection-manager.html devtools/server/tests/mochitest/test_inspector-pick-color.html The refactoring is currently only on: https://github.com/zer0/gecko/tree/event-emitter-1381542 We need to address the test failures before land this patch in m-c.

Here the original try build with the failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bba13e27a2371fa8aad68b9b227534b31829cb0d Those failures are most likely due the breaking change in how the `EventEmitter` emits event. Previously, the first argument was the type event: myEmitter.on("custom-event", (eventType, message) => { ... }); Now the first argument is the message: myEmitter.on("custom-event", (message) => { ... }); In the majority of the scenario the `eventType` is ignored by our code, so we should just remove it from the function's signature. For more details, see: https://github.com/devtools-html/snippets-for-removing-the-sdk/#events

I have been looking at this yesterday. I've got something that seems to work when you run DevTools, but fails a lot of tests. Changing all event-emitters in devtools/server to be the new one is fairly easy, but then it gets more involved, because we require some scripts in devtools/shared from both client and server. In particular, the DevTools protocol transport library is there. I haven't solved all the issues related to that yet. I'm not assigning this bug to myself just yet, because I don't know if I'll be able to work on it enough to actually solve the remaining pieces. But make sure to start from my patch if you want to attempt this (I'll attach it next).

Try build with this patch applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9d5771876c23d4c1a3814e4a230dd206dd5268b&group_state=expanded

Comment on attachment 8954740 [details] Bug 1382609 - Remove old-event-emitter usage from devtools/server; . https://reviewboard.mozilla.org/r/223874/#review230212 This looks good. Thank you Nicolas. I only have a few minor comment that you can feel free to ignore. I'm just concerned about the impact of the change. I mean, this is the right change, but I was expecting this to basically require the whole rest of the code to be refactored to the new event-emitter too. But if try is happy and devtools still works, then I'm happy too. ::: devtools/shared/transport/tests/unit/test_queue.js:115 (Diff revision 1) > info(Object.keys(DebuggerServer._connections)); > for (let connId in DebuggerServer._connections) { > DebuggerServer._connections[connId].onBulkPacket = on_bulk_packet; > } > > - DebuggerServer.on("connectionchange", (event, type) => { > + DebuggerServer.on("connectionchange", (type) => { nit: no need for parens around a single parameter in arrow functions. ::: devtools/shared/transport/tests/unit/test_transport_bulk.js:88 (Diff revision 1) > info(Object.keys(DebuggerServer._connections)); > for (let connId in DebuggerServer._connections) { > DebuggerServer._connections[connId].onBulkPacket = on_bulk_packet; > } > > - DebuggerServer.on("connectionchange", (event, type) => { > + DebuggerServer.on("connectionchange", (type) => { same here ::: devtools/shared/transport/tests/unit/test_transport_events.js:31 (Diff revision 1) > - let rootReceived = transport.once("packet", (event, packet) => { > - info(`Packet event: ${event} ${JSON.stringify(packet)}`); > + let rootReceived = transport.once("packet", (packet) => { > + info(`Packet event: ${JSON.stringify(packet)}`); > - Assert.equal(event, "packet"); > Assert.equal(packet.from, "root"); > }); > > transport.ready(); > await rootReceived; > > - let echoSent = transport.once("send", (event, packet) => { > - info(`Send event: ${event} ${JSON.stringify(packet)}`); > + let echoSent = transport.once("send", (packet) => { > + info(`Send event: ${JSON.stringify(packet)}`); > - Assert.equal(event, "send"); > Assert.equal(packet.to, "root"); > Assert.equal(packet.type, "echo"); > }); > > - let echoReceived = transport.once("packet", (event, packet) => { > - info(`Packet event: ${event} ${JSON.stringify(packet)}`); > + let echoReceived = transport.once("packet", (packet) => { > + info(`Packet event: ${JSON.stringify(packet)}`); and here. ::: devtools/shared/transport/tests/unit/test_transport_events.js:59 (Diff revision 1) > - let clientClosed = transport.once("close", (event) => { > - info(`Close event: ${event}`); > + let clientClosed = transport.once("close", () => { > + info(`Close event`); > - Assert.equal(event, "close"); > }); > > - let serverClosed = DebuggerServer.once("connectionchange", (event, type) => { > + let serverClosed = DebuggerServer.once("connectionchange", (type) => { and here

Comment on attachment 8954740 [details] Bug 1382609 - Remove old-event-emitter usage from devtools/server; . https://reviewboard.mozilla.org/r/223874/#review230212 Thanks for the review Patrick. I fixed the nits, and I'm going to push to TRY on different platform (previous one was linux only) so we are extra careful about this.

Pushed by nchevobbe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/784928adcf87 Remove old-event-emitter usage from devtools/server; r=pbro.