Get rid of eventSource in devtools/shared/client/*
Categories
(DevTools :: Framework, defect, P3)
Tracking
(Not tracked)
People
(Reporter: ejpbruel, Assigned: yulia)
References
Details
Attachments
(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.
|
Reporter | |
Updated•10 years ago
|
|
|
Reporter | |
Updated•10 years ago
|
|
||
Comment 1•10 years ago
|
||
We should use toolkit/devtools/event-emitter.js instead.
|
||
Updated•8 years ago
|
|
||
Updated•7 years ago
|
|
||
Comment 2•7 years ago
|
||
The file discussed here has been moved to devtools/shared/client/main, which is already tracked by 1378922. Closing as duplicate.
|
|
||
Updated•7 years ago
|
I believe this bug is separate from bug 1378922. The files in question now live in devtools/shared/client: https://searchfox.org/mozilla-central/search?q=eventSource&case=true®exp=false&path=devtools%2Fshared%2Fclient `eventSource` and all its usages should be replaced by event emitter instead.
|
||
Updated•6 years ago
|
|
||
Updated•6 years ago
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 4•5 years ago
|
||
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: https://searchfox.org/mozilla-central/search?q=client.addListener(%22paused&case=false®exp=false&path=devtools
|
|
Assignee | |
Comment 5•5 years ago
|
||
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:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=242251058&revision=df4bb52f188f79b8006e8c40401e5af2258493ce
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 ystartsev@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72e61b1971a3 Remove eventSource from EnvironmentClient; r=jdescottes
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 7•5 years ago
|
||
|
|
Assignee | |
Comment 8•5 years ago
|
||
|
|
Assignee | |
Comment 9•5 years ago
|
||
|
|
Assignee | |
Comment 10•5 years ago
|
||
|
||
Comment 11•5 years ago
|
||
| bugherder | ||
|
|
||
Updated•5 years ago
|
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 12•5 years ago
|
||
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.
|
||
Comment 13•5 years ago
|
||
Pushed by ystartsev@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/453945168753 do not use DebuggerClient for functions that should only take ThreadClient; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/cef2f6228b9b use EventEmitter instead of eventSource on the DebuggerClient; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/035e95bfd41c Use EventEmitter instead of EventSource for the threadClient; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/84e5d70c6b5e Stop using the DebuggerClient instance to listen for ThreadClient events in tests; r=jdescottes https://hg.mozilla.org/integration/autoland/rev/daf1de2ade3b Do not pass the threadClient to EnvironmentClient during instantiation in tests; r=jdescottes
|
||
Comment 14•5 years ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 16•5 years ago
|
||
no, this is now done.
|
|
Assignee | |
Updated•5 years ago
|
Description
•