Closed Bug 1042642 Opened 6 years ago Closed 1 year ago

Get rid of eventSource in devtools/shared/client/*


(DevTools :: Framework, defect, P3)



(Not tracked)



(Reporter: ejpbruel, Assigned: yulia)




(6 files)

The sendActorEvent API from bug 797621 expects clients to be EventTargets. This means the API is broken for some of the existing clients, since they use eventSource instead, which is not API compatible with EventTarget.

This didn't surface before because none of the existing clients in dbg-client.jsm have an non-empty events array. However, adding an event to any of those clients will cause the API to break, because they don't have the required emit function from EventTarget (instead, they have notify, which is the eventSource equivalent).

As a quick fix, I'll be filing a patch in bug 1035206 that changes eventSource.notify to eventSource.emit. This will solve the immediate problem, but as a long term solution I suggest that we get rid of the eventSource API completely, and replace it with EventTarget. This will reduce code duplication, but will probably require some consumers of the existing clients to be updated.
Blocks: dbg-client
Mentor: ejpbruel
We should use toolkit/devtools/event-emitter.js instead.
Mentor: ejpbruel
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [nosdk]
The file discussed here has been moved to devtools/shared/client/main, which is already tracked by 1378922.
Closing as duplicate.
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1378922
Whiteboard: [nosdk]
I believe this bug is separate from bug 1378922.

The files in question now live in devtools/shared/client:

`eventSource` and all its usages should be replaced by event emitter instead.
Resolution: DUPLICATE → ---
Summary: Get rid of eventSource in dbg-client.jsm → Get rid of eventSource in devtools/shared/client/*
Product: Firefox → DevTools
Priority: P2 → P3
Assignee: nobody → ystartsev
Blocks: 1494796

a couple of notes here:

EnvironmentClient doesn't seem to use eventSource

ThreadClient conversion to protocol.js front needs ThreadClient to be moved to EventEmitter (which is why I picked this up)

DebuggerClient seems to be used in tests in places where ThreadClient should be use, examples:

After reviewing how the EnvironmentClient is used, it looks like the use of eventSource
might be some cruft from the past. Here is the try run:

with the exception of whatever is going on Window 7 (which appeares also on central), it looks like
things are working as expected. The environment client will eventually have the event emitter, once
it is moved to being a front.

adding @nchevobbe as a subscriber, as this touches a dependancy of the scratchpad.

Pushed by
Remove eventSource from EnvironmentClient; r=jdescottes
Keywords: leave-open
Component: Debugger → Framework

turns out we were incorrectly passing the threadClient to the environmentClient while
instantiating in one of the tests. It worked before because the threadClient exposed the API surface
of the debugger-client.

Pushed by
do not use DebuggerClient for functions that should only take ThreadClient; r=jdescottes
use EventEmitter instead of eventSource on the DebuggerClient; r=jdescottes
Use EventEmitter instead of EventSource for the threadClient; r=jdescottes
Stop using the DebuggerClient instance to listen for ThreadClient events in tests; r=jdescottes
Do not pass the threadClient to EnvironmentClient during instantiation in tests; r=jdescottes

Does this still need to remain open?

Flags: needinfo?(ystartsev)

no, this is now done.

Closed: 3 years ago1 year ago
Flags: needinfo?(ystartsev)
Resolution: --- → FIXED
Keywords: leave-open
You need to log in before you can comment on or make changes to this bug.