Streamline xpcom thread event target helper methods
Categories
(Core :: XPCOM, enhancement, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox111 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
Details
Attachments
(4 files)
The setup around GetCurrentSerialEventTarget and related methods is a bit needlessly complex, as it was built as part of the work on Quantum DOM, which has since been abandoned.
|
Assignee | |
Comment 1•1 year ago
|
||
We aren't likely to try to make these changes any time soon, so cleaning out
these unnecessary methods which just return this will simplify things.
I was unable to find any calls to the .eventTarget getter in JS, which makes
sense, as the nsIThread type is only really used in JS as a wrapper around the
main thread in older code. Because of that, it has been removed as well.
Depends on D166604
|
|
Assignee | |
Comment 2•1 year ago
|
||
These callers are depending on the existing behaviour which always returns a
thread, and does not respect custom TaskQueues. In the next part, all remaining
callers will be switched to GetCurrentSerialEventTarget.
Depends on D166605
|
|
Assignee | |
Comment 3•1 year ago
|
||
This only changes the behaviour when called with a TaskQueue or other type
using SerialEventTargetGuard on the stack. They are being switched over as the
existing GetCurrentEventTarget method is being removed, as it is somewhat
confusing, and poorly documented.
Callers which need to get the current thread even when on a threadpool or
behind a TaskQueue were switched to GetCurrentEventTarget in the previous part.
Depends on D166606
|
|
Assignee | |
Comment 4•1 year ago
|
||
This method always returned GetMainThreadSerialEventTarget(). This patch
switches all callers over to use that method instead.
We can't easily switch all calls to be calls to NS_GetMainThread(), as there is
no version of that method returning a bare nsIThread* instance.
I didn't introduce one, as we may want to add a lock around mMainThread in the
future, which would require removing nsThreadManager::GetMainThreadWeak. As
this method only returns nsISerialEventTarget, it method could remain
implemented, however, by returning a statically allocated fake event target
which forwards dispatches (and QIs to nsIThread) to the real main thread.
Depends on D166607
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/aa9f2971bd6f Part 1: Remove quantum-dom nsIThread::EventTarget methods, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/d0267ac2256d Part 2: Switch some calls to GetCurrentEventTarget() to NS_GetCurrentThread, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/7c9b20eebcc8 Part 3: Replace all callers of GetCurrentEventTarget with GetCurrentSerialEventTarget, r=mccr8,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/ea05784d74c4 Part 4: Remove unnecessary GetMainThreadEventTarget, r=mccr8
|
||
Comment 6•1 year ago
|
||
Backed out for causing for causing perma failures in browser/components/firefoxview/tests/browser/browser_feature_callout_position.js
Backout link: https://hg.mozilla.org/integration/autoland/rev/0bbd4819fb46a437b424810a619a5d1c1cfa6905
Pushed by csabou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00c198908842 Part 1: Remove quantum-dom nsIThread::EventTarget methods, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/0b8c99f50fda Part 2: Switch some calls to GetCurrentEventTarget() to NS_GetCurrentThread, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/69d9a1c509e7 Part 3: Replace all callers of GetCurrentEventTarget with GetCurrentSerialEventTarget, r=mccr8,necko-reviewers,valentin https://hg.mozilla.org/integration/autoland/rev/a7465d2d89ba Part 4: Remove unnecessary GetMainThreadEventTarget, r=mccr8
|
||
Comment 8•1 year ago
|
||
Relanded this because it was not the cause of those failures.
|
||
Comment 9•1 year ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/00c198908842
https://hg.mozilla.org/mozilla-central/rev/0b8c99f50fda
https://hg.mozilla.org/mozilla-central/rev/69d9a1c509e7
https://hg.mozilla.org/mozilla-central/rev/a7465d2d89ba
Description
•