Try to fix ProfileBrowserTest.

chrome/browser/profiles/profile_browsertest.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/profiles/profile.h"
 
 #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"
 #include "chrome/browser/profiles/profile_impl.h"
-#include "chrome/browser/profiles/startup_task_runner_service.h"
-#include "chrome/browser/profiles/startup_task_runner_service_factory.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
@@ skipping 17 matching lines @@
     created_by_version = version_info.Version();
   } else {
     created_by_version = "1.0.0.0";
   }
   std::string pref_version =
       ChromeVersionService::GetVersion(profile->GetPrefs());
   // Assert that created_by_version pref gets set to current version.
   EXPECT_EQ(created_by_version, pref_version);
 }
 
+void BlockThread(
+    base::WaitableEvent* is_blocked,
+    base::WaitableEvent* unblock) {
+  is_blocked->Signal();
+  unblock->Wait();
+}
+
 }  // namespace
 
 typedef InProcessBrowserTest ProfileBrowserTest;
 
 // Test OnProfileCreate is called with is_new_profile set to true when
 // creating a new profile synchronously.
 //
 // Flaky (sometimes timeout): http://crbug.com/141141
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
                        DISABLED_CreateNewProfileSynchronous) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   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(Profile::CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
+    ASSERT_TRUE(profile.get());
+    CheckChromeVersion(profile.get(), true);
+  }
+
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);

[rpetterson, 2014/04/09 20:05:11]

Can you use a ThreadBundle and eliminate all this special casing?

[Marc Treib, 2014/04/10 09:25:09]

You mean a TestBrowserThreadBundle? As I understand it, that's meant only for
unit tests, not browser tests? Here, all the threads already exist.
However, I could replicate the full shutdown behavior from
TestBrowserThreadBundle.

[rpetterson, 2014/04/16 17:50:44]

Ah, I didn't realize that that was just for browser tests. No worries. Although
you could group them into a helper function to cut down on repeated code. Same
with the profile creation. But I'm fine doing that in a cleanup CL.

 }
 
 // Test OnProfileCreate is called with is_new_profile set to false when
 // creating a profile synchronously with an existing prefs file.
 // Flaky: http://crbug.com/141517
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
                        DISABLED_CreateOldProfileSynchronous) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   CreatePrefsFileInDirectory(temp_dir.path());
 
   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(Profile::CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_SYNCHRONOUS));
+    ASSERT_TRUE(profile.get());
+    CheckChromeVersion(profile.get(), false);
+  }
+
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);
 }
 
 // Test OnProfileCreate is called with is_new_profile set to true when
 // creating a new profile asynchronously.
 // This test is flaky on Linux, Win and Mac.  See crbug.com/142787
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
                        DISABLED_CreateNewProfileAsynchronous) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   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();
+  {
+    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();
-  CheckChromeVersion(profile.get(), true);
+    scoped_ptr<Profile> profile(Profile::CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
+    ASSERT_TRUE(profile.get());
+
+    // Wait for the profile to be created.
+    observer.Wait();
+    CheckChromeVersion(profile.get(), true);
+  }
+
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);
 }
 
 // Test OnProfileCreate is called with is_new_profile set to false when
 // creating a profile asynchronously with an existing prefs file.
 // Flaky: http://crbug.com/141517
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest,
                        DISABLED_CreateOldProfileAsynchronous) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
   CreatePrefsFileInDirectory(temp_dir.path());
 
   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(Profile::CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
+    ASSERT_TRUE(profile.get());
+
+    // Wait for the profile to be created.
+    observer.Wait();
+    CheckChromeVersion(profile.get(), false);
+  }
+
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);
 }
 
 // Test that a README file is created for profiles that didn't have it.
 // Flaky: http://crbug.com/140882
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, DISABLED_ProfileReadmeCreated) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, true));
 
   // 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(Profile::CreateProfile(
+        temp_dir.path(), &delegate, Profile::CREATE_MODE_ASYNCHRONOUS));
+    ASSERT_TRUE(profile.get());
 
+    // Wait for the profile to be created.
+    observer.Wait();
+
+    // 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)));
+  }
+
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);
-
-  // Verify that README exists.
-  EXPECT_TRUE(base::PathExists(
-      temp_dir.path().Append(chrome::kReadmeFilename)));
 }
 
 // Test that Profile can be deleted before README file is created.
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, ProfileDeletedBeforeReadmeCreated) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   MockProfileDelegate delegate;
   EXPECT_CALL(delegate, OnProfileCreated(testing::NotNull(), true, true));
 
   // No delay before README creation.
   ProfileImpl::create_readme_delay_ms = 0;
 
+  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(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();
+
+  // Now unblock the file thread again and run pending tasks (this includes the
+  // task for README creation).
+  unblock->Signal();
   content::RunAllPendingInMessageLoop();
   content::RunAllPendingInMessageLoop(content::BrowserThread::DB);
   content::RunAllPendingInMessageLoop(content::BrowserThread::FILE);
 }
 
 // Test that repeated setting of exit type is handled correctly.
 #if defined(OS_WIN)
 // Flaky on Windows: http://crbug.com/163713
 #define MAYBE_ExitType DISABLED_ExitType
 #else
 #define MAYBE_ExitType ExitType
 #endif
 IN_PROC_BROWSER_TEST_F(ProfileBrowserTest, MAYBE_ExitType) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
   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());
 
-  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));
 
-  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);
 
-  // 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);
 
-  // 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);
+  }
 
-  // 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();
+  // Give threads a chance to do their stuff before deleting scoped temp dir.
+  // 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);
 }