Add TestBrowserThreadBundle into RenderViewHostTestHarness. Kill some unnecessary real threads.

content/public/test/test_browser_thread_bundle.h

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// TestBrowserThreadBundle is a convenience class for creating a set of
+// TestBrowserThreads in unit tests.  For most tests, it is sufficient to
+// just instantiate the TestBrowserThreadBundle as a member variable.
+//
+// By default, each of the created TestBrowserThreads will be backed by
+// one MessageLoop running on the instantiating thread. If a test truly

[Jeffrey Yasskin, 2013/05/22 22:24:58]

How about "all of the created TestBrowserThreads will be backed by a single
shared MessageLoop". I think the current sentence admits the parsing that each
of the threads is backed by its own MessageLoop, even though that's not really
implementable.

[awong, 2013/05/24 23:39:28]

Done.

+// needs separate threads, it can do so by passing the apprpriate
+// combination of RealThreadsMask values during the TestBrowserThreadBundle
+// construction.
+//
+// The TestBrowserThreadBundle will attempt to drain the MessageLoop on

[Jeffrey Yasskin, 2013/05/22 22:24:58]

Could you document how to drain the MessageLoop in the middle of the test?
(content::RunAllPendingInMessageLoop(), right?) Otherwise I'd probably look for
a MessageLoop accessor and be confused. 

[awong, 2013/05/24 23:39:28]

Added a comment. I think calling base::RunLoop directly is better.

+// destruction.
+
+#ifndef BROWSER_THREAD_TEST_BUNDLE_H_
+#define BROWSER_THREAD_TEST_BUNDLE_H_
+
+#include "base/message_loop.h"
+#include "content/public/test/test_browser_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace content {
+
+class TestBrowserThreadBundle {
+ public:
+  // Used to specifying which named BrowserThreads should be backed by a real
+  // thread rather than main main thread.  The UI thread is always the main
+  // thread in a unit test so it is left out of this enum.
+  enum RealThreadsMask {

[Jeffrey Yasskin, 2013/05/22 22:24:58]

Instead of "real" maybe "separate" or "concurrent"? I guess "real" is accurate
too, so if you want to keep it, that's fine with me.

[awong, 2013/05/24 23:39:28]

I like real in this case. Shorter.

+    NO_REAL_THREAD = 0x0,
+    REAL_DB_THREAD = 0x01,
+    REAL_FILE_THREAD = 0x2,
+    REAL_FILE_USER_BLOCKING_THREAD = 0x4,
+    REAL_IO_THREAD = 0x8
+  };
+
+  TestBrowserThreadBundle();
+  explicit TestBrowserThreadBundle(int real_threads_mask);
+
+  ~TestBrowserThreadBundle();
+
+  TestBrowserThread* ui_thread() { return &ui_thread_; }

[Jeffrey Yasskin, 2013/05/22 22:24:58]

You could return a reference to be clear that these can't be NULL. The style
guide only prohibits non-const reference _parameters_:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Refere...

[awong, 2013/05/24 23:39:28]

I generally dislike non-const references (or really references in general).  In
this particular case, returning a non-const reference gets kinda funny if
someone was to hold a copy of it.

TestBrowserThread& db = bundle_.db_thread();
TestBrowserThread* db = &bundle_.db_thread();

both aren't very common patterns.

+  TestBrowserThread* db_thread() { return &db_thread_; }
+  TestBrowserThread* file_thread() { return &file_thread_; }
+  TestBrowserThread* file_user_blocking_thread() {
+    return &file_user_blocking_thread_;
+  }
+  TestBrowserThread* io_thread() { return &io_thread_; }
+
+ private:
+  MessageLoopForUI message_loop_;
+  TestBrowserThread ui_thread_;
+  TestBrowserThread db_thread_;
+  TestBrowserThread file_thread_;
+  TestBrowserThread file_user_blocking_thread_;
+  TestBrowserThread io_thread_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestBrowserThreadBundle);
+};
+
+}  // namespace content
+
+#endif /* BROWSER_THREAD_TEST_BUNDLE_H_ */