1630802 - Remove AutoEnter

Status: RESOLVED FIXED

(Core :: XPCOM, task)

Description

[Jean-Yves Avenard [:jya]]

AbstractThread::AutoEnter was added in bug 1364821 has a work around conflicts between the main-thread AbstractThread and the MessageLoopAbstractThreadWrapper that happened to get initialized from the main thread.

MessageLoopAbstractThreadWrapper was set the AbstractThread::sCurrentThreadTLS object which would later influence what AbstractThread::GetCurrent() would return.

MessageLoopAbstractThreadWrapper no longer exists. So AutoEnter is no longer necessary and should be removed as it caused unnecessary logical burden.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P3. Make AbstractThread::GetCurrent() return MainThread on the main thread. r?bholley

prior bug 1364821, AbstractThread::GetCurrent() would always return AbstractThread::MainThread() when called from the main thread.
After this change, we had to run within an AutoEnter scope.

A hidden side effect of this change was that under most cases AbstractThread::MainThread::Dispatch() would no longer use the tail dispatcher to dispatch a task.

It can be safely assume that whenever you're on the main thread, the equivalent AbstractThread is usable.

In the next commit, we will be removing AutoEnter entirely.

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P2. Do not use AbstractThread too early. r?padenot

AbstractThread::MainThread() requires TailDispatcher to work which itself relies on the nsCycleCollectors to have been initialised.

This early in the XPCOM initilization, it doesn't work.

A bug was hiding the fact that the tail dispatcher wasn't available on the main thread by default.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P8. Remove unnecessary AutoEnter. r?bholley

AutoEnter was an attempt around a race between AbstractThread and MessageLoopAbstractThreadWrap that would cause AbstractThread::GetCurrent() to return an incorrect value. MessageLoopAbstractThreadWrapper is no more and as such AutoEnter is no longer required.

Include reversal of commit 3218d3c2662a (bug 1529399)

Depends on D71148

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P1. Cleanup AbstractThread TLS lookup. r?gerald

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P4. Remove unused member. r?bholley

It lead AbstractThread assumption that there can only be one active per thread to be false.

Depends on D71148

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P9. EventTargetWrapper runners don't need to be cancellable. r?bholley

It was required once upon a time to be able to use MozPromise on Workers.
Today a MozPromise work with nsISerialEventTarget and no longer rely on this. It can go.

Depends on D71279

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P5. Remove no longer required GetManagerAbstractThread. r?bholley,mattwoodrow

Once upon a time, a MozPromise required an AbstractThread as target thread. So we had to wrap the current child manager thread into an AbstractThread.

This is no longer the case, MozPromise now work with nsISerialEventTarger, which a nsIThread is. The AbstractThread used didn't support tail dispatch so the replacement was straight forward.

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P6. Always return AbstractThread::MainThread for AbstractMainThreadFor. r?bholley

This mechanism is currently unused (see bug 1624819), we can re-assess the issue once it becomes needed.

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1630802 - P7. Remove CreateEventTargetWrapper and ensure that only a single AbstractThread exists par nsIThread. r?bholley

Depends on D71491

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e5316093afd
P1. Cleanup AbstractThread TLS lookup. r=gerald
https://hg.mozilla.org/integration/autoland/rev/a5b19d93ab3f
P2. Do not use AbstractThread too early. r=padenot
https://hg.mozilla.org/integration/autoland/rev/9ecfeb1915cd
P3. Make AbstractThread::GetCurrent() return MainThread on the main thread. r=bholley
https://hg.mozilla.org/integration/autoland/rev/b94e2c408ecc
P4. Remove unused member. r=bholley
https://hg.mozilla.org/integration/autoland/rev/5c3d7c51ecc3
P5. Remove no longer required GetManagerAbstractThread. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/8d9486cd19e8
P6. Always return AbstractThread::MainThread for AbstractMainThreadFor. r=bholley
https://hg.mozilla.org/integration/autoland/rev/7d104d25415c
P7. Remove CreateEventTargetWrapper and ensure that only a single AbstractThread exists par nsIThread. r=bholley
https://hg.mozilla.org/integration/autoland/rev/c2a43696053d
P8. Remove unnecessary AutoEnter. r=bholley
https://hg.mozilla.org/integration/autoland/rev/ce0291e3f743
P9. EventTargetWrapper runners don't need to be cancellable. r=bholley

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1e5316093afd
https://hg.mozilla.org/mozilla-central/rev/a5b19d93ab3f
https://hg.mozilla.org/mozilla-central/rev/9ecfeb1915cd
https://hg.mozilla.org/mozilla-central/rev/b94e2c408ecc
https://hg.mozilla.org/mozilla-central/rev/5c3d7c51ecc3
https://hg.mozilla.org/mozilla-central/rev/8d9486cd19e8
https://hg.mozilla.org/mozilla-central/rev/7d104d25415c
https://hg.mozilla.org/mozilla-central/rev/c2a43696053d
https://hg.mozilla.org/mozilla-central/rev/ce0291e3f743