Try to fix ProfileBrowserTest, again.

Previous CL (which caused memleaks):
https://codereview.chromium.org/225693005

BUG=165760, 140882, 141141, 141517, 142787

Review URL: https://codereview.chromium.org/246923004

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

chrome/browser/profiles/profile_browsertest.cc

@@ -7,6 +7,7 @@
 #include "base/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/prefs/pref_service.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/version.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/chrome_version_service.h"
@@ -36,6 +37,20 @@ void CreatePrefsFileInDirectory(const base::FilePath& directory_path) {
   ASSERT_TRUE(base::WriteFile(pref_path, data.c_str(), data.size()));
 }
 
+scoped_ptr<Profile> CreateProfile(
+    const base::FilePath& path,
+    Profile::Delegate* delegate,
+    Profile::CreateMode create_mode) {
+  scoped_ptr<Profile> profile(Profile::CreateProfile(
+      path, delegate, create_mode));
+  EXPECT_TRUE(profile.get());
+  // This is necessary to avoid a memleak from BookmarkModel::Load.
+  // Unfortunately, this also results in warnings during debug runs.
+  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
+      StartDeferredTaskRunners();
+  return profile.Pass();
+}
+
 void CheckChromeVersion(Profile *profile, bool is_new) {
   std::string created_by_version;
   if (is_new) {
@@ -50,6 +65,23 @@ void CheckChromeVersion(Profile *profile, bool is_new) {
   EXPECT_EQ(created_by_version, pref_version);
 }
 
+void BlockThread(
+    base::WaitableEvent* is_blocked,
+    base::WaitableEvent* unblock) {
+  is_blocked->Signal();
+  unblock->Wait();
+}
+
+void SpinThreads() {
+  // Give threads a chance to do their stuff before shutting down (i.e.
+  // deleting scoped temp dir etc).
+  // Should not be necessary anymore once Profile deletion is fixed
+  // (see crbug.com/88586).
+  content::RunAllPendingInMessageLoop();
+  content::RunAllPendingInMessageLoop(content::BrowserThread::DB);
+  content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
+}
+
 }  // namespace
 
 typedef InProcessBrowserTest ProfileBrowserTest;
@@ -66,12 +98,13 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, true));
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  CheckChromeVersion(profile.get(), true);
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
+  {
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
+    CheckChromeVersion(profile.get(), true);
+  }
+
+  SpinThreads();
 }
 
 // Test OnProfileCreate is called with is_new_profile set to false when
@@ -86,12 +119,13 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, false));
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  CheckChromeVersion(profile.get(), false);
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
+  {
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
+    CheckChromeVersion(profile.get(), false);
+  }
+
+  SpinThreads();
 }
 
 // Test OnProfileCreate is called with is_new_profile set to true when
@@ -105,18 +139,20 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, true));
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
-
-  // Wait for the profile to be created.
-  content::WindowedNotificationObserver observer(
-      chrome::NOTIFICATION_PROFILE_CREATED,
-      content::Source<Profile>(profile.get()));
-  observer.Wait();
-  CheckChromeVersion(profile.get(), true);
+  {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_PROFILE_CREATED,
+        content::NotificationService::AllSources());
+
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
+
+    // Wait for the profile to be created.
+    observer.Wait();
+    CheckChromeVersion(profile.get(), true);
+  }
+
+  SpinThreads();
 }
 
 // Test OnProfileCreate is called with is_new_profile set to false when
@@ -130,18 +166,21 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
 
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, false));
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
-
-  // Wait for the profile to be created.
-  content::WindowedNotificationObserver observer(
-      chrome::NOTIFICATION_PROFILE_CREATED,
-      content::Source<Profile>(profile.get()));
-  observer.Wait();
-  CheckChromeVersion(profile.get(), false);
+
+  {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_PROFILE_CREATED,
+        content::NotificationService::AllSources());
+
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
+
+    // Wait for the profile to be created.
+    observer.Wait();
+    CheckChromeVersion(profile.get(), false);
+  }
+
+  SpinThreads();
 }
 
 // Test that a README file is created for profiles that didn't have it.
@@ -156,23 +195,26 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, DISABLED_ProfileReadmeCreated) {
   // No delay before README creation.
   ProfileImpl::create_readme_delay_ms = 0;
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
+  {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_PROFILE_CREATED,
+        content::NotificationService::AllSources());
 
-  // Wait for the profile to be created.
-  content::WindowedNotificationObserver observer(
-      chrome::NOTIFICATION_PROFILE_CREATED,
-      content::Source<Profile>(profile.get()));
-  observer.Wait();
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
 
-  content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
+    // Wait for the profile to be created.
+    observer.Wait();
 
-  // Verify that README exists.
-  EXPECT_TRUE(base::PathExists(
-      temp_dir.path().Append(chrome::kReadmeFilename)));
+    // Wait for file thread to create the README.
+    content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
+
+    // Verify that README exists.
+    EXPECT_TRUE(base::PathExists(
+        temp_dir.path().Append(chrome::kReadmeFilename)));
+  }
+
+  SpinThreads();
 }
 
 // Test that Profile can be deleted before README file is created.
@@ -186,18 +228,28 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, ProfileDeletedBeforeReadmeCreated) {
   // No delay before README creation.
   ProfileImpl::create_readme_delay_ms = 0;
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
+  base::WaitableEvent is_blocked(false, false);
+  base::WaitableEvent* unblock = new base::WaitableEvent(false, false);
+
+  // Block file thread.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::FILE, FROM_HERE,
+      base::Bind(&BlockThread, &is_blocked, base::Owned(unblock)));
+  // Wait for file thread to actually be blocked.
+  is_blocked.Wait();
+
+  scoped_ptr<Profile> profile(CreateProfile(
       temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
 
-  // Delete the Profile instance and run pending tasks (this includes the task
-  // for README creation).
+  // Delete the Profile instance before we give the file thread a chance to
+  // create the README.
   profile.reset();
-  content::RunAllPendingInMessageLoop();
-  content::RunAllPendingInMessageLoop(content::BrowserThread::DB);
-  content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
+
+  // Now unblock the file thread again and run pending tasks (this includes the
+  // task for README creation).
+  unblock->Signal();
+
+  SpinThreads();
 }
 
 // Test that repeated setting of exit type is handled correctly.
@@ -213,36 +265,29 @@ IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, MAYBE_ExitType) {
 
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, true));
+  {
+    scoped_ptr<Profile> profile(CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
+
+    PrefService* prefs = profile->GetPrefs();
+    // The initial state is crashed; store for later reference.
+    std::string crash_value(prefs->GetString(prefs::kSessionExitType));
+
+    // The first call to a type other than crashed should change the value.
+    profile->SetExitType(Profile::EXIT_SESSION_ENDED);
+    std::string first_call_value(prefs->GetString(prefs::kSessionExitType));
+    EXPECT_NE(crash_value, first_call_value);
+
+    // Subsequent calls to a non-crash value should be ignored.
+    profile->SetExitType(Profile::EXIT_NORMAL);
+    std::string second_call_value(prefs->GetString(prefs::kSessionExitType));
+    EXPECT_EQ(first_call_value, second_call_value);
+
+    // Setting back to a crashed value should work.
+    profile->SetExitType(Profile::EXIT_CRASHED);
+    std::string final_value(prefs->GetString(prefs::kSessionExitType));
+    EXPECT_EQ(crash_value, final_value);
+  }
 
-  scoped_ptr<Profile> profile(Profile::CreateProfile(
-      temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
-  ASSERT_TRUE(profile.get());
-  StartupTaskRunnerServiceFactory::GetForProfile(profile.get())->
-              StartDeferredTaskRunners();
-
-  PrefService* prefs = profile->GetPrefs();
-  // The initial state is crashed; store for later reference.
-  std::string crash_value(prefs->GetString(prefs::kSessionExitType));
-
-  // The first call to a type other than crashed should change the value.
-  profile->SetExitType(Profile::EXIT_SESSION_ENDED);
-  std::string first_call_value(prefs->GetString(prefs::kSessionExitType));
-  EXPECT_NE(crash_value, first_call_value);
-
-  // Subsequent calls to a non-crash value should be ignored.
-  profile->SetExitType(Profile::EXIT_NORMAL);
-  std::string second_call_value(prefs->GetString(prefs::kSessionExitType));
-  EXPECT_EQ(first_call_value, second_call_value);
-
-  // Setting back to a crashed value should work.
-  profile->SetExitType(Profile::EXIT_CRASHED);
-  std::string final_value(prefs->GetString(prefs::kSessionExitType));
-  EXPECT_EQ(crash_value, final_value);
-
-  // This test runs fast enough that the WebDataService may still be
-  // initializing (which uses the temp directory) when the test
-  // ends. Give it a chance to complete.
-  profile.reset();
-  content::RunAllPendingInMessageLoop();
-  content::RunAllPendingInMessageLoop(content::BrowserThread::DB);
+  SpinThreads();
 }