From a76bf96db2cb5aef56de46860630da6a0a6c2477 Mon Sep 17 00:00:00 2001 From: "akalin@chromium.org" Date: Thu, 15 Sep 2011 18:54:03 +0000 Subject: Add GROUP_FILE ModelSafeGroup Original patch by kalman@chromium.org BUG= TEST=Added FileModelWorkerUnittest TBR=kalman@chromium.org Review URL: http://codereview.chromium.org/7908010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@101346 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/sync/engine/mock_model_safe_workers.cc | 2 + .../browser/sync/engine/mock_model_safe_workers.h | 5 ++ chrome/browser/sync/engine/model_safe_worker.cc | 2 + chrome/browser/sync/engine/model_safe_worker.h | 1 + .../sync/glue/browser_thread_model_worker.cc | 52 ++++++++++++ .../sync/glue/browser_thread_model_worker.h | 44 ++++++++++ .../glue/browser_thread_model_worker_unittest.cc | 98 ++++++++++++++++++++++ chrome/browser/sync/glue/database_model_worker.cc | 41 --------- chrome/browser/sync/glue/database_model_worker.h | 37 -------- .../sync/glue/database_model_worker_unittest.cc | 93 -------------------- chrome/browser/sync/glue/sync_backend_registrar.cc | 9 +- .../sync/glue/sync_backend_registrar_unittest.cc | 4 +- chrome/chrome_browser.gypi | 4 +- chrome/chrome_tests.gypi | 2 +- 14 files changed, 216 insertions(+), 178 deletions(-) create mode 100644 chrome/browser/sync/glue/browser_thread_model_worker.cc create mode 100644 chrome/browser/sync/glue/browser_thread_model_worker.h create mode 100644 chrome/browser/sync/glue/browser_thread_model_worker_unittest.cc delete mode 100644 chrome/browser/sync/glue/database_model_worker.cc delete mode 100644 chrome/browser/sync/glue/database_model_worker.h delete mode 100644 chrome/browser/sync/glue/database_model_worker_unittest.cc (limited to 'chrome') diff --git a/chrome/browser/sync/engine/mock_model_safe_workers.cc b/chrome/browser/sync/engine/mock_model_safe_workers.cc index 8e121af..44aebdd 100644 --- a/chrome/browser/sync/engine/mock_model_safe_workers.cc +++ b/chrome/browser/sync/engine/mock_model_safe_workers.cc @@ -10,6 +10,8 @@ ModelSafeGroup MockUIModelWorker::GetModelSafeGroup() { return GROUP_UI; } ModelSafeGroup MockDBModelWorker::GetModelSafeGroup() { return GROUP_DB; } +ModelSafeGroup MockFileModelWorker::GetModelSafeGroup() { return GROUP_FILE; } + MockModelSafeWorkerRegistrar::~MockModelSafeWorkerRegistrar() {} // static diff --git a/chrome/browser/sync/engine/mock_model_safe_workers.h b/chrome/browser/sync/engine/mock_model_safe_workers.h index e5526cf..efe2a1e 100644 --- a/chrome/browser/sync/engine/mock_model_safe_workers.h +++ b/chrome/browser/sync/engine/mock_model_safe_workers.h @@ -24,6 +24,11 @@ class MockDBModelWorker : public ModelSafeWorker { virtual ModelSafeGroup GetModelSafeGroup(); }; +class MockFileModelWorker : public ModelSafeWorker { + public: + virtual ModelSafeGroup GetModelSafeGroup(); +}; + class MockModelSafeWorkerRegistrar : public ModelSafeWorkerRegistrar { public: virtual ~MockModelSafeWorkerRegistrar(); diff --git a/chrome/browser/sync/engine/model_safe_worker.cc b/chrome/browser/sync/engine/model_safe_worker.cc index d879bb2..8142be2 100644 --- a/chrome/browser/sync/engine/model_safe_worker.cc +++ b/chrome/browser/sync/engine/model_safe_worker.cc @@ -59,6 +59,8 @@ std::string ModelSafeGroupToString(ModelSafeGroup group) { return "GROUP_UI"; case GROUP_DB: return "GROUP_DB"; + case GROUP_FILE: + return "GROUP_FILE"; case GROUP_HISTORY: return "GROUP_HISTORY"; case GROUP_PASSIVE: diff --git a/chrome/browser/sync/engine/model_safe_worker.h b/chrome/browser/sync/engine/model_safe_worker.h index 4b8e4e7..f4829ec 100644 --- a/chrome/browser/sync/engine/model_safe_worker.h +++ b/chrome/browser/sync/engine/model_safe_worker.h @@ -26,6 +26,7 @@ enum ModelSafeGroup { // native model. GROUP_UI, // Models that live on UI thread and are being synced. GROUP_DB, // Models that live on DB thread and are being synced. + GROUP_FILE, // Models that live on FILE thread and are being synced. GROUP_HISTORY, // Models that live on history thread and are being // synced. GROUP_PASSWORD, // Models that live on the password thread and are diff --git a/chrome/browser/sync/glue/browser_thread_model_worker.cc b/chrome/browser/sync/glue/browser_thread_model_worker.cc new file mode 100644 index 0000000..49bdfb8 --- /dev/null +++ b/chrome/browser/sync/glue/browser_thread_model_worker.cc @@ -0,0 +1,52 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/browser/sync/glue/browser_thread_model_worker.h" + +#include "base/synchronization/waitable_event.h" +#include "content/browser/browser_thread.h" + +using base::WaitableEvent; + +namespace browser_sync { + +BrowserThreadModelWorker::BrowserThreadModelWorker( + BrowserThread::ID thread, ModelSafeGroup group) + : thread_(thread), group_(group) {} + +BrowserThreadModelWorker::~BrowserThreadModelWorker() {} + +void BrowserThreadModelWorker::DoWorkAndWaitUntilDone(Callback0::Type* work) { + if (BrowserThread::CurrentlyOn(thread_)) { + DLOG(WARNING) << "Already on thread " << thread_; + work->Run(); + return; + } + WaitableEvent done(false, false); + if (!BrowserThread::PostTask( + thread_, + FROM_HERE, + NewRunnableMethod( + this, + &BrowserThreadModelWorker::CallDoWorkAndSignalTask, + work, + &done))) { + NOTREACHED() << "Failed to post task to thread " << thread_; + return; + } + done.Wait(); +} + +void BrowserThreadModelWorker::CallDoWorkAndSignalTask( + Callback0::Type* work, WaitableEvent* done) { + DCHECK(BrowserThread::CurrentlyOn(thread_)); + work->Run(); + done->Signal(); +} + +ModelSafeGroup BrowserThreadModelWorker::GetModelSafeGroup() { + return group_; +} + +} // namespace browser_sync diff --git a/chrome/browser/sync/glue/browser_thread_model_worker.h b/chrome/browser/sync/glue/browser_thread_model_worker.h new file mode 100644 index 0000000..51670a1 --- /dev/null +++ b/chrome/browser/sync/glue/browser_thread_model_worker.h @@ -0,0 +1,44 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_SYNC_GLUE_BROWSER_THREAD_MODEL_WORKER_H_ +#define CHROME_BROWSER_SYNC_GLUE_BROWSER_THREAD_MODEL_WORKER_H_ +#pragma once + +#include "base/callback.h" +#include "base/basictypes.h" +#include "chrome/browser/sync/engine/model_safe_worker.h" +#include "content/browser/browser_thread.h" + +namespace base { +class WaitableEvent; +} + +namespace browser_sync { + +// A ModelSafeWorker for models that accept requests from the syncapi that need +// to be fulfilled on a browser thread, for example autofill on the DB thread. +// TODO(sync): Try to generalize other ModelWorkers (e.g. history, etc). +class BrowserThreadModelWorker : public browser_sync::ModelSafeWorker { + public: + BrowserThreadModelWorker(BrowserThread::ID thread, ModelSafeGroup group); + virtual ~BrowserThreadModelWorker(); + + // ModelSafeWorker implementation. Called on the sync thread. + virtual void DoWorkAndWaitUntilDone(Callback0::Type* work); + virtual ModelSafeGroup GetModelSafeGroup(); + + private: + void CallDoWorkAndSignalTask( + Callback0::Type* work, base::WaitableEvent* done); + + BrowserThread::ID thread_; + ModelSafeGroup group_; + + DISALLOW_COPY_AND_ASSIGN(BrowserThreadModelWorker); +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_GLUE_BROWSER_THREAD_MODEL_WORKER_H_ diff --git a/chrome/browser/sync/glue/browser_thread_model_worker_unittest.cc b/chrome/browser/sync/glue/browser_thread_model_worker_unittest.cc new file mode 100644 index 0000000..3501e53 --- /dev/null +++ b/chrome/browser/sync/glue/browser_thread_model_worker_unittest.cc @@ -0,0 +1,98 @@ +// Copyright (c) 2011 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "base/callback.h" +#include "base/memory/scoped_ptr.h" +#include "base/message_loop.h" +#include "base/threading/thread.h" +#include "base/test/test_timeouts.h" +#include "base/timer.h" +#include "chrome/browser/sync/glue/browser_thread_model_worker.h" +#include "content/browser/browser_thread.h" +#include "testing/gtest/include/gtest/gtest.h" + +using base::OneShotTimer; +using base::Thread; +using base::TimeDelta; +using browser_sync::BrowserThreadModelWorker; +using browser_sync::GROUP_DB; + +namespace { + +class BrowserThreadModelWorkerTest : public testing::Test { + public: + BrowserThreadModelWorkerTest() : + did_do_work_(false), + db_thread_(BrowserThread::DB), + io_thread_(BrowserThread::IO, &io_loop_), + ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {} + + bool did_do_work() { return did_do_work_; } + BrowserThreadModelWorker* worker() { return worker_.get(); } + OneShotTimer* timer() { return &timer_; } + ScopedRunnableMethodFactory* factory() { + return &method_factory_; + } + + // Schedule DoWork to be executed on the DB thread and have the test fail if + // DoWork hasn't executed within action_timeout_ms() ms. + void ScheduleWork() { + scoped_ptr c(NewCallback(this, + &BrowserThreadModelWorkerTest::DoWork)); + timer()->Start( + FROM_HERE, + TimeDelta::FromMilliseconds(TestTimeouts::action_timeout_ms()), + this, + &BrowserThreadModelWorkerTest::Timeout); + worker()->DoWorkAndWaitUntilDone(c.get()); + } + + // This is the work that will be scheduled to be done on the DB thread. + void DoWork() { + EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::DB)); + timer_.Stop(); // Stop the failure timer so the test succeeds. + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, new MessageLoop::QuitTask()); + did_do_work_ = true; + } + + // This will be called by the OneShotTimer and make the test fail unless + // DoWork is called first. + void Timeout() { + ADD_FAILURE() << "Timed out waiting for work to be done on the DB thread."; + BrowserThread::PostTask( + BrowserThread::IO, FROM_HERE, new MessageLoop::QuitTask()); + } + + protected: + virtual void SetUp() { + db_thread_.Start(); + worker_ = new BrowserThreadModelWorker(BrowserThread::DB, GROUP_DB); + } + + virtual void Teardown() { + worker_.release(); + db_thread_.Stop(); + } + + private: + bool did_do_work_; + scoped_refptr worker_; + OneShotTimer timer_; + + BrowserThread db_thread_; + MessageLoopForIO io_loop_; + BrowserThread io_thread_; + + ScopedRunnableMethodFactory method_factory_; +}; + +TEST_F(BrowserThreadModelWorkerTest, DoesWorkOnDatabaseThread) { + MessageLoop::current()->PostTask(FROM_HERE, factory()->NewRunnableMethod( + &BrowserThreadModelWorkerTest::ScheduleWork)); + MessageLoop::current()->Run(); + EXPECT_TRUE(did_do_work()); +} + +} // namespace diff --git a/chrome/browser/sync/glue/database_model_worker.cc b/chrome/browser/sync/glue/database_model_worker.cc deleted file mode 100644 index 83330af..0000000 --- a/chrome/browser/sync/glue/database_model_worker.cc +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/sync/glue/database_model_worker.h" - -#include "base/synchronization/waitable_event.h" -#include "content/browser/browser_thread.h" - -using base::WaitableEvent; - -namespace browser_sync { - -void DatabaseModelWorker::DoWorkAndWaitUntilDone(Callback0::Type* work) { - if (BrowserThread::CurrentlyOn(BrowserThread::DB)) { - DLOG(WARNING) << "DoWorkAndWaitUntilDone called from the DB thread."; - work->Run(); - return; - } - WaitableEvent done(false, false); - if (!BrowserThread::PostTask(BrowserThread::DB, FROM_HERE, - NewRunnableMethod(this, &DatabaseModelWorker::CallDoWorkAndSignalTask, - work, &done))) { - NOTREACHED() << "Failed to post task to the db thread."; - return; - } - done.Wait(); -} - -void DatabaseModelWorker::CallDoWorkAndSignalTask(Callback0::Type* work, - WaitableEvent* done) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB)); - work->Run(); - done->Signal(); -} - -ModelSafeGroup DatabaseModelWorker::GetModelSafeGroup() { - return GROUP_DB; -} - -} // namespace browser_sync diff --git a/chrome/browser/sync/glue/database_model_worker.h b/chrome/browser/sync/glue/database_model_worker.h deleted file mode 100644 index b23b052..0000000 --- a/chrome/browser/sync/glue/database_model_worker.h +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROME_BROWSER_SYNC_GLUE_DATABASE_MODEL_WORKER_H_ -#define CHROME_BROWSER_SYNC_GLUE_DATABASE_MODEL_WORKER_H_ -#pragma once - -#include "base/callback.h" -#include "chrome/browser/sync/engine/model_safe_worker.h" - -namespace base { -class WaitableEvent; -} - -namespace browser_sync { - -// A ModelSafeWorker for database models (eg. autofill) that accepts requests -// from the syncapi that need to be fulfilled on the database thread. -class DatabaseModelWorker : public browser_sync::ModelSafeWorker { - public: - explicit DatabaseModelWorker() {} - - // ModelSafeWorker implementation. Called on syncapi SyncerThread. - virtual void DoWorkAndWaitUntilDone(Callback0::Type* work); - virtual ModelSafeGroup GetModelSafeGroup(); - - private: - void CallDoWorkAndSignalTask(Callback0::Type* work, - base::WaitableEvent* done); - - DISALLOW_COPY_AND_ASSIGN(DatabaseModelWorker); -}; - -} // namespace browser_sync - -#endif // CHROME_BROWSER_SYNC_GLUE_DATABASE_MODEL_WORKER_H_ diff --git a/chrome/browser/sync/glue/database_model_worker_unittest.cc b/chrome/browser/sync/glue/database_model_worker_unittest.cc deleted file mode 100644 index 150dd31..0000000 --- a/chrome/browser/sync/glue/database_model_worker_unittest.cc +++ /dev/null @@ -1,93 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/callback.h" -#include "base/memory/scoped_ptr.h" -#include "base/message_loop.h" -#include "base/threading/thread.h" -#include "base/timer.h" -#include "chrome/browser/sync/glue/database_model_worker.h" -#include "content/browser/browser_thread.h" -#include "testing/gtest/include/gtest/gtest.h" - -using base::OneShotTimer; -using base::Thread; -using base::TimeDelta; -using browser_sync::DatabaseModelWorker; - -namespace { - -class DatabaseModelWorkerTest : public testing::Test { - public: - DatabaseModelWorkerTest() : - did_do_work_(false), - db_thread_(BrowserThread::DB), - io_thread_(BrowserThread::IO, &io_loop_), - ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {} - - bool did_do_work() { return did_do_work_; } - DatabaseModelWorker* worker() { return worker_.get(); } - OneShotTimer* timer() { return &timer_; } - ScopedRunnableMethodFactory* factory() { - return &method_factory_; - } - - // Schedule DoWork to be executed on the DB thread and have the test fail if - // DoWork hasn't executed within 10 seconds. - void ScheduleWork() { - scoped_ptr c(NewCallback(this, - &DatabaseModelWorkerTest::DoWork)); - timer()->Start(FROM_HERE, TimeDelta::FromSeconds(10), - this, &DatabaseModelWorkerTest::Timeout); - worker()->DoWorkAndWaitUntilDone(c.get()); - } - - // This is the work that will be scheduled to be done on the DB thread. - void DoWork() { - EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::DB)); - timer_.Stop(); // Stop the failure timer so the test succeeds. - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, new MessageLoop::QuitTask()); - did_do_work_ = true; - } - - // This will be called by the OneShotTimer and make the test fail unless - // DoWork is called first. - void Timeout() { - ADD_FAILURE() << "Timed out waiting for work to be done on the DB thread."; - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, new MessageLoop::QuitTask()); - } - - protected: - virtual void SetUp() { - db_thread_.Start(); - worker_ = new DatabaseModelWorker(); - } - - virtual void Teardown() { - worker_.release(); - db_thread_.Stop(); - } - - private: - bool did_do_work_; - scoped_refptr worker_; - OneShotTimer timer_; - - BrowserThread db_thread_; - MessageLoopForIO io_loop_; - BrowserThread io_thread_; - - ScopedRunnableMethodFactory method_factory_; -}; - -TEST_F(DatabaseModelWorkerTest, DoesWorkOnDatabaseThread) { - MessageLoop::current()->PostTask(FROM_HERE, factory()->NewRunnableMethod( - &DatabaseModelWorkerTest::ScheduleWork)); - MessageLoop::current()->Run(); - EXPECT_TRUE(did_do_work()); -} - -} // namespace diff --git a/chrome/browser/sync/glue/sync_backend_registrar.cc b/chrome/browser/sync/glue/sync_backend_registrar.cc index 376ab5b..5edf2b7 100644 --- a/chrome/browser/sync/glue/sync_backend_registrar.cc +++ b/chrome/browser/sync/glue/sync_backend_registrar.cc @@ -11,7 +11,7 @@ #include "base/message_loop.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/glue/change_processor.h" -#include "chrome/browser/sync/glue/database_model_worker.h" +#include "chrome/browser/sync/glue/browser_thread_model_worker.h" #include "chrome/browser/sync/glue/history_model_worker.h" #include "chrome/browser/sync/glue/password_model_worker.h" #include "chrome/browser/sync/glue/ui_model_worker.h" @@ -31,6 +31,8 @@ bool IsOnThreadForGroup(ModelSafeGroup group) { return BrowserThread::CurrentlyOn(BrowserThread::UI); case GROUP_DB: return BrowserThread::CurrentlyOn(BrowserThread::DB); + case GROUP_FILE: + return BrowserThread::CurrentlyOn(BrowserThread::FILE); case GROUP_HISTORY: // TODO(ncarter): How to determine this? return true; @@ -57,7 +59,10 @@ SyncBackendRegistrar::SyncBackendRegistrar( DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); CHECK(profile_); DCHECK(sync_loop_); - workers_[GROUP_DB] = new DatabaseModelWorker(); + workers_[GROUP_DB] = + new BrowserThreadModelWorker(BrowserThread::DB, GROUP_DB); + workers_[GROUP_FILE] = + new BrowserThreadModelWorker(BrowserThread::FILE, GROUP_FILE); workers_[GROUP_UI] = ui_worker_; workers_[GROUP_PASSIVE] = new ModelSafeWorker(); diff --git a/chrome/browser/sync/glue/sync_backend_registrar_unittest.cc b/chrome/browser/sync/glue/sync_backend_registrar_unittest.cc index 806ff12..4f9da66 100644 --- a/chrome/browser/sync/glue/sync_backend_registrar_unittest.cc +++ b/chrome/browser/sync/glue/sync_backend_registrar_unittest.cc @@ -79,7 +79,7 @@ TEST_F(SyncBackendRegistrarTest, ConstructorEmpty) { { std::vector workers; registrar.GetWorkers(&workers); - EXPECT_EQ(3u, workers.size()); + EXPECT_EQ(4u, workers.size()); } ExpectRoutingInfo(®istrar, ModelSafeRoutingInfo()); ExpectHasProcessorsForTypes(®istrar, ModelTypeSet()); @@ -98,7 +98,7 @@ TEST_F(SyncBackendRegistrarTest, ConstructorNonEmpty) { { std::vector workers; registrar.GetWorkers(&workers); - EXPECT_EQ(3u, workers.size()); + EXPECT_EQ(4u, workers.size()); } { ModelSafeRoutingInfo expected_routing_info; diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index cb4742f..4b3183c 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2074,6 +2074,8 @@ 'browser/sync/glue/bookmark_data_type_controller.h', 'browser/sync/glue/bookmark_model_associator.cc', 'browser/sync/glue/bookmark_model_associator.h', + 'browser/sync/glue/browser_thread_model_worker.cc', + 'browser/sync/glue/browser_thread_model_worker.h', 'browser/sync/glue/change_processor.cc', 'browser/sync/glue/change_processor.h', 'browser/sync/glue/data_type_controller.h', @@ -2081,8 +2083,6 @@ 'browser/sync/glue/data_type_manager.h', 'browser/sync/glue/data_type_manager_impl.cc', 'browser/sync/glue/data_type_manager_impl.h', - 'browser/sync/glue/database_model_worker.cc', - 'browser/sync/glue/database_model_worker.h', 'browser/sync/glue/do_optimistic_refresh_task.cc', 'browser/sync/glue/do_optimistic_refresh_task.h', 'browser/sync/glue/extension_data_type_controller.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 88235ff..f610189 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1467,6 +1467,7 @@ 'browser/sync/glue/autofill_model_associator_unittest.cc', 'browser/sync/glue/autofill_profile_syncable_service_unittest.cc', 'browser/sync/glue/bookmark_data_type_controller_unittest.cc', + 'browser/sync/glue/browser_thread_model_worker_unittest.cc', 'browser/sync/glue/change_processor_mock.cc', 'browser/sync/glue/change_processor_mock.h', 'browser/sync/glue/data_type_controller_mock.cc', @@ -1474,7 +1475,6 @@ 'browser/sync/glue/data_type_manager_impl_unittest.cc', 'browser/sync/glue/data_type_manager_mock.cc', 'browser/sync/glue/data_type_manager_mock.h', - 'browser/sync/glue/database_model_worker_unittest.cc', 'browser/sync/glue/extension_data_type_controller_unittest.cc', 'browser/sync/glue/frontend_data_type_controller_mock.cc', 'browser/sync/glue/frontend_data_type_controller_mock.h', -- cgit v1.1