18078 - [arch] need consistent shortcuts for accessing threads

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Warren Harris]

It turns out that nsIProxyObjectManager uses NULL to mean "the UI thread"
whereas necko has various places where NULL means "the current thread". I'd like
to see this cleaned up.

I think the best way to approach this is to do this: Add the following special
constants to nsIEventQueueService.h:

#define NS_CURRENT_THREAD    ((PRThread*)0)
#define NS_UI_THREAD         ((PRThread*)1)

Then fix nsEventQueueServiceImpl::GetThreadEventQueue to handle these special
constants.

Then fix nsAsyncStreamObserver::Init to not special case NULL so that it simply
propagates its arguments to nsIEventQueueServiceImpl::GetThreadEventQueue.

Then fix up the callers of nsAsyncStreamObserver::Init to be explicit about
using the constants instead of NULL for clarity (e.g. NS_NewAsyncStreamObserver,
NS_NewAsyncStreamListener, nsIChannel::AsyncOpen, AsyncRead, ... (you'll have to
find everything that flows into here)). Also you should fix the callers of the
nsIProxyObjectManager routines.

Let me know if you want me to take this. I don't mind.

One final note: Maybe nsIThread should factor into this equation in some way
too. Ideally, everyone would use nsIThread instead of PRThread, but that's a
much bigger task. If we could achieve this, the constants would become arguments
to nsIThread::GetIThread, and nsIEventQueueService would deal with nsIThreads
instead of PRThreads.

Comment 1

[Brendan Eich [:brendan]]

Those SIG_IGN like castings of small ints are great worse-is-better hacking.
The choice of ((PRThread*)1) for UI is good too, because another better-is-worse
long term thing we should do is multithread or apartment-thread our UI/layout
code, so this constant goes away.  Whether that happens before we switch from
PRThread to nsIThread, I would not want to guess!

/be

Comment 2

[Doug Turner (:dougt)]

I am fixing this now.  I think that I will ask you warren to take a look at
converting necko to these new defines.

I also have to add two more defines so that we do not have to case, so here is
the total list:

#define NS_CURRENT_THREAD    ((PRThread*)0)
#define NS_CURRENT_EVENTQ    ((nsIEventQueue*)0)

#define NS_UI_THREAD         ((PRThread*)1)
#define NS_UI_THREAD_EVENTQ  ((nsIEventQueue*)1)

Comment 3

[Warren Harris]

Cool. Assign it to me when your done.

Comment 4

[Doug Turner (:dougt)]

Fixes checked in.  Warren, you can update necko.

Comment 5

[Warren Harris]

Doug: Looks like you already updated all the call sites. Marking fixed.