Give each HistoryService instance it's own backend thread.

I had originally planned to push history_thread up to BrowserProcess,
but was scared away by the comment in ~Profile that talks about HistoryService
calling back into the bookmark bar model, and that it depended on join()ing at
that particular time to ensure this doesn't happen after the bookmark bar model has been reset.

I didn't use scoped_ptr for the thread because it makes the little dance
in CleanUp awkward.

TEST=any existing test that exersizes the history service. I added a ProfileManager
test that would fail without this change.

Review URL: http://codereview.chromium.org/73012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@13703 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 42 insertions, 25 deletions

diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 313abad..55b9dc2 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -190,7 +190,6 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
   void CreateIOThread();
   void CreateFileThread();
   void CreateDBThread();
-  void CreateSafeBrowsingThread();
   void CreateTemplateURLModel();
   void CreateProfileManager();
   void CreateWebDataService();
diff --git a/chrome/browser/chrome_thread.cc b/chrome/browser/chrome_thread.cc
index b8421b8..9c0d10c 100644
--- a/chrome/browser/chrome_thread.cc
+++ b/chrome/browser/chrome_thread.cc
@@ -9,7 +9,6 @@ static const char* chrome_thread_names[ChromeThread::ID_COUNT] = {
   "Chrome_IOThread",  // IO
   "Chrome_FileThread",  // FILE
   "Chrome_DBThread",  // DB
-  "Chrome_HistoryThread",  // HISTORY
 };
 
 Lock ChromeThread::lock_;
@@ -18,7 +17,6 @@ ChromeThread* ChromeThread::chrome_threads_[ID_COUNT] = {
   NULL,  // IO
   NULL,  // FILE
   NULL,  // DB
-  NULL,  // HISTORY
 };
 
 ChromeThread::ChromeThread(ChromeThread::ID identifier)
diff --git a/chrome/browser/chrome_thread.h b/chrome/browser/chrome_thread.h
index 53fa50b..6570f92 100644
--- a/chrome/browser/chrome_thread.h
+++ b/chrome/browser/chrome_thread.h
@@ -38,9 +38,6 @@ class ChromeThread : public base::Thread {
     // This is the thread that interacts with the database.
     DB,
 
-    // This is the thread that interacts with the history database.
-    HISTORY,
-
     // This identifier does not represent a thread.  Instead it counts the
     // number of well-known threads.  Insert new well-known threads before this
     // identifier.
@@ -72,7 +69,7 @@ class ChromeThread : public base::Thread {
 
   // An array of the ChromeThread objects.  This array is protected by |lock_|.
   // The threads are not owned by this array.  Typically, the threads are owned
-  // on the IU thread by the g_browser_process object.  ChromeThreads remove
+  // on the UI thread by the g_browser_process object.  ChromeThreads remove
   // themselves from this array upon destruction.
   static ChromeThread* chrome_threads_[ID_COUNT];
 };
diff --git a/chrome/browser/chrome_thread_unittest.cc b/chrome/browser/chrome_thread_unittest.cc
index c68cbf0..0bcef05 100644
--- a/chrome/browser/chrome_thread_unittest.cc
+++ b/chrome/browser/chrome_thread_unittest.cc
@@ -14,36 +14,30 @@ TEST_F(ChromeThreadTest, Get) {
   scoped_ptr<ChromeThread> io_thread;
   scoped_ptr<ChromeThread> file_thread;
   scoped_ptr<ChromeThread> db_thread;
-  scoped_ptr<ChromeThread> history_thread;
 
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) == NULL);
-  EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::HISTORY) == NULL);
 
   // Phase 1: Create threads.
 
   io_thread.reset(new ChromeThread(ChromeThread::IO));
   file_thread.reset(new ChromeThread(ChromeThread::FILE));
   db_thread.reset(new ChromeThread(ChromeThread::DB));
-  history_thread.reset(new ChromeThread(ChromeThread::HISTORY));
 
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) == NULL);
-  EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::HISTORY) == NULL);
 
   // Phase 2: Start the threads.
 
   io_thread->Start();
   file_thread->Start();
   db_thread->Start();
-  history_thread->Start();
 
   EXPECT_TRUE(io_thread->message_loop() != NULL);
   EXPECT_TRUE(file_thread->message_loop() != NULL);
   EXPECT_TRUE(db_thread->message_loop() != NULL);
-  EXPECT_TRUE(history_thread->message_loop() != NULL);
 
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) ==
               io_thread->message_loop());
@@ -51,30 +45,24 @@ TEST_F(ChromeThreadTest, Get) {
               file_thread->message_loop());
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) ==
               db_thread->message_loop());
-  EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::HISTORY) ==
-              history_thread->message_loop());
 
   // Phase 3: Stop the threads.
 
   io_thread->Stop();
   file_thread->Stop();
   db_thread->Stop();
-  history_thread->Stop();
 
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) == NULL);
-  EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::HISTORY) == NULL);
 
   // Phase 4: Destroy the threads.
 
   io_thread.reset();
   file_thread.reset();
   db_thread.reset();
-  history_thread.reset();
 
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::IO) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::FILE) == NULL);
   EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::DB) == NULL);
-  EXPECT_TRUE(ChromeThread::GetMessageLoop(ChromeThread::HISTORY) == NULL);
 }
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index 7e1130c..1e93694 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -51,6 +51,8 @@
 using base::Time;
 using history::HistoryBackend;
 
+static const char* kHistoryThreadName = "Chrome_HistoryThread";
+
 // Sends messages from the backend to us on the main thread. This must be a
 // separate class from the history service so that it can hold a reference to
 // the history service (otherwise we would have to manually AddRef and
@@ -96,7 +98,7 @@ class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
 const history::StarID HistoryService::kBookmarkBarID = 1;
 
 HistoryService::HistoryService()
-    : thread_(new ChromeThread(ChromeThread::HISTORY)),
+    : thread_(new base::Thread(kHistoryThreadName)),
       profile_(NULL),
       backend_loaded_(false) {
   // Is NULL when running generate_profile.
@@ -108,7 +110,7 @@ HistoryService::HistoryService()
 }
 
 HistoryService::HistoryService(Profile* profile)
-    : thread_(new ChromeThread(ChromeThread::HISTORY)),
+    : thread_(new base::Thread(kHistoryThreadName)),
       profile_(profile),
       backend_loaded_(false) {
   NotificationService::current()->AddObserver(
@@ -175,7 +177,7 @@ void HistoryService::Cleanup() {
   // Delete the thread, which joins with the background thread. We defensively
   // NULL the pointer before deleting it in case somebody tries to use it
   // during shutdown, but this shouldn't happen.
-  ChromeThread* thread = thread_;
+  base::Thread* thread = thread_;
   thread_ = NULL;
   delete thread;
 }
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index eb62217..3804b8e 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -24,7 +24,6 @@
 #include "chrome/common/ref_counted_util.h"
 
 class BookmarkService;
-class ChromeThread;
 struct DownloadCreateInfo;
 class FilePath;
 class GURL;
@@ -39,7 +38,8 @@ class SkBitmap;
 struct ThumbnailScore;
 
 namespace base {
-  class Time;
+class Thread;
+class Time;
 }
 
 namespace history {
@@ -742,7 +742,7 @@ class HistoryService : public CancelableRequestProvider,
   CancelableRequestConsumer internal_consumer_;
 
   // The thread used by the history service to run complicated operations
-  ChromeThread* thread_;
+  base::Thread* thread_;
 
   // This class has most of the implementation and runs on the 'thread_'.
   // You MUST communicate with this class ONLY through the thread_'s
diff --git a/chrome/browser/profile_manager_unittest.cc b/chrome/browser/profile_manager_unittest.cc
index b4cf233..e7f03ec 100644
--- a/chrome/browser/profile_manager_unittest.cc
+++ b/chrome/browser/profile_manager_unittest.cc
@@ -85,3 +85,36 @@ TEST_F(ProfileManagerTest, CreateProfile) {
   ASSERT_EQ(L"new-profile", prefs->GetString(prefs::kProfileID));
 #endif
 }
+
+TEST_F(ProfileManagerTest, CreateAndUseTwoProfiles) {
+  FilePath source_path;
+  PathService::Get(chrome::DIR_TEST_DATA, &source_path);
+  source_path = source_path.Append(FILE_PATH_LITERAL("profiles"));
+  source_path = source_path.Append(FILE_PATH_LITERAL("sample"));
+
+  FilePath dest_path1 = test_dir_;
+  dest_path1 = dest_path1.Append(FILE_PATH_LITERAL("New Profile 1"));
+  
+  FilePath dest_path2 = test_dir_;
+  dest_path2 = dest_path2.Append(FILE_PATH_LITERAL("New Profile 2"));
+
+  scoped_ptr<Profile> profile1;
+  scoped_ptr<Profile> profile2;
+
+  // Successfully create the profiles.
+  profile1.reset(ProfileManager::CreateProfile(dest_path1, L"New Profile 1",
+      L"", L"new-profile-1"));
+  ASSERT_TRUE(profile1.get());
+
+  profile2.reset(ProfileManager::CreateProfile(dest_path2, L"New Profile 2",
+      L"", L"new-profile-2"));
+  ASSERT_TRUE(profile2.get());
+
+  // Force lazy-init of some profile services to simulate use.
+  EXPECT_TRUE(profile1->GetHistoryService(Profile::EXPLICIT_ACCESS));
+  EXPECT_TRUE(profile1->GetBookmarkModel());
+  EXPECT_TRUE(profile2->GetBookmarkModel());
+  EXPECT_TRUE(profile2->GetHistoryService(Profile::EXPLICIT_ACCESS));
+  profile1.reset();
+  profile2.reset();
+}