Add TestBrowserThreadBundle into RenderViewHostTestHarness. Kill some unnecessary real threads.

content/browser/browser_thread_impl.cc

Index: content/browser/browser_thread_impl.cc
diff --git a/content/browser/browser_thread_impl.cc b/content/browser/browser_thread_impl.cc
index 37f67bc191a1c1fb933932c053bfe82f84e8745e..97c0387e4a5d73d70d3e05cae64a13a7188f6d7e 100644
--- a/content/browser/browser_thread_impl.cc
+++ b/content/browser/browser_thread_impl.cc
@@ -70,8 +70,11 @@ BrowserThreadImpl::BrowserThreadImpl(ID identifier)
 
 BrowserThreadImpl::BrowserThreadImpl(ID identifier,
                                      base::MessageLoop* message_loop)
-    : Thread(message_loop->thread_name().c_str()), identifier_(identifier) {
-  set_message_loop(message_loop);
+    : Thread(message_loop ? message_loop->thread_name().c_str() :

[jam, 2013/05/31 17:29:25]

it seems odd using this constructor and passing in a NULL message loop. i
understand you're doing this to slightly make things easier in
TestBrowserThreadBundle's constructor. it seems that instead that one could just
use scoped pointers and call the different constructors manually depending on
the flags

[awong, 2013/05/31 20:57:17]

I considered that initially, but I think this is cleaner.

Why make the caller have to change the code used to instantiate a BrowserThread
when they want to support 2 different configurations? Also, if these were
overloaded functions rather than constructors, I would want to make one just
call the other. We only have this weird duplicated code issue because C++98
doesn't have delegated constructor like Java and C++11.

So to me, the semantics of making this understand NULL feels like the right API
reflection of the configuration options.

There's also the small implied cost of N dynamic allocations per TEST_F with
associated heap fragmentation, but that's probably ignorable.

[jam, 2013/05/31 22:53:04]

I really don't like having two ways of doing the same thing. In this case, it
means that someone who's trying to use this constructor with a MessageLoop, but
for some error condition passes a NULL ML, would now not get a crash and find
out right away.

[awong, 2013/06/05 00:18:48]

I'm trying to imagine a situation when someone could cause a NULL MessageLoop
and am having a hard time seeing it since this api is only used in tests.

In either case, I can't really see a cost to either API.  I've moved
TestBrowserThreadBundle to dynamically allocate things.

+             g_browser_thread_names[identifier]),
+      identifier_(identifier) {
+  if (message_loop)
+    set_message_loop(message_loop);
   Initialize();
 }