summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorhaitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-14 22:10:46 +0000
committerhaitaol@chromium.org <haitaol@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-14 22:12:50 +0000
commitcad784beeacc9a084852de1c7202ee3fdfd408c0 (patch)
tree7575f78b73449b3a53ee2fc5e3a8698eec82178b
parent32f6bc8c638647304f333331a6a79de7019943ea (diff)
downloadchromium_src-cad784beeacc9a084852de1c7202ee3fdfd408c0.zip
chromium_src-cad784beeacc9a084852de1c7202ee3fdfd408c0.tar.gz
chromium_src-cad784beeacc9a084852de1c7202ee3fdfd408c0.tar.bz2
Let SyncBackupManager keep backup data in memory until shutdown. Only persist
backup data to disk if shutdown is due to switching to sync. BUG=362679 Review URL: https://codereview.chromium.org/455023003 Cr-Commit-Position: refs/heads/master@{#289709} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289709 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/sync/test_profile_sync_service.cc5
-rw-r--r--sync/BUILD.gn3
-rw-r--r--sync/internal_api/internal_components_factory_impl.cc17
-rw-r--r--sync/internal_api/public/internal_components_factory.h16
-rw-r--r--sync/internal_api/public/internal_components_factory_impl.h6
-rw-r--r--sync/internal_api/public/test/test_internal_components_factory.h17
-rw-r--r--sync/internal_api/sync_backup_manager.cc14
-rw-r--r--sync/internal_api/sync_backup_manager.h1
-rw-r--r--sync/internal_api/sync_backup_manager_unittest.cc41
-rw-r--r--sync/internal_api/sync_manager_impl.cc4
-rw-r--r--sync/internal_api/sync_manager_impl_unittest.cc14
-rw-r--r--sync/internal_api/sync_rollback_manager.cc1
-rw-r--r--sync/internal_api/sync_rollback_manager_base.cc10
-rw-r--r--sync/internal_api/sync_rollback_manager_base.h8
-rw-r--r--sync/internal_api/sync_rollback_manager_base_unittest.cc7
-rw-r--r--sync/internal_api/sync_rollback_manager_unittest.cc22
-rw-r--r--sync/internal_api/test/test_internal_components_factory.cc19
-rw-r--r--sync/sync.gyp2
-rw-r--r--sync/sync_tests.gypi1
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store.cc73
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store.h45
-rw-r--r--sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc114
22 files changed, 382 insertions, 58 deletions
diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc
index 6242dd9..1d46b26 100644
--- a/chrome/browser/sync/test_profile_sync_service.cc
+++ b/chrome/browser/sync/test_profile_sync_service.cc
@@ -61,8 +61,9 @@ void SyncBackendHostForProfileSyncTest::InitCore(
InternalComponentsFactory::Switches factory_switches =
options->internal_components_factory->GetSwitches();
options->internal_components_factory.reset(
- new TestInternalComponentsFactory(factory_switches,
- syncer::STORAGE_IN_MEMORY));
+ new TestInternalComponentsFactory(
+ factory_switches, InternalComponentsFactory::STORAGE_IN_MEMORY,
+ NULL));
SyncBackendHostImpl::InitCore(options.Pass());
}
diff --git a/sync/BUILD.gn b/sync/BUILD.gn
index 02d38e1..daa4d5d 100644
--- a/sync/BUILD.gn
+++ b/sync/BUILD.gn
@@ -311,6 +311,8 @@ source_set("sync_core") {
"sessions/sync_session_context.h",
"sessions/sync_session.h",
"syncable/blob.h",
+ "syncable/deferred_on_disk_directory_backing_store.cc",
+ "syncable/deferred_on_disk_directory_backing_store.h",
"syncable/directory_backing_store.cc",
"syncable/directory_backing_store.h",
"syncable/directory.cc",
@@ -615,6 +617,7 @@ test("sync_unit_tests") {
"sessions/model_type_registry_unittest.cc",
"sessions/nudge_tracker_unittest.cc",
"sessions/status_controller_unittest.cc",
+ "syncable/deferred_on_disk_directory_backing_store_unittest.cc",
"syncable/directory_backing_store_unittest.cc",
"syncable/directory_unittest.cc",
"syncable/directory_unittest.h",
diff --git a/sync/internal_api/internal_components_factory_impl.cc b/sync/internal_api/internal_components_factory_impl.cc
index e764a16..dbc4243 100644
--- a/sync/internal_api/internal_components_factory_impl.cc
+++ b/sync/internal_api/internal_components_factory_impl.cc
@@ -8,6 +8,7 @@
#include "sync/engine/syncer.h"
#include "sync/engine/sync_scheduler_impl.h"
#include "sync/sessions/sync_session_context.h"
+#include "sync/syncable/deferred_on_disk_directory_backing_store.h"
#include "sync/syncable/on_disk_directory_backing_store.h"
using base::TimeDelta;
@@ -59,9 +60,19 @@ InternalComponentsFactoryImpl::BuildContext(
scoped_ptr<syncable::DirectoryBackingStore>
InternalComponentsFactoryImpl::BuildDirectoryBackingStore(
- const std::string& dir_name, const base::FilePath& backing_filepath) {
- return scoped_ptr<syncable::DirectoryBackingStore>(
- new syncable::OnDiskDirectoryBackingStore(dir_name, backing_filepath));
+ StorageOption storage, const std::string& dir_name,
+ const base::FilePath& backing_filepath) {
+ if (storage == STORAGE_ON_DISK) {
+ return scoped_ptr<syncable::DirectoryBackingStore>(
+ new syncable::OnDiskDirectoryBackingStore(dir_name, backing_filepath));
+ } else if (storage == STORAGE_ON_DISK_DEFERRED) {
+ return scoped_ptr<syncable::DirectoryBackingStore>(
+ new syncable::DeferredOnDiskDirectoryBackingStore(dir_name,
+ backing_filepath));
+ } else {
+ NOTREACHED();
+ return scoped_ptr<syncable::DirectoryBackingStore>();
+ }
}
InternalComponentsFactory::Switches
diff --git a/sync/internal_api/public/internal_components_factory.h b/sync/internal_api/public/internal_components_factory.h
index f43804a..f077af0 100644
--- a/sync/internal_api/public/internal_components_factory.h
+++ b/sync/internal_api/public/internal_components_factory.h
@@ -72,6 +72,21 @@ class SYNC_EXPORT InternalComponentsFactory {
PreCommitUpdatesPolicy pre_commit_updates_policy;
};
+ // For selecting the types of storage to use to persist sync data when
+ // BuildDirectoryBackingStore() is called.
+ enum StorageOption {
+ // BuildDirectoryBackingStore should not use persistent on-disk storage.
+ STORAGE_IN_MEMORY,
+ // Use this if you want BuildDirectoryBackingStore to create/use a real
+ // on disk store.
+ STORAGE_ON_DISK,
+ // Use this to defer creating on-disk database until
+ // DirectoryBackingStore::SaveChanges() is called.
+ STORAGE_ON_DISK_DEFERRED,
+ // Use this to test the case where a directory fails to load.
+ STORAGE_INVALID
+ };
+
virtual ~InternalComponentsFactory() {}
virtual scoped_ptr<SyncScheduler> BuildScheduler(
@@ -90,6 +105,7 @@ class SYNC_EXPORT InternalComponentsFactory {
virtual scoped_ptr<syncable::DirectoryBackingStore>
BuildDirectoryBackingStore(
+ StorageOption storage,
const std::string& dir_name,
const base::FilePath& backing_filepath) = 0;
diff --git a/sync/internal_api/public/internal_components_factory_impl.h b/sync/internal_api/public/internal_components_factory_impl.h
index 7c364dc..c727509 100644
--- a/sync/internal_api/public/internal_components_factory_impl.h
+++ b/sync/internal_api/public/internal_components_factory_impl.h
@@ -34,9 +34,9 @@ class SYNC_EXPORT InternalComponentsFactoryImpl
const std::string& invalidator_client_id) OVERRIDE;
virtual scoped_ptr<syncable::DirectoryBackingStore>
- BuildDirectoryBackingStore(
- const std::string& dir_name,
- const base::FilePath& backing_filepath) OVERRIDE;
+ BuildDirectoryBackingStore(StorageOption storage,
+ const std::string& dir_name,
+ const base::FilePath& backing_filepath) OVERRIDE;
virtual Switches GetSwitches() const OVERRIDE;
diff --git a/sync/internal_api/public/test/test_internal_components_factory.h b/sync/internal_api/public/test/test_internal_components_factory.h
index 3b7d3c5..0c6b909 100644
--- a/sync/internal_api/public/test/test_internal_components_factory.h
+++ b/sync/internal_api/public/test/test_internal_components_factory.h
@@ -9,20 +9,11 @@
namespace syncer {
-enum StorageOption {
- // BuildDirectoryBackingStore should not use persistent on-disk storage.
- STORAGE_IN_MEMORY,
- // Use this if you want BuildDirectoryBackingStore to create a real
- // on disk store.
- STORAGE_ON_DISK,
- // Use this to test the case where a directory fails to load.
- STORAGE_INVALID
-};
-
class TestInternalComponentsFactory : public InternalComponentsFactory {
public:
explicit TestInternalComponentsFactory(const Switches& switches,
- StorageOption option);
+ StorageOption option,
+ StorageOption* storage_used);
virtual ~TestInternalComponentsFactory();
virtual scoped_ptr<SyncScheduler> BuildScheduler(
@@ -41,6 +32,7 @@ class TestInternalComponentsFactory : public InternalComponentsFactory {
virtual scoped_ptr<syncable::DirectoryBackingStore>
BuildDirectoryBackingStore(
+ StorageOption storage,
const std::string& dir_name,
const base::FilePath& backing_filepath) OVERRIDE;
@@ -48,7 +40,8 @@ class TestInternalComponentsFactory : public InternalComponentsFactory {
private:
const Switches switches_;
- const StorageOption storage_option_;
+ const StorageOption storage_override_;
+ StorageOption* storage_used_;
DISALLOW_COPY_AND_ASSIGN(TestInternalComponentsFactory);
};
diff --git a/sync/internal_api/sync_backup_manager.cc b/sync/internal_api/sync_backup_manager.cc
index be8b872..d626229 100644
--- a/sync/internal_api/sync_backup_manager.cc
+++ b/sync/internal_api/sync_backup_manager.cc
@@ -23,6 +23,7 @@ void SyncBackupManager::Init(InitArgs* args) {
if (SyncRollbackManagerBase::InitInternal(
args->database_location,
args->internal_components_factory.get(),
+ InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED,
args->unrecoverable_error_handler.Pass(),
args->report_unrecoverable_error_function)) {
GetUserShare()->directory->CollectMetaHandleCounts(
@@ -34,10 +35,8 @@ void SyncBackupManager::Init(InitArgs* args) {
}
void SyncBackupManager::SaveChanges() {
- if (initialized()) {
+ if (initialized())
NormalizeEntries();
- GetUserShare()->directory->SaveChanges();
- }
}
SyncStatus SyncBackupManager::GetDetailedStatus() const {
@@ -122,6 +121,15 @@ void SyncBackupManager::HideSyncPreference(ModelType type) {
}
}
+void SyncBackupManager::ShutdownOnSyncThread(ShutdownReason reason) {
+ if (reason == SWITCH_MODE_SYNC) {
+ NormalizeEntries();
+ GetUserShare()->directory->SaveChanges();
+ }
+
+ SyncRollbackManagerBase::ShutdownOnSyncThread(reason);
+}
+
void SyncBackupManager::RegisterDirectoryTypeDebugInfoObserver(
syncer::TypeDebugInfoObserver* observer) {}
diff --git a/sync/internal_api/sync_backup_manager.h b/sync/internal_api/sync_backup_manager.h
index f8fa1aa..9a9fdbc 100644
--- a/sync/internal_api/sync_backup_manager.h
+++ b/sync/internal_api/sync_backup_manager.h
@@ -24,6 +24,7 @@ class SYNC_EXPORT_PRIVATE SyncBackupManager : public SyncRollbackManagerBase {
virtual void Init(InitArgs* args) OVERRIDE;
virtual void SaveChanges() OVERRIDE;
virtual SyncStatus GetDetailedStatus() const OVERRIDE;
+ virtual void ShutdownOnSyncThread(ShutdownReason reason) OVERRIDE;
// DirectoryChangeDelegate implementation.
virtual ModelTypeSet HandleTransactionEndingChangeEvent(
diff --git a/sync/internal_api/sync_backup_manager_unittest.cc b/sync/internal_api/sync_backup_manager_unittest.cc
index e771ea3..7c6c568 100644
--- a/sync/internal_api/sync_backup_manager_unittest.cc
+++ b/sync/internal_api/sync_backup_manager_unittest.cc
@@ -49,7 +49,8 @@ class SyncBackupManagerTest : public syncer::SyncManager::Observer,
CHECK(temp_dir_.CreateUniqueTempDir());
}
- void InitManager(SyncManager* manager, StorageOption storage_option) {
+ void InitManager(SyncManager* manager,
+ InternalComponentsFactory::StorageOption storage_option) {
manager_ = manager;
EXPECT_CALL(*this, OnInitializationComplete(_, _, _, _))
.WillOnce(WithArgs<2>(Invoke(this,
@@ -65,8 +66,11 @@ class SyncBackupManagerTest : public syncer::SyncManager::Observer,
args.service_url = GURL("https://example.com/");
args.post_factory = scoped_ptr<HttpPostProviderFactory>().Pass();
args.internal_components_factory.reset(new TestInternalComponentsFactory(
- InternalComponentsFactory::Switches(), storage_option));
+ InternalComponentsFactory::Switches(), storage_option,
+ &storage_used_));
manager->Init(&args);
+ EXPECT_EQ(InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED,
+ storage_used_);
loop_.PostTask(FROM_HERE, run_loop.QuitClosure());
run_loop.Run();
}
@@ -82,7 +86,6 @@ class SyncBackupManagerTest : public syncer::SyncManager::Observer,
node.InitUniqueByCreation(type, type_root, client_tag));
}
- private:
void ConfigureSyncer() {
manager_->ConfigureSyncer(CONFIGURE_REASON_NEW_CLIENT,
ModelTypeSet(SEARCH_ENGINES),
@@ -105,11 +108,12 @@ class SyncBackupManagerTest : public syncer::SyncManager::Observer,
base::ScopedTempDir temp_dir_;
base::MessageLoop loop_; // Needed for WeakHandle
SyncManager* manager_;
+ InternalComponentsFactory::StorageOption storage_used_;
};
-TEST_F(SyncBackupManagerTest, NormalizeAndPersist) {
+TEST_F(SyncBackupManagerTest, NormalizeEntry) {
scoped_ptr<SyncBackupManager> manager(new SyncBackupManager);
- InitManager(manager.get(), STORAGE_ON_DISK);
+ InitManager(manager.get(), InternalComponentsFactory::STORAGE_IN_MEMORY);
CreateEntry(manager->GetUserShare(), SEARCH_ENGINES, "test");
@@ -134,11 +138,20 @@ TEST_F(SyncBackupManagerTest, NormalizeAndPersist) {
EXPECT_TRUE(pref.GetEntry()->GetId().ServerKnows());
EXPECT_FALSE(pref.GetEntry()->GetIsUnsynced());
}
- manager->ShutdownOnSyncThread(STOP_SYNC);
+}
+
+TEST_F(SyncBackupManagerTest, PersistWithSwitchToSyncShutdown) {
+ scoped_ptr<SyncBackupManager> manager(new SyncBackupManager);
+ InitManager(manager.get(),
+ InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED);
+
+ CreateEntry(manager->GetUserShare(), SEARCH_ENGINES, "test");
+ manager->SaveChanges();
+ manager->ShutdownOnSyncThread(SWITCH_MODE_SYNC);
// Reopen db to verify entry is persisted.
manager.reset(new SyncBackupManager);
- InitManager(manager.get(), STORAGE_ON_DISK);
+ InitManager(manager.get(), InternalComponentsFactory::STORAGE_ON_DISK);
{
ReadTransaction trans(FROM_HERE, manager->GetUserShare());
ReadNode pref(&trans);
@@ -149,10 +162,22 @@ TEST_F(SyncBackupManagerTest, NormalizeAndPersist) {
}
}
+TEST_F(SyncBackupManagerTest, DontPersistWithOtherShutdown) {
+ scoped_ptr<SyncBackupManager> manager(new SyncBackupManager);
+ InitManager(manager.get(),
+ InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED);
+
+ CreateEntry(manager->GetUserShare(), SEARCH_ENGINES, "test");
+ manager->SaveChanges();
+ manager->ShutdownOnSyncThread(STOP_SYNC);
+ EXPECT_FALSE(base::PathExists(
+ temp_dir_.path().Append(syncable::Directory::kSyncDatabaseFilename)));
+}
+
TEST_F(SyncBackupManagerTest, FailToInitialize) {
// Test graceful shutdown on initialization failure.
scoped_ptr<SyncBackupManager> manager(new SyncBackupManager);
- InitManager(manager.get(), STORAGE_INVALID);
+ InitManager(manager.get(), InternalComponentsFactory::STORAGE_INVALID);
}
} // anonymous namespace
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 30bbafd..8c8894c 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -344,8 +344,8 @@ void SyncManagerImpl::Init(InitArgs* args) {
scoped_ptr<syncable::DirectoryBackingStore> backing_store =
args->internal_components_factory->BuildDirectoryBackingStore(
- args->credentials.email,
- absolute_db_path).Pass();
+ InternalComponentsFactory::STORAGE_ON_DISK,
+ args->credentials.email, absolute_db_path).Pass();
DCHECK(backing_store.get());
share_.directory.reset(
diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc
index 5518d4f..f7c700e 100644
--- a/sync/internal_api/sync_manager_impl_unittest.cc
+++ b/sync/internal_api/sync_manager_impl_unittest.cc
@@ -834,6 +834,8 @@ class SyncManagerTest : public testing::Test,
sync_manager_.GetEncryptionHandler()->AddObserver(&encryption_observer_);
EXPECT_TRUE(js_backend_.IsInitialized());
+ EXPECT_EQ(InternalComponentsFactory::STORAGE_ON_DISK,
+ storage_used_);
if (initialization_succeeded_) {
for (ModelSafeRoutingInfo::iterator i = routing_info.begin();
@@ -951,7 +953,9 @@ class SyncManagerTest : public testing::Test,
}
virtual InternalComponentsFactory* GetFactory() {
- return new TestInternalComponentsFactory(GetSwitches(), STORAGE_IN_MEMORY);
+ return new TestInternalComponentsFactory(
+ GetSwitches(), InternalComponentsFactory::STORAGE_IN_MEMORY,
+ &storage_used_);
}
// Returns true if we are currently encrypting all sync data. May
@@ -1011,6 +1015,7 @@ class SyncManagerTest : public testing::Test,
StrictMock<SyncManagerObserverMock> manager_observer_;
StrictMock<SyncEncryptionHandlerObserverMock> encryption_observer_;
InternalComponentsFactory::Switches switches_;
+ InternalComponentsFactory::StorageOption storage_used_;
};
TEST_F(SyncManagerTest, GetAllNodesForTypeTest) {
@@ -2410,7 +2415,8 @@ class ComponentsFactory : public TestInternalComponentsFactory {
ComponentsFactory(const Switches& switches,
SyncScheduler* scheduler_to_use,
sessions::SyncSessionContext** session_context)
- : TestInternalComponentsFactory(switches, syncer::STORAGE_IN_MEMORY),
+ : TestInternalComponentsFactory(
+ switches, InternalComponentsFactory::STORAGE_IN_MEMORY, NULL),
scheduler_to_use_(scheduler_to_use),
session_context_(session_context) {}
virtual ~ComponentsFactory() {}
@@ -3177,7 +3183,9 @@ class SyncManagerInitInvalidStorageTest : public SyncManagerTest {
}
virtual InternalComponentsFactory* GetFactory() OVERRIDE {
- return new TestInternalComponentsFactory(GetSwitches(), STORAGE_INVALID);
+ return new TestInternalComponentsFactory(
+ GetSwitches(), InternalComponentsFactory::STORAGE_INVALID,
+ &storage_used_);
}
};
diff --git a/sync/internal_api/sync_rollback_manager.cc b/sync/internal_api/sync_rollback_manager.cc
index b4100c7..e7b8ea6 100644
--- a/sync/internal_api/sync_rollback_manager.cc
+++ b/sync/internal_api/sync_rollback_manager.cc
@@ -26,6 +26,7 @@ void SyncRollbackManager::Init(InitArgs* args) {
if (SyncRollbackManagerBase::InitInternal(
args->database_location,
args->internal_components_factory.get(),
+ InternalComponentsFactory::STORAGE_ON_DISK,
args->unrecoverable_error_handler.Pass(),
args->report_unrecoverable_error_function)) {
change_delegate_ = args->change_delegate;
diff --git a/sync/internal_api/sync_rollback_manager_base.cc b/sync/internal_api/sync_rollback_manager_base.cc
index 67e152b..a2f14e4 100644
--- a/sync/internal_api/sync_rollback_manager_base.cc
+++ b/sync/internal_api/sync_rollback_manager_base.cc
@@ -5,7 +5,6 @@
#include "sync/internal_api/sync_rollback_manager_base.h"
#include "sync/internal_api/public/base/model_type.h"
-#include "sync/internal_api/public/internal_components_factory.h"
#include "sync/internal_api/public/read_node.h"
#include "sync/internal_api/public/read_transaction.h"
#include "sync/internal_api/public/write_transaction.h"
@@ -53,12 +52,13 @@ SyncRollbackManagerBase::~SyncRollbackManagerBase() {
bool SyncRollbackManagerBase::InitInternal(
const base::FilePath& database_location,
InternalComponentsFactory* internal_components_factory,
+ InternalComponentsFactory::StorageOption storage,
scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {
unrecoverable_error_handler_ = unrecoverable_error_handler.Pass();
report_unrecoverable_error_function_ = report_unrecoverable_error_function;
- if (!InitBackupDB(database_location, internal_components_factory)) {
+ if (!InitBackupDB(database_location, internal_components_factory, storage)) {
NotifyInitializationFailure();
return false;
}
@@ -140,7 +140,6 @@ void SyncRollbackManagerBase::SaveChanges() {
void SyncRollbackManagerBase::ShutdownOnSyncThread(ShutdownReason reason) {
if (initialized_) {
- SaveChanges();
share_.directory->Close();
share_.directory.reset();
initialized_ = false;
@@ -239,12 +238,13 @@ scoped_ptr<base::ListValue> SyncRollbackManagerBase::GetAllNodesForType(
bool SyncRollbackManagerBase::InitBackupDB(
const base::FilePath& sync_folder,
- InternalComponentsFactory* internal_components_factory) {
+ InternalComponentsFactory* internal_components_factory,
+ InternalComponentsFactory::StorageOption storage) {
base::FilePath backup_db_path = sync_folder.Append(
syncable::Directory::kSyncDatabaseFilename);
scoped_ptr<syncable::DirectoryBackingStore> backing_store =
internal_components_factory->BuildDirectoryBackingStore(
- "backup", backup_db_path).Pass();
+ storage, "backup", backup_db_path).Pass();
DCHECK(backing_store.get());
share_.directory.reset(
diff --git a/sync/internal_api/sync_rollback_manager_base.h b/sync/internal_api/sync_rollback_manager_base.h
index b6ffcd7..b92cafb 100644
--- a/sync/internal_api/sync_rollback_manager_base.h
+++ b/sync/internal_api/sync_rollback_manager_base.h
@@ -10,6 +10,7 @@
#include "sync/base/sync_export.h"
#include "sync/internal_api/public/http_post_provider_factory.h"
+#include "sync/internal_api/public/internal_components_factory.h"
#include "sync/internal_api/public/sync_manager.h"
#include "sync/internal_api/public/user_share.h"
#include "sync/syncable/directory_change_delegate.h"
@@ -99,6 +100,7 @@ class SYNC_EXPORT_PRIVATE SyncRollbackManagerBase :
bool InitInternal(
const base::FilePath& database_location,
InternalComponentsFactory* internal_components_factory,
+ InternalComponentsFactory::StorageOption storage,
scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler,
ReportUnrecoverableErrorFunction report_unrecoverable_error_function);
@@ -118,9 +120,9 @@ class SYNC_EXPORT_PRIVATE SyncRollbackManagerBase :
void NotifyInitializationSuccess();
void NotifyInitializationFailure();
- bool InitBackupDB(
- const base::FilePath& sync_folder,
- InternalComponentsFactory* internal_components_factory);
+ bool InitBackupDB(const base::FilePath& sync_folder,
+ InternalComponentsFactory* internal_components_factory,
+ InternalComponentsFactory::StorageOption storage);
bool InitTypeRootNode(ModelType type);
void InitBookmarkFolder(const std::string& folder);
diff --git a/sync/internal_api/sync_rollback_manager_base_unittest.cc b/sync/internal_api/sync_rollback_manager_base_unittest.cc
index a0866ea..95662f6 100644
--- a/sync/internal_api/sync_rollback_manager_base_unittest.cc
+++ b/sync/internal_api/sync_rollback_manager_base_unittest.cc
@@ -25,6 +25,7 @@ class SyncTestRollbackManager : public SyncRollbackManagerBase {
SyncRollbackManagerBase::InitInternal(
args->database_location,
args->internal_components_factory.get(),
+ InternalComponentsFactory::STORAGE_IN_MEMORY,
args->unrecoverable_error_handler.Pass(),
args->report_unrecoverable_error_function);
}
@@ -37,12 +38,16 @@ class SyncRollbackManagerBaseTest : public testing::Test {
args.database_location = base::FilePath(base::FilePath::kCurrentDirectory);
args.service_url = GURL("https://example.com/");
args.internal_components_factory.reset(new TestInternalComponentsFactory(
- InternalComponentsFactory::Switches(), STORAGE_IN_MEMORY));
+ InternalComponentsFactory::Switches(),
+ InternalComponentsFactory::STORAGE_IN_MEMORY,
+ &storage_used_));
manager_.Init(&args);
+ EXPECT_EQ(InternalComponentsFactory::STORAGE_IN_MEMORY, storage_used_);
}
SyncTestRollbackManager manager_;
base::MessageLoop loop_; // Needed for WeakHandle
+ InternalComponentsFactory::StorageOption storage_used_;
};
TEST_F(SyncRollbackManagerBaseTest, InitTypeOnConfiguration) {
diff --git a/sync/internal_api/sync_rollback_manager_unittest.cc b/sync/internal_api/sync_rollback_manager_unittest.cc
index b243552..ebf9fed 100644
--- a/sync/internal_api/sync_rollback_manager_unittest.cc
+++ b/sync/internal_api/sync_rollback_manager_unittest.cc
@@ -102,7 +102,8 @@ class SyncRollbackManagerTest : public testing::Test,
}
void InitManager(SyncManager* manager, ModelTypeSet types,
- TestChangeDelegate* delegate, StorageOption storage_option) {
+ TestChangeDelegate* delegate,
+ InternalComponentsFactory::StorageOption storage_option) {
manager_ = manager;
types_ = types;
@@ -118,9 +119,12 @@ class SyncRollbackManagerTest : public testing::Test,
args.service_url = GURL("https://example.com/");
args.workers.push_back(worker_);
args.change_delegate = delegate;
+
+ InternalComponentsFactory::StorageOption storage_used;
args.internal_components_factory.reset(new TestInternalComponentsFactory(
- InternalComponentsFactory::Switches(), storage_option));
+ InternalComponentsFactory::Switches(), storage_option, &storage_used));
manager->Init(&args);
+ EXPECT_EQ(storage_option, storage_used);
loop_.PostTask(FROM_HERE, run_loop.QuitClosure());
run_loop.Run();
}
@@ -130,9 +134,9 @@ class SyncRollbackManagerTest : public testing::Test,
SyncBackupManager backup_manager;
TestChangeDelegate delegate;
InitManager(&backup_manager, ModelTypeSet(type), &delegate,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED);
CreateEntry(backup_manager.GetUserShare(), type, client_tag);
- backup_manager.ShutdownOnSyncThread(STOP_SYNC);
+ backup_manager.ShutdownOnSyncThread(SWITCH_MODE_SYNC);
}
// Verify entry with |client_tag| exists in sync directory.
@@ -183,7 +187,7 @@ TEST_F(SyncRollbackManagerTest, RollbackBasic) {
TestChangeDelegate delegate;
SyncRollbackManager rollback_manager;
InitManager(&rollback_manager, ModelTypeSet(PREFERENCES), &delegate,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK);
// Simulate a new entry added during type initialization.
int64 new_pref_id =
@@ -207,7 +211,7 @@ TEST_F(SyncRollbackManagerTest, NoRollbackOfTypesNotBackedUp) {
TestChangeDelegate delegate;
SyncRollbackManager rollback_manager;
InitManager(&rollback_manager, ModelTypeSet(PREFERENCES, APPS), &delegate,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK);
// Simulate new entry added during type initialization.
int64 new_pref_id =
@@ -235,7 +239,7 @@ TEST_F(SyncRollbackManagerTest, BackupDbNotChangedOnAbort) {
scoped_ptr<SyncRollbackManager> rollback_manager(
new SyncRollbackManager);
InitManager(rollback_manager.get(), ModelTypeSet(PREFERENCES), &delegate,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK);
// Simulate a new entry added during type initialization.
CreateEntry(rollback_manager->GetUserShare(), PREFERENCES, "pref2");
@@ -246,7 +250,7 @@ TEST_F(SyncRollbackManagerTest, BackupDbNotChangedOnAbort) {
// Verify new entry was not persisted.
rollback_manager.reset(new SyncRollbackManager);
InitManager(rollback_manager.get(), ModelTypeSet(PREFERENCES), &delegate,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK);
EXPECT_FALSE(VerifyEntry(rollback_manager->GetUserShare(), PREFERENCES,
"pref2"));
}
@@ -256,7 +260,7 @@ TEST_F(SyncRollbackManagerTest, OnInitializationFailure) {
scoped_ptr<SyncRollbackManager> rollback_manager(
new SyncRollbackManager);
InitManager(rollback_manager.get(), ModelTypeSet(PREFERENCES), NULL,
- STORAGE_ON_DISK);
+ InternalComponentsFactory::STORAGE_ON_DISK);
}
} // anonymous namespace
diff --git a/sync/internal_api/test/test_internal_components_factory.cc b/sync/internal_api/test/test_internal_components_factory.cc
index c7d94e9..34c678b 100644
--- a/sync/internal_api/test/test_internal_components_factory.cc
+++ b/sync/internal_api/test/test_internal_components_factory.cc
@@ -5,6 +5,7 @@
#include "sync/internal_api/public/test/test_internal_components_factory.h"
#include "sync/sessions/sync_session_context.h"
+#include "sync/syncable/deferred_on_disk_directory_backing_store.h"
#include "sync/syncable/in_memory_directory_backing_store.h"
#include "sync/syncable/on_disk_directory_backing_store.h"
#include "sync/syncable/invalid_directory_backing_store.h"
@@ -14,9 +15,11 @@ namespace syncer {
TestInternalComponentsFactory::TestInternalComponentsFactory(
const Switches& switches,
- StorageOption option)
+ StorageOption option,
+ StorageOption* storage_used)
: switches_(switches),
- storage_option_(option) {
+ storage_override_(option),
+ storage_used_(storage_used) {
}
TestInternalComponentsFactory::~TestInternalComponentsFactory() { }
@@ -54,8 +57,12 @@ TestInternalComponentsFactory::BuildContext(
scoped_ptr<syncable::DirectoryBackingStore>
TestInternalComponentsFactory::BuildDirectoryBackingStore(
- const std::string& dir_name, const base::FilePath& backing_filepath) {
- switch (storage_option_) {
+ StorageOption storage, const std::string& dir_name,
+ const base::FilePath& backing_filepath) {
+ if (storage_used_)
+ *storage_used_ = storage;
+
+ switch (storage_override_) {
case STORAGE_IN_MEMORY:
return scoped_ptr<syncable::DirectoryBackingStore>(
new syncable::InMemoryDirectoryBackingStore(dir_name));
@@ -63,6 +70,10 @@ TestInternalComponentsFactory::BuildDirectoryBackingStore(
return scoped_ptr<syncable::DirectoryBackingStore>(
new syncable::OnDiskDirectoryBackingStore(dir_name,
backing_filepath));
+ case STORAGE_ON_DISK_DEFERRED:
+ return scoped_ptr<syncable::DirectoryBackingStore>(
+ new syncable::DeferredOnDiskDirectoryBackingStore(dir_name,
+ backing_filepath));
case STORAGE_INVALID:
return scoped_ptr<syncable::DirectoryBackingStore>(
new syncable::InvalidDirectoryBackingStore());
diff --git a/sync/sync.gyp b/sync/sync.gyp
index ad8fd34..9ced955 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -353,6 +353,8 @@
'sessions/sync_session_context.cc',
'sessions/sync_session_context.h',
'syncable/blob.h',
+ 'syncable/deferred_on_disk_directory_backing_store.cc',
+ 'syncable/deferred_on_disk_directory_backing_store.h',
'syncable/dir_open_result.h',
'syncable/directory.cc',
'syncable/directory.h',
diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi
index b7b68c3..ddcdf25 100644
--- a/sync/sync_tests.gypi
+++ b/sync/sync_tests.gypi
@@ -317,6 +317,7 @@
'sessions/model_type_registry_unittest.cc',
'sessions/nudge_tracker_unittest.cc',
'sessions/status_controller_unittest.cc',
+ 'syncable/deferred_on_disk_directory_backing_store_unittest.cc',
'syncable/directory_backing_store_unittest.cc',
'syncable/directory_unittest.cc',
'syncable/directory_unittest.h',
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.cc b/sync/syncable/deferred_on_disk_directory_backing_store.cc
new file mode 100644
index 0000000..0367e5b
--- /dev/null
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.cc
@@ -0,0 +1,73 @@
+// Copyright 2014 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 "sync/syncable/deferred_on_disk_directory_backing_store.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/stl_util.h"
+#include "sync/syncable/syncable-inl.h"
+
+namespace syncer {
+namespace syncable {
+
+DeferredOnDiskDirectoryBackingStore::DeferredOnDiskDirectoryBackingStore(
+ const std::string& dir_name, const base::FilePath& backing_filepath)
+ : DirectoryBackingStore(dir_name),
+ backing_filepath_(backing_filepath),
+ db_is_on_disk_(false) {
+}
+
+DeferredOnDiskDirectoryBackingStore::~DeferredOnDiskDirectoryBackingStore() {}
+
+bool DeferredOnDiskDirectoryBackingStore::SaveChanges(
+ const Directory::SaveChangesSnapshot& snapshot) {
+ DCHECK(CalledOnValidThread());
+
+ // Back out early if there is nothing to save.
+ if (snapshot.dirty_metas.empty() && snapshot.metahandles_to_purge.empty() &&
+ snapshot.delete_journals.empty() &&
+ snapshot.delete_journals_to_purge.empty()) {
+ return true;
+ }
+
+ if (!db_is_on_disk_) {
+ if (!base::DeleteFile(backing_filepath_, false))
+ return false;
+
+ // Reopen DB on disk.
+ db_.reset(new sql::Connection);
+ db_->set_exclusive_locking();
+ db_->set_page_size(4096);
+ if (!db_->Open(backing_filepath_) || !InitializeTables())
+ return false;
+
+ db_is_on_disk_ = true;
+ }
+
+ return DirectoryBackingStore::SaveChanges(snapshot);
+}
+
+DirOpenResult DeferredOnDiskDirectoryBackingStore::Load(
+ Directory::MetahandlesMap* handles_map,
+ JournalIndex* delete_journals,
+ Directory::KernelLoadInfo* kernel_load_info) {
+ // Open an in-memory database at first to create initial sync data needed by
+ // Directory.
+ CHECK(!db_->is_open());
+ if (!db_->OpenInMemory())
+ return FAILED_OPEN_DATABASE;
+
+ if (!InitializeTables())
+ return FAILED_OPEN_DATABASE;
+ if (!LoadEntries(handles_map))
+ return FAILED_DATABASE_CORRUPT;
+ if (!LoadInfo(kernel_load_info))
+ return FAILED_DATABASE_CORRUPT;
+
+ return OPENED;
+}
+
+} // namespace syncable
+} // namespace syncer
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.h b/sync/syncable/deferred_on_disk_directory_backing_store.h
new file mode 100644
index 0000000..9e6429a
--- /dev/null
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.h
@@ -0,0 +1,45 @@
+// Copyright 2014 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 SYNC_SYNCABLE_DEFERRED_ON_DISK_DIRECTORY_BACKING_STORE_H_
+#define SYNC_SYNCABLE_DEFERRED_ON_DISK_DIRECTORY_BACKING_STORE_H_
+
+#include "base/files/file_path.h"
+#include "sync/base/sync_export.h"
+#include "sync/syncable/directory_backing_store.h"
+
+namespace syncer {
+namespace syncable {
+
+// Store used for backing up user's data before first sync. It creates an
+// in-memory store first and switch to on-disk store if SaveChanges() is
+// called, which only happens when SyncBackupManager is shut down and a
+// syncing backend is to be created. Thus we guarantee that user data is not
+// persisted until user is actually going to sync.
+class SYNC_EXPORT_PRIVATE DeferredOnDiskDirectoryBackingStore
+ : public DirectoryBackingStore {
+ public:
+ DeferredOnDiskDirectoryBackingStore(const std::string& dir_name,
+ const base::FilePath& backing_filepath);
+ virtual ~DeferredOnDiskDirectoryBackingStore();
+ virtual DirOpenResult Load(
+ Directory::MetahandlesMap* handles_map,
+ JournalIndex* delete_journals,
+ Directory::KernelLoadInfo* kernel_load_info) OVERRIDE;
+ virtual bool SaveChanges(const Directory::SaveChangesSnapshot& snapshot)
+ OVERRIDE;
+
+ private:
+ base::FilePath backing_filepath_;
+
+ // Whether current DB is on disk.
+ bool db_is_on_disk_;
+
+ DISALLOW_COPY_AND_ASSIGN(DeferredOnDiskDirectoryBackingStore);
+};
+
+} // namespace syncable
+} // namespace syncer
+
+#endif // SYNC_SYNCABLE_DEFERRED_ON_DISK_DIRECTORY_BACKING_STORE_H_
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc b/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc
new file mode 100644
index 0000000..b1053b8
--- /dev/null
+++ b/sync/syncable/deferred_on_disk_directory_backing_store_unittest.cc
@@ -0,0 +1,114 @@
+// Copyright 2014 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/files/scoped_temp_dir.h"
+#include "base/stl_util.h"
+#include "sync/syncable/deferred_on_disk_directory_backing_store.h"
+#include "sync/syncable/directory.h"
+#include "sync/syncable/entry_kernel.h"
+#include "sync/syncable/on_disk_directory_backing_store.h"
+#include "sync/syncable/syncable_delete_journal.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+namespace syncable {
+namespace {
+
+static const base::FilePath::CharType kSyncDataFolderName[] =
+ FILE_PATH_LITERAL("Sync Data");
+
+class DeferredOnDiskDirectoryBackingStoreTest : public testing::Test {
+ protected:
+ virtual void SetUp() OVERRIDE {
+ CHECK(temp_dir_.CreateUniqueTempDir());
+ db_path_ = temp_dir_.path().Append(kSyncDataFolderName);
+ }
+
+ virtual void TearDown() OVERRIDE {
+ STLDeleteValues(&handles_map_);
+ }
+
+ base::ScopedTempDir temp_dir_;
+ base::FilePath db_path_;
+ Directory::MetahandlesMap handles_map_;
+ JournalIndex delete_journals_;
+ Directory::KernelLoadInfo kernel_load_info_;
+};
+
+// Test initialization of root entry when calling Load().
+TEST_F(DeferredOnDiskDirectoryBackingStoreTest, Load) {
+ DeferredOnDiskDirectoryBackingStore store("test", db_path_);
+ EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
+ &kernel_load_info_));
+ EXPECT_TRUE(delete_journals_.empty());
+ ASSERT_EQ(1u, handles_map_.size()); // root node
+ ASSERT_TRUE(handles_map_.count(1));
+ EntryKernel* root = handles_map_[1];
+ EXPECT_TRUE(root->ref(ID).IsRoot());
+ EXPECT_EQ(1, root->ref(META_HANDLE));
+ EXPECT_TRUE(root->ref(IS_DIR));
+}
+
+// Test on-disk DB is not created if SaveChanges() is not called.
+TEST_F(DeferredOnDiskDirectoryBackingStoreTest,
+ DontPersistIfSavingChangesNotCalled) {
+ {
+ // Open and close.
+ DeferredOnDiskDirectoryBackingStore store("test", db_path_);
+ EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
+ &kernel_load_info_));
+ }
+
+ EXPECT_FALSE(base::PathExists(db_path_));
+}
+
+// Test on-disk DB is not created when there are no changes.
+TEST_F(DeferredOnDiskDirectoryBackingStoreTest,
+ DontPersistWhenSavingNoChanges) {
+ {
+ // Open and close.
+ DeferredOnDiskDirectoryBackingStore store("test", db_path_);
+ EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
+ &kernel_load_info_));
+
+ Directory::SaveChangesSnapshot snapshot;
+ store.SaveChanges(snapshot);
+ }
+
+ EXPECT_FALSE(base::PathExists(db_path_));
+}
+
+// Test changes are persisted to disk when SaveChanges() is called.
+TEST_F(DeferredOnDiskDirectoryBackingStoreTest, PersistWhenSavingValidChanges) {
+ {
+ // Open and close.
+ DeferredOnDiskDirectoryBackingStore store("test", db_path_);
+ EXPECT_EQ(OPENED, store.Load(&handles_map_, &delete_journals_,
+ &kernel_load_info_));
+
+ Directory::SaveChangesSnapshot snapshot;
+ EntryKernel* entry = new EntryKernel();
+ entry->put(ID, Id::CreateFromClientString("test_entry"));
+ entry->put(META_HANDLE, 2);
+ entry->mark_dirty(NULL);
+ snapshot.dirty_metas.insert(entry);
+ store.SaveChanges(snapshot);
+ }
+
+ STLDeleteValues(&handles_map_);
+
+ ASSERT_TRUE(base::PathExists(db_path_));
+ OnDiskDirectoryBackingStore read_store("test", db_path_);
+ EXPECT_EQ(OPENED, read_store.Load(&handles_map_, &delete_journals_,
+ &kernel_load_info_));
+ ASSERT_EQ(2u, handles_map_.size());
+ ASSERT_TRUE(handles_map_.count(1)); // root node
+ ASSERT_TRUE(handles_map_.count(2));
+ EntryKernel* entry = handles_map_[2];
+ EXPECT_EQ(Id::CreateFromClientString("test_entry"), entry->ref(ID));
+}
+
+} // namespace
+} // namespace syncable
+} // namespace syncer