Make base::MessageLoops have names at construction time

Make binding of a thread to an existing message loops be done at Thread construction time.

This makes it possible for base::thread to tell debuggers/tracing systems the
name of a thread, which was previously impossible because base::Thread didn't know at construction time whether the thread existed or not.

[brettw, 2011-07-29 17:04:12]

I don't quite follow the CL description. What was previously impossible? It
looks like this is a slight cleanup of the construction but that it does the
same thing.

[nduca, 2011-08-02 01:08:55]

Sorry about that!

The high level proposal I had here was to try to centralize thread naming a bit
more. Since putting up patchset 1, I've come up with a new approach, which is to
make message_loop's have names.

Here's why I think this might be a good thing... right now, thread names get set
differently depending on whether a thread is the main thread versus a thread
created by base::Thread. This works fine, but means that any main loops that
wish to have a ThreadNme in a debugger have to call into PlatformThread::SetName
explicitly. This goes into platform-specific code, meaning its an awkward place
to put platform-neutral code like debug::TraceLog::SetThreadName.

So, I thought, wouldn't it be great to make every message_loop have a name. That
way we can have a single centralized place for calling into both
base/dynamic_annotations, PlatformThread::SetName, and eventually
TraceEvent::SetThreadName.

I'm fine if this doesn't feel right by you... I just wanted to throw this out as
a possibility before I went down a path of instrumenting each PlatformThread.

[nduca, 2011-08-02 01:26:39]

Ok, patch uploaded.

http://codereview.chromium.org/7529003/diff/3001/base/message_loop.h
File base/message_loop.h (right):

http://codereview.chromium.org/7529003/diff/3001/base/message_loop.h#newcode110
base/message_loop.h:110: explicit MessageLoop(const char* thread_name, Type type
= TYPE_DEFAULT);
I'm fine making the thread_name optional, if you think that's too strong a
requirement. :)

[brettw, 2011-08-02 18:06:01]

On Windows, I've always seen thread names in the debugger. Is this necessary for
other platforms or something?

[nduca, 2011-08-02 18:36:20]

On 2011/08/02 18:06:01, brettw wrote:
> On Windows, I've always seen thread names in the debugger. Is this necessary
for
> other platforms or something?

That is done by making a PlatformThread::SetName call in every main loop, and by
making a PlatformThread::SetName call in base::Thread's run function. I.e. in
two places.

trace_event is base/debug concept. I have a cl up for review that adds a
SetCurrentThreadName method to that class:
http://codereview.chromium.org/7495031/

The question is how to make that nice "I just see thread names in the debugger"
also work for trace event. There are two options:

1. Making each of the PlatformThread::SetName impls make a call to
trace_event::SetCurrentThreadName. But that feels like a layering violation to
me -- making platform-specific code call out to another part of base.
2. We could clean up the code so that there's one centralized point in debug
where "thread names" enter. At that centralized point, on message loop, we can
then call down to PlatformThread::SetName to set the debugger thread name, and
to any other services like trace event that also want to know it.

#2 is what I was trying to do here.

[nduca, 2011-08-02 19:27:19]

Hmm, some potentially new ideas coming from a brainstorm with Siggi. I'm going
to ditch this codereview.