diff options
33 files changed, 188 insertions, 330 deletions
diff --git a/base/files/important_file_writer.cc b/base/files/important_file_writer.cc index 536b0fb..5663bdd 100644 --- a/base/files/important_file_writer.cc +++ b/base/files/important_file_writer.cc @@ -13,7 +13,7 @@ #include "base/file_path.h" #include "base/file_util.h" #include "base/logging.h" -#include "base/task_runner.h" +#include "base/message_loop_proxy.h" #include "base/metrics/histogram.h" #include "base/string_number_conversions.h" #include "base/threading/thread.h" @@ -90,14 +90,14 @@ void WriteToDiskTask(const FilePath& path, const std::string& data) { } // namespace ImportantFileWriter::ImportantFileWriter( - const FilePath& path, base::SequencedTaskRunner* task_runner) + const FilePath& path, MessageLoopProxy* file_message_loop_proxy) : path_(path), - task_runner_(task_runner), + file_message_loop_proxy_(file_message_loop_proxy), serializer_(NULL), commit_interval_(TimeDelta::FromMilliseconds( kDefaultCommitIntervalMs)) { DCHECK(CalledOnValidThread()); - DCHECK(task_runner_.get()); + DCHECK(file_message_loop_proxy_.get()); } ImportantFileWriter::~ImportantFileWriter() { @@ -122,8 +122,8 @@ void ImportantFileWriter::WriteNow(const std::string& data) { if (HasPendingWrite()) timer_.Stop(); - if (!task_runner_->PostTask(FROM_HERE, - MakeCriticalClosure(Bind(&WriteToDiskTask, path_, data)))) { + if (!file_message_loop_proxy_->PostTask( + FROM_HERE, MakeCriticalClosure(Bind(&WriteToDiskTask, path_, data)))) { // Posting the task to background message loop is not expected // to fail, but if it does, avoid losing data and just hit the disk // on the current thread. diff --git a/base/files/important_file_writer.h b/base/files/important_file_writer.h index ae814d5..290813c 100644 --- a/base/files/important_file_writer.h +++ b/base/files/important_file_writer.h @@ -17,7 +17,7 @@ namespace base { -class SequencedTaskRunner; +class MessageLoopProxy; class Thread; // Helper to ensure that a file won't be corrupted by the write (for example on @@ -53,11 +53,11 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { // Initialize the writer. // |path| is the name of file to write. - // |task_runner| is the SequencedTaskRunner instance where on which we will - // execute file I/O operations. + // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which + // file I/O can be done. // All non-const methods, ctor and dtor must be called on the same thread. ImportantFileWriter(const FilePath& path, - base::SequencedTaskRunner* task_runner); + MessageLoopProxy* file_message_loop_proxy); // You have to ensure that there are no pending writes at the moment // of destruction. @@ -96,8 +96,8 @@ class BASE_EXPORT ImportantFileWriter : public NonThreadSafe { // Path being written to. const FilePath path_; - // TaskRunner for the thread on which file I/O can be done. - const scoped_refptr<base::SequencedTaskRunner> task_runner_; + // MessageLoopProxy for the thread on which file I/O can be done. + scoped_refptr<MessageLoopProxy> file_message_loop_proxy_; // Timer used to schedule commit after ScheduleWrite. OneShotTimer<ImportantFileWriter> timer_; diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc index 98810d5..18dc0b7 100644 --- a/base/prefs/json_pref_store.cc +++ b/base/prefs/json_pref_store.cc @@ -13,8 +13,6 @@ #include "base/json/json_string_value_serializer.h" #include "base/memory/ref_counted.h" #include "base/message_loop_proxy.h" -#include "base/sequenced_task_runner.h" -#include "base/threading/sequenced_worker_pool.h" #include "base/values.h" namespace { @@ -28,25 +26,25 @@ class FileThreadDeserializer : public base::RefCountedThreadSafe<FileThreadDeserializer> { public: FileThreadDeserializer(JsonPrefStore* delegate, - base::SequencedTaskRunner* sequenced_task_runner) + base::MessageLoopProxy* file_loop_proxy) : no_dir_(false), error_(PersistentPrefStore::PREF_READ_ERROR_NONE), delegate_(delegate), - sequenced_task_runner_(sequenced_task_runner), + file_loop_proxy_(file_loop_proxy), origin_loop_proxy_(base::MessageLoopProxy::current()) { } void Start(const FilePath& path) { DCHECK(origin_loop_proxy_->BelongsToCurrentThread()); - sequenced_task_runner_->PostTask( + file_loop_proxy_->PostTask( FROM_HERE, base::Bind(&FileThreadDeserializer::ReadFileAndReport, this, path)); } - // Deserializes JSON on the sequenced task runner. + // Deserializes JSON on the file thread. void ReadFileAndReport(const FilePath& path) { - DCHECK(sequenced_task_runner_->RunsTasksOnCurrentThread()); + DCHECK(file_loop_proxy_->BelongsToCurrentThread()); value_.reset(DoReading(path, &error_, &no_dir_)); @@ -86,9 +84,9 @@ class FileThreadDeserializer bool no_dir_; PersistentPrefStore::PrefReadError error_; scoped_ptr<Value> value_; - const scoped_refptr<JsonPrefStore> delegate_; - const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_; - const scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_; + scoped_refptr<JsonPrefStore> delegate_; + scoped_refptr<base::MessageLoopProxy> file_loop_proxy_; + scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_; }; // static @@ -139,23 +137,13 @@ void FileThreadDeserializer::HandleErrors( } // namespace -scoped_refptr<base::SequencedTaskRunner> JsonPrefStore::GetTaskRunnerForFile( - const FilePath& filename, - base::SequencedWorkerPool* worker_pool) { - std::string token("json_pref_store-"); - token.append(filename.AsUTF8Unsafe()); - return worker_pool->GetSequencedTaskRunnerWithShutdownBehavior( - worker_pool->GetNamedSequenceToken(token), - base::SequencedWorkerPool::BLOCK_SHUTDOWN); -} - JsonPrefStore::JsonPrefStore(const FilePath& filename, - base::SequencedTaskRunner* sequenced_task_runner) + base::MessageLoopProxy* file_message_loop_proxy) : path_(filename), - sequenced_task_runner_(sequenced_task_runner), + file_message_loop_proxy_(file_message_loop_proxy), prefs_(new DictionaryValue()), read_only_(false), - writer_(filename, sequenced_task_runner), + writer_(filename, file_message_loop_proxy), error_delegate_(NULL), initialized_(false), read_error_(PREF_READ_ERROR_OTHER) { @@ -257,7 +245,7 @@ void JsonPrefStore::ReadPrefsAsync(ReadErrorDelegate *error_delegate) { // Start async reading of the preferences file. It will delete itself // in the end. scoped_refptr<FileThreadDeserializer> deserializer( - new FileThreadDeserializer(this, sequenced_task_runner_.get())); + new FileThreadDeserializer(this, file_message_loop_proxy_.get())); deserializer->Start(path_); } diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h index 7a75d4c..5ae1ca0 100644 --- a/base/prefs/json_pref_store.h +++ b/base/prefs/json_pref_store.h @@ -20,8 +20,7 @@ namespace base { class DictionaryValue; -class SequencedWorkerPool; -class SequencedTaskRunner; +class MessageLoopProxy; class Value; } @@ -32,16 +31,10 @@ class BASE_PREFS_EXPORT JsonPrefStore : public PersistentPrefStore, public base::ImportantFileWriter::DataSerializer { public: - // Returns instance of SequencedTaskRunner which guarantees that file - // operations on the same file will be executed in sequenced order. - static scoped_refptr<base::SequencedTaskRunner> GetTaskRunnerForFile( - const FilePath& pref_filename, - base::SequencedWorkerPool* worker_pool); - - // |sequenced_task_runner| is must be a shutdown-blocking task runner, ideally - // created by GetTaskRunnerForFile() method above. + // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which + // file I/O can be done. JsonPrefStore(const FilePath& pref_filename, - base::SequencedTaskRunner* sequenced_task_runner); + base::MessageLoopProxy* file_message_loop_proxy); // PrefStore overrides: virtual ReadResult GetValue(const std::string& key, @@ -79,7 +72,7 @@ class BASE_PREFS_EXPORT JsonPrefStore virtual bool SerializeData(std::string* output) OVERRIDE; FilePath path_; - const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_; + scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy_; scoped_ptr<base::DictionaryValue> prefs_; diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc index e7239bc..7661acd 100644 --- a/base/prefs/json_pref_store_unittest.cc +++ b/base/prefs/json_pref_store_unittest.cc @@ -5,12 +5,13 @@ #include "base/file_util.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" +#include "base/message_loop_proxy.h" #include "base/path_service.h" #include "base/prefs/json_pref_store.h" #include "base/scoped_temp_dir.h" #include "base/string_number_conversions.h" #include "base/string_util.h" -#include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread.h" #include "base/utf_string_conversions.h" #include "base/values.h" @@ -36,7 +37,9 @@ class MockReadErrorDelegate : public PersistentPrefStore::ReadErrorDelegate { class JsonPrefStoreTest : public testing::Test { protected: - virtual void SetUp() OVERRIDE { + virtual void SetUp() { + message_loop_proxy_ = base::MessageLoopProxy::current(); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_)); @@ -50,6 +53,7 @@ class JsonPrefStoreTest : public testing::Test { FilePath data_dir_; // A message loop that we can use as the file thread message loop. MessageLoop message_loop_; + scoped_refptr<base::MessageLoopProxy> message_loop_proxy_; }; // Test fallback behavior for a nonexistent file. @@ -57,8 +61,7 @@ TEST_F(JsonPrefStoreTest, NonExistentFile) { FilePath bogus_input_file = data_dir_.AppendASCII("read.txt"); ASSERT_FALSE(file_util::PathExists(bogus_input_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - bogus_input_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(bogus_input_file, message_loop_proxy_.get()); EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE, pref_store->ReadPrefs()); EXPECT_FALSE(pref_store->ReadOnly()); @@ -70,8 +73,7 @@ TEST_F(JsonPrefStoreTest, InvalidFile) { FilePath invalid_file = temp_dir_.path().AppendASCII("invalid.json"); ASSERT_TRUE(file_util::CopyFile(invalid_file_original, invalid_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - invalid_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(invalid_file, message_loop_proxy_.get()); EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_JSON_PARSE, pref_store->ReadPrefs()); EXPECT_FALSE(pref_store->ReadOnly()); @@ -86,7 +88,7 @@ TEST_F(JsonPrefStoreTest, InvalidFile) { // This function is used to avoid code duplication while testing synchronous and // asynchronous version of the JsonPrefStore loading. -void RunBasicJsonPrefStoreTest(JsonPrefStore* pref_store, +void RunBasicJsonPrefStoreTest(JsonPrefStore *pref_store, const FilePath& output_file, const FilePath& golden_output_file) { const char kNewWindowsInTabs[] = "tabs.new_windows_in_tabs"; @@ -164,8 +166,7 @@ TEST_F(JsonPrefStoreTest, Basic) { FilePath input_file = temp_dir_.path().AppendASCII("write.json"); ASSERT_TRUE(file_util::PathExists(input_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - input_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(input_file, message_loop_proxy_.get()); ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs()); ASSERT_FALSE(pref_store->ReadOnly()); @@ -192,24 +193,21 @@ TEST_F(JsonPrefStoreTest, BasicAsync) { FilePath input_file = temp_dir_.path().AppendASCII("write.json"); ASSERT_TRUE(file_util::PathExists(input_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - input_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(input_file, message_loop_proxy_.get()); - { - MockPrefStoreObserver mock_observer; - pref_store->AddObserver(&mock_observer); + MockPrefStoreObserver mock_observer; + pref_store->AddObserver(&mock_observer); - MockReadErrorDelegate* mock_error_delegate = new MockReadErrorDelegate; - pref_store->ReadPrefsAsync(mock_error_delegate); + MockReadErrorDelegate *mock_error_delegate = new MockReadErrorDelegate; + pref_store->ReadPrefsAsync(mock_error_delegate); - EXPECT_CALL(mock_observer, OnInitializationCompleted(true)).Times(1); - EXPECT_CALL(*mock_error_delegate, - OnError(PersistentPrefStore::PREF_READ_ERROR_NONE)).Times(0); - message_loop_.RunUntilIdle(); - pref_store->RemoveObserver(&mock_observer); + EXPECT_CALL(mock_observer, OnInitializationCompleted(true)).Times(1); + EXPECT_CALL(*mock_error_delegate, + OnError(PersistentPrefStore::PREF_READ_ERROR_NONE)).Times(0); + message_loop_.RunUntilIdle(); + pref_store->RemoveObserver(&mock_observer); - ASSERT_FALSE(pref_store->ReadOnly()); - } + ASSERT_FALSE(pref_store->ReadOnly()); // The JSON file looks like this: // { @@ -231,8 +229,7 @@ TEST_F(JsonPrefStoreTest, AsyncNonExistingFile) { FilePath bogus_input_file = data_dir_.AppendASCII("read.txt"); ASSERT_FALSE(file_util::PathExists(bogus_input_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - bogus_input_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(bogus_input_file, message_loop_proxy_.get()); MockPrefStoreObserver mock_observer; pref_store->AddObserver(&mock_observer); @@ -258,8 +255,7 @@ TEST_F(JsonPrefStoreTest, NeedsEmptyValue) { // Test that the persistent value can be loaded. ASSERT_TRUE(file_util::PathExists(pref_file)); scoped_refptr<JsonPrefStore> pref_store = - new JsonPrefStore( - pref_file, message_loop_.message_loop_proxy()); + new JsonPrefStore(pref_file, message_loop_proxy_.get()); ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs()); ASSERT_FALSE(pref_store->ReadOnly()); diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc index aacfea08..46b74e6 100644 --- a/chrome/browser/browser_process_impl.cc +++ b/chrome/browser/browser_process_impl.cc @@ -133,9 +133,7 @@ using content::ChildProcessSecurityPolicy; using content::PluginService; using content::ResourceDispatcherHost; -BrowserProcessImpl::BrowserProcessImpl( - base::SequencedTaskRunner* local_state_task_runner, - const CommandLine& command_line) +BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line) : created_metrics_service_(false), created_watchdog_thread_(false), created_browser_policy_connector_(false), @@ -149,8 +147,7 @@ BrowserProcessImpl::BrowserProcessImpl( checked_for_new_frames_(false), using_new_frames_(false), render_widget_snapshot_taker_(new RenderWidgetSnapshotTaker), - download_status_updater_(new DownloadStatusUpdater), - local_state_task_runner_(local_state_task_runner) { + download_status_updater_(new DownloadStatusUpdater) { g_browser_process = this; #if defined(ENABLE_PRINTING) @@ -712,10 +709,10 @@ void BrowserProcessImpl::CreateLocalState() { created_local_state_ = true; FilePath local_state_path; - CHECK(PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path)); + PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path); local_state_.reset( - PrefService::CreatePrefService(local_state_path, local_state_task_runner_, - policy_service(), NULL, false)); + PrefService::CreatePrefService(local_state_path, 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 5f18e62..d5a1e50 100644 --- a/chrome/browser/browser_process_impl.h +++ b/chrome/browser/browser_process_impl.h @@ -31,10 +31,6 @@ class RemoteDebuggingServer; class PluginsResourceService; #endif -namespace base { -class SequencedTaskRunner; -} - namespace policy { class BrowserPolicyConnector; class PolicyService; @@ -45,9 +41,7 @@ class BrowserProcessImpl : public BrowserProcess, public base::NonThreadSafe, public PrefObserver { public: - // |local_state_task_runner| must be a shutdown-blocking task runner. - BrowserProcessImpl(base::SequencedTaskRunner* local_state_task_runner, - const CommandLine& command_line); + explicit BrowserProcessImpl(const CommandLine& command_line); virtual ~BrowserProcessImpl(); // Called before the browser threads are created. @@ -232,9 +226,6 @@ 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 805cc8e..cf77049 100644 --- a/chrome/browser/chrome_browser_main.cc +++ b/chrome/browser/chrome_browser_main.cc @@ -274,11 +274,8 @@ void InitializeNetworkOptions(const CommandLine& parsed_command_line, } // Returns the new local state object, guaranteed non-NULL. -// |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) { +PrefService* InitializeLocalState(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); @@ -332,7 +329,7 @@ PrefService* InitializeLocalState( FilePath parent_profile = parsed_command_line.GetSwitchValuePath(switches::kParentProfile); scoped_ptr<PrefService> parent_local_state( - PrefService::CreatePrefService(parent_profile, local_state_task_runner, + PrefService::CreatePrefService(parent_profile, g_browser_process->policy_service(), NULL, false)); parent_local_state->RegisterStringPref(prefs::kApplicationLocale, @@ -712,14 +709,7 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { parsed_command_line().HasSwitch(switches::kFirstRun)) && !HasImportSwitch(parsed_command_line()); #endif - - 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())); + browser_process_.reset(new BrowserProcessImpl(parsed_command_line())); if (parsed_command_line().HasSwitch(switches::kEnableProfiling)) { // User wants to override default tracking status. @@ -741,9 +731,7 @@ int ChromeBrowserMainParts::PreCreateThreadsImpl() { switches::kProfilingOutputFile)); } - local_state_ = InitializeLocalState(local_state_task_runner, - parsed_command_line(), - is_first_run_); + local_state_ = InitializeLocalState(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 67eb2a1..d01cb41 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.cc +++ b/chrome/browser/extensions/extension_prefs_unittest.cc @@ -53,11 +53,10 @@ static void AddPattern(URLPatternSet* extent, const std::string& pattern) { ExtensionPrefsTest::ExtensionPrefsTest() : ui_thread_(BrowserThread::UI, &message_loop_), - prefs_(message_loop_.message_loop_proxy()) { + file_thread_(BrowserThread::FILE, &message_loop_) { } -ExtensionPrefsTest::~ExtensionPrefsTest() { -} +ExtensionPrefsTest::~ExtensionPrefsTest() {} void ExtensionPrefsTest::RegisterPreferences() {} @@ -73,8 +72,6 @@ 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 f96ffb6..ba5832a 100644 --- a/chrome/browser/extensions/extension_prefs_unittest.h +++ b/chrome/browser/extensions/extension_prefs_unittest.h @@ -44,6 +44,7 @@ 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 f9ca9e4..ed4382b 100644 --- a/chrome/browser/extensions/extension_service_unittest.cc +++ b/chrome/browser/extensions/extension_service_unittest.cc @@ -422,8 +422,7 @@ 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, loop_.message_loop_proxy()).Create()); + PrefServiceMockBuilder().WithUserFilePrefs(pref_file).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 445a6c4..ef20359 100644 --- a/chrome/browser/extensions/menu_manager_unittest.cc +++ b/chrome/browser/extensions/menu_manager_unittest.cc @@ -15,7 +15,6 @@ #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" @@ -45,15 +44,9 @@ 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 f03df25..7c3cf8e 100644 --- a/chrome/browser/extensions/test_extension_prefs.cc +++ b/chrome/browser/extensions/test_extension_prefs.cc @@ -11,8 +11,6 @@ #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" @@ -33,8 +31,6 @@ 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. @@ -58,10 +54,9 @@ class MockExtensionPrefs : public ExtensionPrefs { } // namespace -TestExtensionPrefs::TestExtensionPrefs( - base::SequencedTaskRunner* task_runner) : pref_service_(NULL), - task_runner_(task_runner), - extensions_disabled_(false) { +TestExtensionPrefs::TestExtensionPrefs() + : pref_service_(NULL), + extensions_disabled_(false) { EXPECT_TRUE(temp_dir_.CreateUniqueTempDir()); preferences_file_ = temp_dir_.path().AppendASCII("Preferences"); extensions_dir_ = temp_dir_.path().AppendASCII("Extensions"); @@ -70,29 +65,36 @@ 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()) { - // 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(); + // 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(); } extension_pref_value_map_.reset(new ExtensionPrefValueMap); PrefServiceMockBuilder builder; - builder.WithUserFilePrefs(preferences_file_, task_runner_); + builder.WithUserFilePrefs(preferences_file_); 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 b2b1faa..5fb1ec5 100644 --- a/chrome/browser/extensions/test_extension_prefs.h +++ b/chrome/browser/extensions/test_extension_prefs.h @@ -16,7 +16,6 @@ class PrefService; namespace base { class DictionaryValue; -class SequencedTaskRunner; } namespace extensions { @@ -26,7 +25,7 @@ class ExtensionPrefs; // in tests. class TestExtensionPrefs { public: - explicit TestExtensionPrefs(base::SequencedTaskRunner* task_runner); + TestExtensionPrefs(); virtual ~TestExtensionPrefs(); ExtensionPrefs* prefs() { return prefs_.get(); } @@ -77,7 +76,6 @@ 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 8f0c464..5bf0548 100644 --- a/chrome/browser/extensions/updater/extension_updater_unittest.cc +++ b/chrome/browser/extensions/updater/extension_updater_unittest.cc @@ -162,12 +162,10 @@ class NotificationsObserver : public content::NotificationObserver { // Base class for further specialized test classes. class MockService : public TestExtensionService { public: - explicit MockService(TestExtensionPrefs* prefs) - : prefs_(prefs), - pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) { + MockService() + : pending_extension_manager_(ALLOW_THIS_IN_INITIALIZER_LIST(*this)) { profile_.CreateRequestContext(); } - virtual ~MockService() {} virtual PendingExtensionManager* pending_extension_manager() OVERRIDE { @@ -182,9 +180,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 @@ -203,15 +201,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: @@ -262,9 +260,7 @@ void SetupPendingExtensionManagerForTest( class ServiceForManifestTests : public MockService { public: - explicit ServiceForManifestTests(TestExtensionPrefs* prefs) - : MockService(prefs) { - } + ServiceForManifestTests() {} virtual ~ServiceForManifestTests() {} @@ -309,8 +305,8 @@ class ServiceForManifestTests : public MockService { class ServiceForDownloadTests : public MockService { public: - explicit ServiceForDownloadTests(TestExtensionPrefs* prefs) - : MockService(prefs) { + ServiceForDownloadTests() + : MockService() { } // Add a fake crx installer to be returned by a call to UpdateExtension() @@ -373,8 +369,8 @@ class ServiceForDownloadTests : public MockService { class ServiceForBlacklistTests : public MockService { public: - explicit ServiceForBlacklistTests(TestExtensionPrefs* prefs) - : MockService(prefs), + ServiceForBlacklistTests() + : MockService(), processed_blacklist_(false) { } virtual void UpdateExtensionBlacklist( @@ -421,26 +417,16 @@ class ExtensionUpdaterTest : public testing::Test { ExtensionUpdaterTest() : ui_thread_(BrowserThread::UI, &loop_), file_thread_(BrowserThread::FILE, &loop_), - io_thread_(BrowserThread::IO, &loop_) { - } - - virtual ~ExtensionUpdaterTest() { - } - - virtual void SetUp() OVERRIDE { - prefs_.reset(new TestExtensionPrefs(loop_.message_loop_proxy())); - } + io_thread_(BrowserThread::IO, &loop_) {} 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. - RunAllPending(); - prefs_.reset(); + loop_.RunAllPending(); } void RunAllPending() { - prefs_->pref_service()->CommitPendingWrite(); loop_.RunAllPending(); } @@ -480,7 +466,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestExtensionUpdateCheckRequests(bool pending) { // Create an extension with an update_url. - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; std::string update_url("http://foo.com/bar"); ExtensionList extensions; PendingExtensionManager* pending_extension_manager = @@ -540,7 +526,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestBlacklistUpdateCheckRequests() { // Setup and start the updater. - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; net::TestURLFetcherFactory factory; ExtensionUpdater updater( @@ -622,7 +608,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestUpdateUrlDataFromGallery(const std::string& gallery_url) { net::TestURLFetcherFactory factory; - MockService service(prefs_.get()); + MockService service; MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); ExtensionList extensions; @@ -703,7 +689,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestDetermineUpdatesPending() { // Create a set of test extensions - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; PendingExtensionManager* pending_extension_manager = service.pending_extension_manager(); SetupPendingExtensionManagerForTest(3, GURL(), pending_extension_manager); @@ -745,7 +731,7 @@ class ExtensionUpdaterTest : public testing::Test { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; NotificationsObserver observer; - MockService service(prefs_.get()); + MockService service; MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); @@ -857,8 +843,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestSingleExtensionDownloading(bool pending) { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - scoped_ptr<ServiceForDownloadTests> service( - new ServiceForDownloadTests(prefs_.get())); + scoped_ptr<ServiceForDownloadTests> service(new ServiceForDownloadTests); ExtensionUpdater updater(service.get(), service->extension_prefs(), service->pref_service(), service->profile(), @@ -914,7 +899,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestBlacklistDownloading() { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - ServiceForBlacklistTests service(prefs_.get()); + ServiceForBlacklistTests service; ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -962,7 +947,7 @@ class ExtensionUpdaterTest : public testing::Test { void TestMultipleExtensionDownloading(bool updates_start_running) { net::TestURLFetcherFactory factory; net::TestURLFetcher* fetcher = NULL; - ServiceForDownloadTests service(prefs_.get()); + ServiceForDownloadTests service; ExtensionUpdater updater( &service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1131,8 +1116,7 @@ class ExtensionUpdaterTest : public testing::Test { // Set up 2 mock extensions, one with a google.com update url and one // without. - prefs_.reset(new TestExtensionPrefs(loop_.message_loop_proxy())); - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; ExtensionList tmp; GURL url1("http://clients2.google.com/service/update2/crx"); GURL url2("http://www.somewebsite.com"); @@ -1244,8 +1228,6 @@ 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 @@ -1253,7 +1235,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(prefs_.get()); + ServiceForManifestTests service; GURL update_url("http://www.google.com/manifest"); ExtensionList tmp; service.CreateTestExtensions(1, 1, &tmp, &update_url.spec(), @@ -1283,9 +1265,6 @@ 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_; @@ -1366,7 +1345,7 @@ TEST_F(ExtensionUpdaterTest, TestHandleManifestResults) { TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) { net::TestURLFetcherFactory factory; - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; ExtensionUpdater updater(&service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1397,7 +1376,7 @@ TEST_F(ExtensionUpdaterTest, TestNonAutoUpdateableLocations) { TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) { net::TestURLFetcherFactory factory; - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; ExtensionUpdater updater(&service, service.extension_prefs(), service.pref_service(), service.profile(), kUpdateFrequencySecs); @@ -1435,7 +1414,7 @@ TEST_F(ExtensionUpdaterTest, TestUpdatingDisabledExtensions) { TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { net::TestURLFetcherFactory factory; - MockService service(prefs_.get()); + MockService service; MockExtensionDownloaderDelegate delegate; scoped_ptr<ExtensionDownloader> downloader( new ExtensionDownloader(&delegate, service.request_context())); @@ -1487,7 +1466,7 @@ TEST_F(ExtensionUpdaterTest, TestManifestFetchesBuilderAddExtension) { TEST_F(ExtensionUpdaterTest, TestStartUpdateCheckMemory) { net::TestURLFetcherFactory factory; - MockService service(prefs_.get()); + MockService service; MockExtensionDownloaderDelegate delegate; ExtensionDownloader downloader(&delegate, service.request_context()); @@ -1501,7 +1480,7 @@ TEST_F(ExtensionUpdaterTest, TestStartUpdateCheckMemory) { } TEST_F(ExtensionUpdaterTest, TestCheckSoon) { - ServiceForManifestTests service(prefs_.get()); + ServiceForManifestTests service; 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 680e4a5..58f398d 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/chrome/browser/metrics/metrics_service.cc @@ -1723,11 +1723,27 @@ 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 c55f87f..7fecdb1 100644 --- a/chrome/browser/policy/managed_mode_policy_provider.cc +++ b/chrome/browser/policy/managed_mode_policy_provider.cc @@ -5,7 +5,6 @@ #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" @@ -20,11 +19,11 @@ const char ManagedModePolicyProvider::kPolicies[] = "policies"; // static ManagedModePolicyProvider* ManagedModePolicyProvider::Create(Profile* profile) { - FilePath path = profile->GetPath().Append(chrome::kManagedModePolicyFilename); - JsonPrefStore* pref_store = new JsonPrefStore( - path, - JsonPrefStore::GetTaskRunnerForFile(path, - BrowserThread::GetBlockingPool())); + JsonPrefStore* pref_store = + new JsonPrefStore(profile->GetPath().Append( + chrome::kManagedModePolicyFilename), + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE)); return new ManagedModePolicyProvider(pref_store); } diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index 1eb80b4..40ae0d3 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -123,7 +123,6 @@ 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) { @@ -157,7 +156,8 @@ PrefService* PrefService::CreatePrefService( CommandLinePrefStore* command_line = new CommandLinePrefStore(CommandLine::ForCurrentProcess()); JsonPrefStore* user = new JsonPrefStore( - pref_filename, pref_io_task_runner); + pref_filename, + BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)); 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 1536b95..ff4585c 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -33,10 +33,6 @@ class PrefServiceObserver; class PrefStore; class PrefValueStore; -namespace base { - class SequencedTaskRunner; -} - namespace syncer { class SyncableService; } @@ -114,12 +110,10 @@ 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, - base::SequencedTaskRunner* pref_io_task_runner, - policy::PolicyService* policy_service, - PrefStore* extension_pref_store, - bool async); + static PrefService* CreatePrefService(const FilePath& pref_filename, + 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 77d8080..2782882 100644 --- a/chrome/browser/prefs/pref_service_mock_builder.cc +++ b/chrome/browser/prefs/pref_service_mock_builder.cc @@ -77,17 +77,16 @@ PrefServiceMockBuilder::WithCommandLine(CommandLine* command_line) { PrefServiceMockBuilder& PrefServiceMockBuilder::WithUserFilePrefs(const FilePath& prefs_file) { - return WithUserFilePrefs( - prefs_file, - JsonPrefStore::GetTaskRunnerForFile(prefs_file, - BrowserThread::GetBlockingPool())); + return WithUserFilePrefs(prefs_file, + BrowserThread::GetMessageLoopProxyForThread( + BrowserThread::FILE)); } PrefServiceMockBuilder& PrefServiceMockBuilder::WithUserFilePrefs( const FilePath& prefs_file, - base::SequencedTaskRunner* task_runner) { - user_prefs_ = new JsonPrefStore(prefs_file, task_runner); + base::MessageLoopProxy* message_loop_proxy) { + user_prefs_ = new JsonPrefStore(prefs_file, message_loop_proxy); return *this; } diff --git a/chrome/browser/prefs/pref_service_mock_builder.h b/chrome/browser/prefs/pref_service_mock_builder.h index 708b622..b939ca6 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 SequencedTaskRunner; +class MessageLoopProxy; } namespace policy { @@ -48,11 +48,10 @@ 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::SequencedTaskRunner* task_runner); + base::MessageLoopProxy* message_loop_proxy); // 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 280a346..1286f9f 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, message_loop_.message_loop_proxy()); + builder.WithUserFilePrefs(pref_file, base::MessageLoopProxy::current()); scoped_ptr<PrefService> prefs(builder.Create()); // Register testing prefs. @@ -344,7 +344,7 @@ TEST_F(PrefServiceUserFilePrefsTest, PreserveEmptyValue) { // Write to file. prefs->CommitPendingWrite(); - message_loop_.RunAllPending(); + MessageLoop::current()->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 7902049..420a0fc 100644 --- a/chrome/browser/profiles/profile_impl.cc +++ b/chrome/browser/profiles/profile_impl.cc @@ -13,13 +13,10 @@ #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" @@ -152,38 +149,6 @@ 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); } @@ -234,15 +199,12 @@ 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); - CreateProfileDirectory(sequenced_task_runner, path); + // 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)); } 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 @@ -255,7 +217,7 @@ Profile* Profile::CreateProfile(const FilePath& path, NOTREACHED(); } - return new ProfileImpl(path, delegate, create_mode, sequenced_task_runner); + return new ProfileImpl(path, delegate, create_mode); } // static @@ -308,11 +270,9 @@ void ProfileImpl::RegisterUserPrefs(PrefService* prefs) { PrefService::SYNCABLE_PREF); } -ProfileImpl::ProfileImpl( - const FilePath& path, - Delegate* delegate, - CreateMode create_mode, - base::SequencedTaskRunner* sequenced_task_runner) +ProfileImpl::ProfileImpl(const FilePath& path, + Delegate* delegate, + CreateMode create_mode) : path_(path), ALLOW_THIS_IN_INITIALIZER_LIST(io_data_(this)), host_content_settings_map_(NULL), @@ -357,7 +317,6 @@ ProfileImpl::ProfileImpl( if (create_mode == CREATE_MODE_ASYNCHRONOUS) { prefs_.reset(PrefService::CreatePrefService( GetPrefFilePath(), - sequenced_task_runner, policy_service_.get(), new ExtensionPrefStore( ExtensionPrefValueMapFactory::GetForProfile(this), false), @@ -372,7 +331,6 @@ ProfileImpl::ProfileImpl( // Load prefs synchronously. prefs_.reset(PrefService::CreatePrefService( GetPrefFilePath(), - sequenced_task_runner, policy_service_.get(), new ExtensionPrefStore( ExtensionPrefValueMapFactory::GetForProfile(this), false), @@ -396,10 +354,10 @@ void ProfileImpl::DoFinalInit(bool is_new_profile) { // to PathService. chrome::GetUserCacheDirectory(path_, &base_cache_path_); // Always create the cache directory asynchronously. - scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner = - JsonPrefStore::GetTaskRunnerForFile(base_cache_path_, - BrowserThread::GetBlockingPool()); - CreateProfileDirectory(sequenced_task_runner, base_cache_path_); + BrowserThread::PostTask( + BrowserThread::FILE, FROM_HERE, + base::Bind(base::IgnoreResult(&file_util::CreateDirectory), + 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 36ed8bc..bb8d9c3 100644 --- a/chrome/browser/profiles/profile_impl.h +++ b/chrome/browser/profiles/profile_impl.h @@ -34,10 +34,6 @@ class Preferences; } #endif -namespace base { -class SequencedTaskRunner; -} - namespace content { class SpeechRecognitionPreferences; } @@ -151,8 +147,7 @@ class ProfileImpl : public Profile, ProfileImpl(const FilePath& path, Delegate* delegate, - CreateMode create_mode, - base::SequencedTaskRunner* sequenced_task_runner); + CreateMode create_mode); // 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 8d05e0d..50fbe95 100644 --- a/chrome/browser/sync/credential_cache_service_win.cc +++ b/chrome/browser/sync/credential_cache_service_win.cc @@ -691,9 +691,8 @@ 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( - path, + GetCredentialPathInAlternateProfile(), content::BrowserThread::GetMessageLoopProxyForThread( content::BrowserThread::FILE)); alternate_store_observer_ = new AlternateStoreObserver(this, diff --git a/chrome/service/cloud_print/connector_settings_unittest.cc b/chrome/service/cloud_print/connector_settings_unittest.cc index b86c315..09ceeef 100644 --- a/chrome/service/cloud_print/connector_settings_unittest.cc +++ b/chrome/service/cloud_print/connector_settings_unittest.cc @@ -44,7 +44,7 @@ const char kServiceStateContent[] = class ConnectorSettingsTest : public testing::Test { protected: - virtual void SetUp() OVERRIDE { + virtual void SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); message_loop_proxy_ = base::MessageLoopProxy::current(); } @@ -58,7 +58,7 @@ class ConnectorSettingsTest : public testing::Test { file_util::WriteFile(file_name, content.c_str(), content.size()); } ServiceProcessPrefs* prefs = - new ServiceProcessPrefs(file_name, message_loop_proxy_); + new ServiceProcessPrefs(file_name, message_loop_proxy_.get()); prefs->ReadPrefs(); return prefs; } diff --git a/chrome/service/service_process.cc b/chrome/service/service_process.cc index 40607d9..f820f93 100644 --- a/chrome/service/service_process.cc +++ b/chrome/service/service_process.cc @@ -13,7 +13,6 @@ #include "base/i18n/rtl.h" #include "base/memory/singleton.h" #include "base/path_service.h" -#include "base/prefs/json_pref_store.h" #include "base/string16.h" #include "base/utf_string_conversions.h" #include "base/values.h" @@ -153,7 +152,6 @@ bool ServiceProcess::Initialize(MessageLoopForUI* message_loop, Teardown(); return false; } - blocking_pool_ = new base::SequencedWorkerPool(3, "ServiceBlocking"); request_context_getter_ = new ServiceURLRequestContextGetter(); @@ -161,9 +159,7 @@ bool ServiceProcess::Initialize(MessageLoopForUI* message_loop, PathService::Get(chrome::DIR_USER_DATA, &user_data_dir); FilePath pref_path = user_data_dir.Append(chrome::kServiceStateFileName); service_prefs_.reset( - new ServiceProcessPrefs( - pref_path, - JsonPrefStore::GetTaskRunnerForFile(pref_path, blocking_pool_))); + new ServiceProcessPrefs(pref_path, file_thread_->message_loop_proxy())); service_prefs_->ReadPrefs(); // Check if a locale override has been specified on the command-line. @@ -222,12 +218,6 @@ bool ServiceProcess::Teardown() { shutdown_event_.Signal(); io_thread_.reset(); file_thread_.reset(); - - if (blocking_pool_.get()) { - blocking_pool_->Shutdown(); - blocking_pool_ = NULL; - } - // The NetworkChangeNotifier must be destroyed after all other threads that // might use it have been shut down. network_change_notifier_.reset(); diff --git a/chrome/service/service_process.h b/chrome/service/service_process.h index af2afb5..47617ae 100644 --- a/chrome/service/service_process.h +++ b/chrome/service/service_process.h @@ -10,7 +10,6 @@ #include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/threading/sequenced_worker_pool.h" #include "base/threading/thread.h" #include "base/synchronization/waitable_event.h" #include "chrome/service/cloud_print/cloud_print_proxy.h" @@ -124,7 +123,6 @@ class ServiceProcess : public CloudPrintProxy::Client { scoped_ptr<net::NetworkChangeNotifier> network_change_notifier_; scoped_ptr<base::Thread> io_thread_; scoped_ptr<base::Thread> file_thread_; - scoped_refptr<base::SequencedWorkerPool> blocking_pool_; scoped_ptr<CloudPrintProxy> cloud_print_proxy_; scoped_ptr<ServiceProcessPrefs> service_prefs_; scoped_ptr<ServiceIPCServer> ipc_server_; diff --git a/chrome/service/service_process_prefs.cc b/chrome/service/service_process_prefs.cc index e417988..811647d 100644 --- a/chrome/service/service_process_prefs.cc +++ b/chrome/service/service_process_prefs.cc @@ -4,13 +4,12 @@ #include "chrome/service/service_process_prefs.h" -#include "base/message_loop_proxy.h" #include "base/values.h" ServiceProcessPrefs::ServiceProcessPrefs( const FilePath& pref_filename, - base::SequencedTaskRunner* task_runner) - : prefs_(new JsonPrefStore(pref_filename, task_runner)) { + base::MessageLoopProxy* file_message_loop_proxy) + : prefs_(new JsonPrefStore(pref_filename, file_message_loop_proxy)) { } ServiceProcessPrefs::~ServiceProcessPrefs() {} diff --git a/chrome/service/service_process_prefs.h b/chrome/service/service_process_prefs.h index fdbf22e..018ce1d 100644 --- a/chrome/service/service_process_prefs.h +++ b/chrome/service/service_process_prefs.h @@ -12,16 +12,16 @@ namespace base { class DictionaryValue; class ListValue; -class SequencedTaskRunner; } // Manages persistent preferences for the service process. This is basically a // thin wrapper around JsonPrefStore for more comfortable use. class ServiceProcessPrefs { public: - // |sequenced_task_runner| must be a shutdown-blocking task runner. + // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which + // file I/O can be done. ServiceProcessPrefs(const FilePath& pref_filename, - base::SequencedTaskRunner* task_runner); + base::MessageLoopProxy* file_message_loop_proxy); ~ServiceProcessPrefs(); // Read preferences from the backing file. diff --git a/chrome/service/service_process_prefs_unittest.cc b/chrome/service/service_process_prefs_unittest.cc index 5ce2808..0268395 100644 --- a/chrome/service/service_process_prefs_unittest.cc +++ b/chrome/service/service_process_prefs_unittest.cc @@ -5,30 +5,29 @@ #include <string> #include "base/message_loop.h" +#include "base/message_loop_proxy.h" #include "base/scoped_temp_dir.h" -#include "base/sequenced_task_runner.h" #include "chrome/service/service_process_prefs.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" class ServiceProcessPrefsTest : public testing::Test { protected: - virtual void SetUp() OVERRIDE { + virtual void SetUp() { + message_loop_proxy_ = base::MessageLoopProxy::current(); + ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); prefs_.reset(new ServiceProcessPrefs( temp_dir_.path().AppendASCII("service_process_prefs.txt"), - message_loop_.message_loop_proxy())); - } - - virtual void TearDown() OVERRIDE { - prefs_.reset(); + message_loop_proxy_.get())); } // The path to temporary directory used to contain the test operations. ScopedTempDir temp_dir_; // A message loop that we can use as the file thread message loop. MessageLoop message_loop_; + scoped_refptr<base::MessageLoopProxy> message_loop_proxy_; scoped_ptr<ServiceProcessPrefs> prefs_; }; @@ -37,7 +36,7 @@ TEST_F(ServiceProcessPrefsTest, RetrievePrefs) { prefs_->SetBoolean("testb", true); prefs_->SetString("tests", "testvalue"); prefs_->WritePrefs(); - message_loop_.RunAllPending(); + MessageLoop::current()->RunAllPending(); prefs_->SetBoolean("testb", false); // overwrite prefs_->SetString("tests", ""); // overwrite prefs_->ReadPrefs(); diff --git a/chrome/test/reliability/page_load_test.cc b/chrome/test/reliability/page_load_test.cc index 9ac520c..9e3909d 100644 --- a/chrome/test/reliability/page_load_test.cc +++ b/chrome/test/reliability/page_load_test.cc @@ -739,8 +739,7 @@ class PageLoadTest : public UITest { // returned PrefService object. PrefService* GetLocalState() { FilePath path = user_data_dir().Append(chrome::kLocalStateFilename); - return PrefServiceMockBuilder().WithUserFilePrefs( - path, MessageLoop::current()->message_loop_proxy()).Create(); + return PrefServiceMockBuilder().WithUserFilePrefs(path).Create(); } void GetStabilityMetrics(NavigationMetrics* metrics) { diff --git a/chrome_frame/test/net/fake_external_tab.cc b/chrome_frame/test/net/fake_external_tab.cc index 2fd8530..1f6c808 100644 --- a/chrome_frame/test/net/fake_external_tab.cc +++ b/chrome_frame/test/net/fake_external_tab.cc @@ -18,7 +18,6 @@ #include "base/lazy_instance.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" -#include "base/prefs/json_pref_store.h" #include "base/scoped_temp_dir.h" #include "base/string_piece.h" #include "base/string_util.h" @@ -353,9 +352,8 @@ void FilterDisabledTests() { // Same as BrowserProcessImpl, but uses custom profile manager. class FakeBrowserProcessImpl : public BrowserProcessImpl { public: - FakeBrowserProcessImpl(base::SequencedTaskRunner* local_state_task_runner, - const CommandLine& command_line) - : BrowserProcessImpl(local_state_task_runner, command_line) { + explicit FakeBrowserProcessImpl(const CommandLine& command_line) + : BrowserProcessImpl(command_line) { profiles_dir_.CreateUniqueTempDir(); } @@ -496,13 +494,7 @@ void FakeExternalTab::Initialize() { cmd->AppendSwitch(switches::kDisableWebResources); cmd->AppendSwitch(switches::kSingleProcess); - 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 FakeBrowserProcessImpl(local_state_task_runner, - *cmd)); + browser_process_.reset(new FakeBrowserProcessImpl(*cmd)); // BrowserProcessImpl's constructor should set g_browser_process. DCHECK(g_browser_process); g_browser_process->SetApplicationLocale("en-US"); |