diff options
Diffstat (limited to 'chrome/browser')
20 files changed, 213 insertions, 117 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index d28013b..28d7512 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -131,7 +131,9 @@ using content::ChildProcessSecurityPolicy; using content::PluginService; using content::ResourceDispatcherHost; -BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) +BrowserProcessImpl::BrowserProcessImpl( + base::SequencedTaskRunner* local_state_task_runner, + const CommandLine& command_line) : created_metrics_service_(false), created_watchdog_thread_(false), created_browser_policy_connector_(false), @@ -145,7 +147,8 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) checked_for_new_frames_(false), using_new_frames_(false), render_widget_snapshot_taker_(new RenderWidgetSnapshotTaker), - download_status_updater_(new DownloadStatusUpdater) { + download_status_updater_(new DownloadStatusUpdater), + local_state_task_runner_(local_state_task_runner) { g_browser_process = this; #if defined(ENABLE_PRINTING) @@ -712,10 +715,10 @@ void BrowserProcessImpl::CreateLocalState() { created_local_state_ = true; FilePath local_state_path; - PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path); + CHECK(PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path)); local_state_.reset( - PrefService::CreatePrefService(local_state_path, policy_service(), NULL, - false)); + PrefService::CreatePrefService(local_state_path, local_state_task_runner_, + policy_service(), NULL, false)); // Initialize the prefs of the local state. chrome::RegisterLocalState(local_state_.get()); diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h index b354983..18c00ae 100644 --- a/chrome/browser/browser_process_impl.h +++ b/chrome/browser/browser_process_impl.h @@ -32,6 +32,10 @@ class RemoteDebuggingServer; class PluginsResourceService; #endif +namespace base { +class SequencedTaskRunner; +} + namespace policy { class BrowserPolicyConnector; class PolicyService; @@ -42,7 +46,9 @@ class BrowserProcessImpl : public BrowserProcess, public base::NonThreadSafe, public content::NotificationObserver { public: - explicit BrowserProcessImpl(const CommandLine& command_line); + // |local_state_task_runner| must be a shutdown-blocking task runner. + BrowserProcessImpl(base::SequencedTaskRunner* local_state_task_runner, + const CommandLine& command_line); virtual ~BrowserProcessImpl(); // Called before the browser threads are created. @@ -225,6 +231,9 @@ class BrowserProcessImpl : public BrowserProcess, scoped_refptr<DownloadRequestLimiter> download_request_limiter_; + // Sequenced task runner for local state related I/O tasks. + const scoped_refptr<base::SequencedTaskRunner> local_state_task_runner_; + // Ensures that the observers of plugin/print disable/enable state // notifications are properly added and removed. PrefChangeRegistrar pref_change_registrar_; diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc index 1d98c38..06d96b9 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -274,8 +274,11 @@ void InitializeNetworkOptions(const CommandLine& parsed_command_line, } // Returns the new local state object, guaranteed non-NULL. -PrefService* InitializeLocalState(const CommandLine& parsed_command_line, - bool is_first_run) { +// |local_state_task_runner| must be a shutdown-blocking task runner. +PrefService* InitializeLocalState( + base::SequencedTaskRunner* local_state_task_runner, + const CommandLine& parsed_command_line, + bool is_first_run) { FilePath local_state_path; PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path); bool local_state_file_exists = file_util::PathExists(local_state_path); @@ -329,7 +332,7 @@ PrefService* InitializeLocalState(const CommandLine& parsed_command_line, FilePath parent_profile = parsed_command_line.GetSwitchValuePath(switches::kParentProfile); scoped_ptr<PrefService> parent_local_state( - PrefService::CreatePrefService(parent_profile, + PrefService::CreatePrefService(parent_profile, local_state_task_runner, g_browser_process->policy_service(), NULL, false)); parent_local_state->RegisterStringPref(prefs::kApplicationLocale, @@ -710,7 +713,14 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { parsed_command_line().HasSwitch(switches::kFirstRun)) && !HasImportSwitch(parsed_command_line()); #endif - browser_process_.reset(new BrowserProcessImpl(parsed_command_line())); + + FilePath local_state_path; + CHECK(PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path)); + scoped_refptr<base::SequencedTaskRunner> local_state_task_runner = + JsonPrefStore::GetTaskRunnerForFile(local_state_path, + BrowserThread::GetBlockingPool()); + browser_process_.reset(new BrowserProcessImpl(local_state_task_runner, + parsed_command_line())); if (parsed_command_line().HasSwitch(switches::kEnableProfiling)) { // User wants to override default tracking status. @@ -732,7 +742,9 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { switches::kProfilingOutputFile)); } - local_state_ = InitializeLocalState(parsed_command_line(), is_first_run_); + local_state_ = InitializeLocalState(local_state_task_runner, + parsed_command_line(), + is_first_run_); // These members must be initialized before returning from this function. master_prefs_.reset(new first_run::MasterPrefs); diff --git a/chrome/browser/extensions/extension_prefs_unittest.cc b/chrome/browser/extensions/extension_prefs_unittest.cc index fb8060e..22a8c50 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -52,10 +52,11 @@ static void AddPattern(URLPatternSet* extent, const std::string& pattern) { ExtensionPrefsTest::ExtensionPrefsTest() : ui_thread_(BrowserThread::UI, &message_loop_), - file_thread_(BrowserThread::FILE, &message_loop_) { + prefs_(message_loop_.message_loop_proxy()) { } -ExtensionPrefsTest::~ExtensionPrefsTest() {} +ExtensionPrefsTest::~ExtensionPrefsTest() { +} void ExtensionPrefsTest::RegisterPreferences() {} @@ -71,6 +72,8 @@ void ExtensionPrefsTest::TearDown() { prefs_.RecreateExtensionPrefs(); RegisterPreferences(); Verify(); + prefs_.pref_service()->CommitPendingWrite(); + message_loop_.RunAllPending(); } // Tests the LastPingDay/SetLastPingDay functions. diff --git a/chrome/browser/extensions/extension_prefs_unittest.h b/chrome/browser/extensions/extension_prefs_unittest.h index ba5832a..f96ffb6 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.h +++ b/chrome/browser/extensions/extension_prefs_unittest.h @@ -44,7 +44,6 @@ class ExtensionPrefsTest : public testing::Test { MessageLoop message_loop_; content::TestBrowserThread ui_thread_; - content::TestBrowserThread file_thread_; TestExtensionPrefs prefs_; diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc index 1a68f05..e1e3dc4 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -422,7 +422,8 @@ void ExtensionServiceTestBase::InitializeExtensionService( TestingProfile::Builder profile_builder; // Create a PrefService that only contains user defined preference values. scoped_ptr<PrefService> prefs( - PrefServiceMockBuilder().WithUserFilePrefs(pref_file).Create()); + PrefServiceMockBuilder().WithUserFilePrefs( + pref_file, loop_.message_loop_proxy()).Create()); Profile::RegisterUserPrefs(prefs.get()); chrome::RegisterUserPrefs(prefs.get()); profile_builder.SetPrefService(prefs.Pass()); diff --git a/chrome/browser/extensions/menu_manager_unittest.cc b/chrome/browser/extensions/menu_manager_unittest.cc index ef20359..445a6c4 100644 --- a/chrome/browser/extensions/menu_manager_unittest.cc +++ b/chrome/browser/extensions/menu_manager_unittest.cc @@ -15,6 +15,7 @@ #include "chrome/browser/extensions/event_router.h" #include "chrome/browser/extensions/extension_system_factory.h" #include "chrome/browser/extensions/menu_manager.h" +#include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/extensions/test_extension_prefs.h" #include "chrome/browser/extensions/test_extension_system.h" #include "chrome/common/chrome_notification_types.h" @@ -44,9 +45,15 @@ class MenuManagerTest : public testing::Test { MenuManagerTest() : ui_thread_(BrowserThread::UI, &message_loop_), file_thread_(BrowserThread::FILE, &message_loop_), manager_(&profile_), + prefs_(message_loop_.message_loop_proxy()), next_id_(1) { } + virtual void TearDown() OVERRIDE { + prefs_.pref_service()->CommitPendingWrite(); + message_loop_.RunAllPending(); + } + // Returns a test item. MenuItem* CreateTestItem(Extension* extension, bool incognito = false) { MenuItem::Type type = MenuItem::NORMAL; diff --git a/chrome/browser/extensions/test_extension_prefs.cc b/chrome/browser/extensions/test_extension_prefs.cc index 7c3cf8e..f03df25 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -11,6 +11,8 @@ #include "base/message_loop.h" #include "base/message_loop_proxy.h" #include "base/prefs/json_pref_store.h" +#include "base/run_loop.h" +#include "base/sequenced_task_runner.h" #include "base/synchronization/waitable_event.h" #include "base/values.h" #include "chrome/browser/extensions/extension_pref_store.h" @@ -31,6 +33,8 @@ namespace extensions { namespace { +void DoNothing() {} + // Mock ExtensionPrefs class with artificial clock to guarantee that no two // extensions get the same installation time stamp and we can reliably // assert the installation order in the tests below. @@ -54,9 +58,10 @@ class MockExtensionPrefs : public ExtensionPrefs { } // namespace -TestExtensionPrefs::TestExtensionPrefs() - : pref_service_(NULL), - extensions_disabled_(false) { +TestExtensionPrefs::TestExtensionPrefs( + base::SequencedTaskRunner* task_runner) : pref_service_(NULL), + task_runner_(task_runner), + extensions_disabled_(false) { EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); preferences_file_ = temp_dir_.path().AppendASCII("Preferences"); extensions_dir_ = temp_dir_.path().AppendASCII("Extensions"); @@ -65,36 +70,29 @@ TestExtensionPrefs::TestExtensionPrefs() RecreateExtensionPrefs(); } -TestExtensionPrefs::~TestExtensionPrefs() {} +TestExtensionPrefs::~TestExtensionPrefs() { +} void TestExtensionPrefs::RecreateExtensionPrefs() { // We persist and reload the PrefService's PrefStores because this process // deletes all empty dictionaries. The ExtensionPrefs implementation // needs to be able to handle this situation. if (pref_service_.get()) { - // The PrefService writes its persistent file on the file thread, so we - // need to wait for any pending I/O to complete before creating a new - // PrefService. - base::WaitableEvent io_finished(false, false); - pref_service_-> CommitPendingWrite(); - EXPECT_TRUE(BrowserThread::PostTask( - BrowserThread::FILE, - FROM_HERE, - base::Bind(&base::WaitableEvent::Signal, - base::Unretained(&io_finished)))); - - // If the FILE thread is in fact the current thread (possible in testing - // scenarios), we have to ensure the task has a chance to run. If the FILE - // thread is a different thread, the test must ensure that thread is running - // (otherwise the Wait below will hang). - MessageLoop::current()->RunAllPending(); - - io_finished.Wait(); + // Commit a pending write (which posts a task to task_runner_) and wait for + // it to finish. + pref_service_->CommitPendingWrite(); + base::RunLoop run_loop; + ASSERT_TRUE( + task_runner_->PostTaskAndReply( + FROM_HERE, + base::Bind(&DoNothing), + run_loop.QuitClosure())); + run_loop.Run(); } extension_pref_value_map_.reset(new ExtensionPrefValueMap); PrefServiceMockBuilder builder; - builder.WithUserFilePrefs(preferences_file_); + builder.WithUserFilePrefs(preferences_file_, task_runner_); builder.WithExtensionPrefs( new ExtensionPrefStore(extension_pref_value_map_.get(), false)); pref_service_.reset(builder.Create()); diff --git a/chrome/browser/extensions/test_extension_prefs.h b/chrome/browser/extensions/test_extension_prefs.h index 5fb1ec5..b2b1faa 100644 --- a/chrome/browser/extensions/test_extension_prefs.h +++ b/chrome/browser/extensions/test_extension_prefs.h @@ -16,6 +16,7 @@ class PrefService; namespace base { class DictionaryValue; +class SequencedTaskRunner; } namespace extensions { @@ -25,7 +26,7 @@ class ExtensionPrefs; // in tests. class TestExtensionPrefs { public: - TestExtensionPrefs(); + explicit TestExtensionPrefs(base::SequencedTaskRunner* task_runner); virtual ~TestExtensionPrefs(); ExtensionPrefs* prefs() { return prefs_.get(); } @@ -76,6 +77,7 @@ class TestExtensionPrefs { scoped_ptr<PrefService> pref_service_; scoped_ptr<ExtensionPrefs> prefs_; scoped_ptr<ExtensionPrefValueMap> extension_pref_value_map_; + const scoped_refptr<base::SequencedTaskRunner> task_runner_; private: bool extensions_disabled_; diff --git a/chrome/browser/extensions/updater/extension_updater_unittest.cc b/chrome/browser/extensions/updater/extension_updater_unittest.cc index 5bf0548..8f0c464 100644 --- a/chrome/browser/extensions/updater/extension_updater_unittest.cc +++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc @@ -162,10 +162,12 @@ class NotificationsObserver : public content::NotificationObserver { // Base class for further specialized test classes. class MockService : public TestExtensionService { public: - MockService() - : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) { + explicit MockService(TestExtensionPrefs* prefs) + : prefs_(prefs), + pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) { profile_.CreateRequestContext(); } + virtual ~MockService() {} virtual PendingExtensionManager* pending_extension_manager() OVERRIDE { @@ -180,9 +182,9 @@ class MockService : public TestExtensionService { return profile_.GetRequestContext(); } - ExtensionPrefs* extension_prefs() { return prefs_.prefs(); } + ExtensionPrefs* extension_prefs() { return prefs_->prefs(); } - PrefService* pref_service() { return prefs_.pref_service(); } + PrefService* pref_service() { return prefs_->pref_service(); } // Creates test extensions and inserts them into list. The name and // version are all based on their index. If |update_url| is non-null, it @@ -201,15 +203,15 @@ class MockService : public TestExtensionService { if (update_url) manifest.SetString(extension_manifest_keys::kUpdateURL, *update_url); scoped_refptr<Extension> e = - prefs_.AddExtensionWithManifest(manifest, location); + prefs_->AddExtensionWithManifest(manifest, location); ASSERT_TRUE(e != NULL); list->push_back(e); } } protected: + TestExtensionPrefs* const prefs_; PendingExtensionManager pending_extension_manager_; - TestExtensionPrefs prefs_; TestingProfile profile_; private: @@ -260,7 +262,9 @@ void SetupPendingExtensionManagerForTest( class ServiceForManifestTests : public MockService { public: - ServiceForManifestTests() {} + explicit ServiceForManifestTests(TestExtensionPrefs* prefs) + : MockService(prefs) { + } virtual ~ServiceForManifestTests() {} @@ -305,8 +309,8 @@ class ServiceForManifestTests : public MockService { class ServiceForDownloadTests : public MockService { public: - ServiceForDownloadTests() - : MockService() { + explicit ServiceForDownloadTests(TestExtensionPrefs* prefs) + : MockService(prefs) { } // Add a fake crx installer to be returned by a call to UpdateExtension() @@ -369,8 +373,8 @@ class ServiceForDownloadTests : public MockService { class ServiceForBlacklistTests : public MockService { public: - ServiceForBlacklistTests() - : MockService(), + explicit ServiceForBlacklistTests(TestExtensionPrefs* prefs) + : MockService(prefs), processed_blacklist_(false) { } virtual void UpdateExtensionBlacklist( @@ -417,16 +421,26 @@ class ExtensionUpdaterTest : public testing::Test { ExtensionUpdaterTest() : ui_thread_(BrowserThread::UI, &loop_), file_thread_(BrowserThread::FILE, &loop_), - io_thread_(BrowserThread::IO, &loop_) {} + io_thread_(BrowserThread::IO, &loop_) { + } + + virtual ~ExtensionUpdaterTest() { + } + + virtual void SetUp() OVERRIDE { + prefs_.reset(new TestExtensionPrefs(loop_.message_loop_proxy())); + } virtual void TearDown() OVERRIDE { // Some tests create URLRequestContextGetters, whose destruction must run // on the IO thread. Make sure the IO loop spins before shutdown so that // those objects are released. - loop_.RunAllPending(); + RunAllPending(); + prefs_.reset(); } void RunAllPending() { + prefs_->pref_service()->CommitPendingWrite(); loop_.RunAllPending(); } @@ -466,7 +480,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestExtensionUpdateCheckRequests(bool pending) { // Create an extension with an update_url. - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); std::string update_url("http://foo.com/bar"); ExtensionList extensions; PendingExtensionManager* pending_extension_manager = @@ -526,7 +540,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestBlacklistUpdateCheckRequests() { // Setup and start the updater. - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); net::TestURLFetcherFactory factory; ExtensionUpdater updater( @@ -608,7 +622,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestUpdateUrlDataFromGallery(const std::string& gallery_url) { net::TestURLFetcherFactory factory; - MockService service; + MockService service(prefs_.get()); MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); ExtensionList extensions; @@ -689,7 +703,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestDetermineUpdatesPending() { // Create a set of test extensions - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); PendingExtensionManager* pending_extension_manager = service.pending_extension_manager(); SetupPendingExtensionManagerForTest(3, GURL(), pending_extension_manager); @@ -731,7 +745,7 @@ class ExtensionUpdaterTest : public testing::Test { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; NotificationsObserver observer; - MockService service; + MockService service(prefs_.get()); MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); @@ -843,7 +857,8 @@ class ExtensionUpdaterTest : public testing::Test { void TestSingleExtensionDownloading(bool pending) { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - scoped_ptr<ServiceForDownloadTests> service(new ServiceForDownloadTests); + scoped_ptr<ServiceForDownloadTests> service( + new ServiceForDownloadTests(prefs_.get())); ExtensionUpdater updater(service.get(), service->extension_prefs(), service->pref_service(), service->profile(), @@ -899,7 +914,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestBlacklistDownloading() { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - ServiceForBlacklistTests service; + ServiceForBlacklistTests service(prefs_.get()); ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -947,7 +962,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestMultipleExtensionDownloading(bool updates_start_running) { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - ServiceForDownloadTests service; + ServiceForDownloadTests service(prefs_.get()); ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1116,7 +1131,8 @@ class ExtensionUpdaterTest : public testing::Test { // Set up 2 mock extensions, one with a google.com update url and one // without. - ServiceForManifestTests service; + prefs_.reset(new TestExtensionPrefs(loop_.message_loop_proxy())); + ServiceForManifestTests service(prefs_.get()); ExtensionList tmp; GURL url1("http://clients2.google.com/service/update2/crx"); GURL url2("http://www.somewebsite.com"); @@ -1228,6 +1244,8 @@ class ExtensionUpdaterTest : public testing::Test { // queries. EXPECT_TRUE(url1_query.find(brand_string) == std::string::npos); #endif + + RunAllPending(); } // This makes sure that the extension updater properly stores the results @@ -1235,7 +1253,7 @@ class ExtensionUpdaterTest : public testing::Test { // the first time we fetched the extension, or 2) We sent a ping value of // >= 1 day for the extension. void TestHandleManifestResults() { - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); GURL update_url("http://www.google.com/manifest"); ExtensionList tmp; service.CreateTestExtensions(1, 1, &tmp, &update_url.spec(), @@ -1265,6 +1283,9 @@ class ExtensionUpdaterTest : public testing::Test { EXPECT_LT(seconds_diff - results.daystart_elapsed_seconds, 5); } + protected: + scoped_ptr<TestExtensionPrefs> prefs_; + private: MessageLoop loop_; content::TestBrowserThread ui_thread_; @@ -1345,7 +1366,7 @@ TEST_F(ExtensionUpdaterTest, TestHandleManifestResults) { TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) { net::TestURLFetcherFactory factory; - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); ExtensionUpdater updater(&service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1376,7 +1397,7 @@ TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) { TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) { net::TestURLFetcherFactory factory; - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); ExtensionUpdater updater(&service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1414,7 +1435,7 @@ TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) { TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { net::TestURLFetcherFactory factory; - MockService service; + MockService service(prefs_.get()); MockExtensionDownloaderDelegate delegate; scoped_ptr<ExtensionDownloader> downloader( new ExtensionDownloader(&delegate, service.request_context())); @@ -1466,7 +1487,7 @@ TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { TEST_F(ExtensionUpdaterTest, TestStartUpdateCheckMemory) { net::TestURLFetcherFactory factory; - MockService service; + MockService service(prefs_.get()); MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); @@ -1480,7 +1501,7 @@ TEST_F(ExtensionUpdaterTest, TestStartUpdateCheckMemory) { } TEST_F(ExtensionUpdaterTest, TestCheckSoon) { - ServiceForManifestTests service; + ServiceForManifestTests service(prefs_.get()); net::TestURLFetcherFactory factory; ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc index 58f398d..680e4a5 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/chrome/browser/metrics/metrics_service.cc @@ -1723,27 +1723,11 @@ bool MetricsService::UmaMetricsProperlyShutdown() { return clean_shutdown_status_ == CLEANLY_SHUTDOWN; } -// For use in hack in LogCleanShutdown. -static void Signal(base::WaitableEvent* event) { - event->Signal(); -} - void MetricsService::LogCleanShutdown() { // Redundant hack to write pref ASAP. PrefService* pref = g_browser_process->local_state(); pref->SetBoolean(prefs::kStabilityExitedCleanly, true); pref->CommitPendingWrite(); - // Hack: TBD: Remove this wait. - // We are so concerned that the pref gets written, we are now willing to stall - // the UI thread until we get assurance that a pref-writing task has - // completed. - base::WaitableEvent done_writing(false, false); - BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, - base::Bind(Signal, &done_writing)); - // http://crbug.com/124954 - base::ThreadRestrictions::ScopedAllowWait allow_wait; - done_writing.TimedWait(base::TimeDelta::FromHours(1)); - // Redundant setting to assure that we always reset this value at shutdown // (and that we don't use some alternate path, and not call LogCleanShutdown). clean_shutdown_status_ = CLEANLY_SHUTDOWN; diff --git a/chrome/browser/policy/managed_mode_policy_provider.cc b/chrome/browser/policy/managed_mode_policy_provider.cc index 7fecdb1..c55f87f 100644 --- a/chrome/browser/policy/managed_mode_policy_provider.cc +++ b/chrome/browser/policy/managed_mode_policy_provider.cc @@ -5,6 +5,7 @@ #include "chrome/browser/policy/managed_mode_policy_provider.h" #include "base/prefs/json_pref_store.h" +#include "base/threading/sequenced_worker_pool.h" #include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_constants.h" @@ -19,11 +20,11 @@ const char ManagedModePolicyProvider::kPolicies[] = "policies"; // static ManagedModePolicyProvider* ManagedModePolicyProvider::Create(Profile* profile) { - JsonPrefStore* pref_store = - new JsonPrefStore(profile->GetPath().Append( - chrome::kManagedModePolicyFilename), - BrowserThread::GetMessageLoopProxyForThread( - BrowserThread::FILE)); + FilePath path = profile->GetPath().Append(chrome::kManagedModePolicyFilename); + JsonPrefStore* pref_store = new JsonPrefStore( + path, + JsonPrefStore::GetTaskRunnerForFile(path, + BrowserThread::GetBlockingPool())); return new ManagedModePolicyProvider(pref_store); } diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index 8862f7f..eedef9b 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -123,6 +123,7 @@ PrefServiceBase* PrefServiceBase::FromBrowserContext(BrowserContext* context) { // static PrefService* PrefService::CreatePrefService( const FilePath& pref_filename, + base::SequencedTaskRunner* pref_io_task_runner, policy::PolicyService* policy_service, PrefStore* extension_prefs, bool async) { @@ -156,8 +157,7 @@ PrefService* PrefService::CreatePrefService( CommandLinePrefStore* command_line = new CommandLinePrefStore(CommandLine::ForCurrentProcess()); JsonPrefStore* user = new JsonPrefStore( - pref_filename, - BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)); + pref_filename, pref_io_task_runner); DefaultPrefStore* default_pref_store = new DefaultPrefStore(); PrefNotifierImpl* pref_notifier = new PrefNotifierImpl(); diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h index 82b9167..c855ef1 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -31,6 +31,10 @@ class PrefServiceObserver; class PrefStore; class PrefValueStore; +namespace base { + class SequencedTaskRunner; +} + namespace syncer { class SyncableService; } @@ -108,10 +112,12 @@ class PrefService : public PrefServiceBase, public base::NonThreadSafe { // the created PrefService or NULL if creation has failed. Note, it is // guaranteed that in asynchronous version initialization happens after this // function returned. - static PrefService* CreatePrefService(const FilePath& pref_filename, - policy::PolicyService* policy_service, - PrefStore* extension_pref_store, - bool async); + static PrefService* CreatePrefService( + const FilePath& pref_filename, + base::SequencedTaskRunner* pref_io_task_runner, + policy::PolicyService* policy_service, + PrefStore* extension_pref_store, + bool async); // Creates an incognito copy of the pref service that shares most pref stores // but uses a fresh non-persistent overlay for the user pref store and an diff --git a/chrome/browser/prefs/pref_service_mock_builder.cc b/chrome/browser/prefs/pref_service_mock_builder.cc index 2782882..77d8080 100644 --- a/chrome/browser/prefs/pref_service_mock_builder.cc +++ b/chrome/browser/prefs/pref_service_mock_builder.cc @@ -77,16 +77,17 @@ PrefServiceMockBuilder::WithCommandLine(CommandLine* command_line) { PrefServiceMockBuilder& PrefServiceMockBuilder::WithUserFilePrefs(const FilePath& prefs_file) { - return WithUserFilePrefs(prefs_file, - BrowserThread::GetMessageLoopProxyForThread( - BrowserThread::FILE)); + return WithUserFilePrefs( + prefs_file, + JsonPrefStore::GetTaskRunnerForFile(prefs_file, + BrowserThread::GetBlockingPool())); } PrefServiceMockBuilder& PrefServiceMockBuilder::WithUserFilePrefs( const FilePath& prefs_file, - base::MessageLoopProxy* message_loop_proxy) { - user_prefs_ = new JsonPrefStore(prefs_file, message_loop_proxy); + base::SequencedTaskRunner* task_runner) { + user_prefs_ = new JsonPrefStore(prefs_file, task_runner); return *this; } diff --git a/chrome/browser/prefs/pref_service_mock_builder.h b/chrome/browser/prefs/pref_service_mock_builder.h index b939ca6..708b622 100644 --- a/chrome/browser/prefs/pref_service_mock_builder.h +++ b/chrome/browser/prefs/pref_service_mock_builder.h @@ -15,7 +15,7 @@ class FilePath; class PrefService; namespace base { -class MessageLoopProxy; +class SequencedTaskRunner; } namespace policy { @@ -48,10 +48,11 @@ class PrefServiceMockBuilder { PrefServiceMockBuilder& WithCommandLine(CommandLine* command_line); // Specifies to use an actual file-backed user pref store. + // TODO(zelidrag): Remove the first overloaded method below. PrefServiceMockBuilder& WithUserFilePrefs(const FilePath& prefs_file); PrefServiceMockBuilder& WithUserFilePrefs( const FilePath& prefs_file, - base::MessageLoopProxy* message_loop_proxy); + base::SequencedTaskRunner* task_runner); // Creates the PrefService, invalidating the entire builder configuration. PrefService* Create(); diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc index 2dc90a8..cd046e8 100644 --- a/chrome/browser/prefs/pref_service_unittest.cc +++ b/chrome/browser/prefs/pref_service_unittest.cc @@ -315,7 +315,7 @@ TEST_F(PrefServiceUserFilePrefsTest, PreserveEmptyValue) { pref_file)); PrefServiceMockBuilder builder; - builder.WithUserFilePrefs(pref_file, base::MessageLoopProxy::current()); + builder.WithUserFilePrefs(pref_file, message_loop_.message_loop_proxy()); scoped_ptr<PrefService> prefs(builder.Create()); // Register testing prefs. @@ -344,7 +344,7 @@ TEST_F(PrefServiceUserFilePrefsTest, PreserveEmptyValue) { // Write to file. prefs->CommitPendingWrite(); - MessageLoop::current()->RunAllPending(); + message_loop_.RunAllPending(); // Compare to expected output. FilePath golden_output_file = diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc index 0766394..1d917aa 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -12,10 +12,13 @@ #include "base/file_util.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" +#include "base/prefs/json_pref_store.h" #include "base/string_number_conversions.h" #include "base/string_tokenizer.h" #include "base/string_util.h" #include "base/stringprintf.h" +#include "base/synchronization/waitable_event.h" +#include "base/threading/sequenced_worker_pool.h" #include "base/utf_string_conversions.h" #include "base/version.h" #include "chrome/browser/autocomplete/autocomplete_classifier.h" @@ -148,6 +151,38 @@ static const char kReadmeText[] = const char* const kPrefExitTypeCrashed = "Crashed"; const char* const kPrefExitTypeSessionEnded = "SessionEnded"; +// Helper method needed because PostTask cannot currently take a Callback +// function with non-void return type. +// TODO(jhawkins): Remove once IgnoreResult is fixed. +void CreateDirectoryAndSignal(const FilePath& path, + base::WaitableEvent* done_creating) { + file_util::CreateDirectory(path); + done_creating->Signal(); +} + +// Task that blocks the FILE thread until CreateDirectoryAndSignal() finishes on +// blocking I/O pool. +void BlockFileThreadOnDirectoryCreate(base::WaitableEvent* done_creating) { + done_creating->Wait(); +} + +// Initiates creation of profile directory on |sequenced_task_runner| and +// ensures that FILE thread is blocked until that operation finishes. +void CreateProfileDirectory(base::SequencedTaskRunner* sequenced_task_runner, + const FilePath& path) { + base::WaitableEvent* done_creating = new base::WaitableEvent(false, false); + sequenced_task_runner->PostTask(FROM_HERE, + base::Bind(&CreateDirectoryAndSignal, + path, + done_creating)); + // Block the FILE thread until directory is created on I/O pool to make sure + // that we don't attempt any operation until that part completes. + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + base::Bind(&BlockFileThreadOnDirectoryCreate, + base::Owned(done_creating))); +} + FilePath GetCachePath(const FilePath& base) { return base.Append(chrome::kCacheDirname); } @@ -198,12 +233,15 @@ std::string ExitTypeToSessionTypePrefValue(Profile::ExitType type) { Profile* Profile::CreateProfile(const FilePath& path, Delegate* delegate, CreateMode create_mode) { + // Get sequenced task runner for making sure that file operations of + // this profile (defined by |path|) are executed in expected order + // (what was previously assured by the FILE thread). + scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner = + JsonPrefStore::GetTaskRunnerForFile(path, + BrowserThread::GetBlockingPool()); if (create_mode == CREATE_MODE_ASYNCHRONOUS) { DCHECK(delegate); - // This is safe while all file operations are done on the FILE thread. - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - base::Bind(base::IgnoreResult(&file_util::CreateDirectory), path)); + CreateProfileDirectory(sequenced_task_runner, path); } else if (create_mode == CREATE_MODE_SYNCHRONOUS) { if (!file_util::PathExists(path)) { // TODO(tc): http://b/1094718 Bad things happen if we can't write to the @@ -216,7 +254,7 @@ Profile* Profile::CreateProfile(const FilePath& path, NOTREACHED(); } - return new ProfileImpl(path, delegate, create_mode); + return new ProfileImpl(path, delegate, create_mode, sequenced_task_runner); } // static @@ -269,9 +307,11 @@ void ProfileImpl::RegisterUserPrefs(PrefService* prefs) { PrefService::SYNCABLE_PREF); } -ProfileImpl::ProfileImpl(const FilePath& path, - Delegate* delegate, - CreateMode create_mode) +ProfileImpl::ProfileImpl( + const FilePath& path, + Delegate* delegate, + CreateMode create_mode, + base::SequencedTaskRunner* sequenced_task_runner) : path_(path), ALLOW_THIS_IN_INITIALIZER_LIST(io_data_(this)), host_content_settings_map_(NULL), @@ -316,6 +356,7 @@ ProfileImpl::ProfileImpl(const FilePath& path, if (create_mode == CREATE_MODE_ASYNCHRONOUS) { prefs_.reset(PrefService::CreatePrefService( GetPrefFilePath(), + sequenced_task_runner, policy_service_.get(), new ExtensionPrefStore( ExtensionPrefValueMapFactory::GetForProfile(this), false), @@ -328,6 +369,7 @@ ProfileImpl::ProfileImpl(const FilePath& path, // Load prefs synchronously. prefs_.reset(PrefService::CreatePrefService( GetPrefFilePath(), + sequenced_task_runner, policy_service_.get(), new ExtensionPrefStore( ExtensionPrefValueMapFactory::GetForProfile(this), false), @@ -351,10 +393,10 @@ void ProfileImpl::DoFinalInit(bool is_new_profile) { // to PathService. chrome::GetUserCacheDirectory(path_, &base_cache_path_); // Always create the cache directory asynchronously. - BrowserThread::PostTask( - BrowserThread::FILE, FROM_HERE, - base::Bind(base::IgnoreResult(&file_util::CreateDirectory), - base_cache_path_)); + scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner = + JsonPrefStore::GetTaskRunnerForFile(base_cache_path_, + BrowserThread::GetBlockingPool()); + CreateProfileDirectory(sequenced_task_runner, base_cache_path_); // Now that the profile is hooked up to receive pref change notifications to // kGoogleServicesUsername, initialize components that depend on it to reflect diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h index 533b68b..ad7c17f 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h @@ -33,6 +33,10 @@ class Preferences; } #endif +namespace base { +class SequencedTaskRunner; +} + namespace content { class SpeechRecognitionPreferences; } @@ -141,7 +145,8 @@ class ProfileImpl : public Profile, ProfileImpl(const FilePath& path, Delegate* delegate, - CreateMode create_mode); + CreateMode create_mode, + base::SequencedTaskRunner* sequenced_task_runner); // Does final initialization. Should be called after prefs were loaded. void DoFinalInit(bool is_new_profile); diff --git a/chrome/browser/sync/credential_cache_service_win.cc b/chrome/browser/sync/credential_cache_service_win.cc index 23ac826..9c938d0 100644 --- a/chrome/browser/sync/credential_cache_service_win.cc +++ b/chrome/browser/sync/credential_cache_service_win.cc @@ -693,8 +693,9 @@ void CredentialCacheService::LookForCachedCredentialsInAlternateProfile() { // Attempt to read cached credentials from the alternate profile. If no file // exists, ReadPrefsAsync() will cause PREF_READ_ERROR_NO_FILE to be returned // after initialization is complete. + FilePath path = GetCredentialPathInAlternateProfile(); alternate_store_ = new JsonPrefStore( - GetCredentialPathInAlternateProfile(), + path, content::BrowserThread::GetMessageLoopProxyForThread( content::BrowserThread::FILE)); alternate_store_observer_ = new AlternateStoreObserver(this, |