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

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289709

Review URL: https://codereview.chromium.org/455023003

Cr-Commit-Position: refs/heads/master@{#289840}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289840 0039d316-1c4b-4281-b951-d872f2087c98

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());
 }

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",

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

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;
 

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;
 

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);
 };

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) {}
 

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(

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

sync/internal_api/sync_manager_impl.cc

@@ -345,8 +345,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(

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) {
@@ -2409,8 +2414,10 @@ class ComponentsFactory : public TestInternalComponentsFactory {
  public:
   ComponentsFactory(const Switches& switches,
                     SyncScheduler* scheduler_to_use,
-                    sessions::SyncSessionContext** session_context)
-      : TestInternalComponentsFactory(switches, syncer::STORAGE_IN_MEMORY),
+                    sessions::SyncSessionContext** session_context,
+                    InternalComponentsFactory::StorageOption* storage_used)
+      : TestInternalComponentsFactory(
+          switches, InternalComponentsFactory::STORAGE_IN_MEMORY, storage_used),
         scheduler_to_use_(scheduler_to_use),
         session_context_(session_context) {}
   virtual ~ComponentsFactory() {}
@@ -2433,7 +2440,8 @@ class SyncManagerTestWithMockScheduler : public SyncManagerTest {
   SyncManagerTestWithMockScheduler() : scheduler_(NULL) {}
   virtual InternalComponentsFactory* GetFactory() OVERRIDE {
     scheduler_ = new MockSyncScheduler();
-    return new ComponentsFactory(GetSwitches(), scheduler_, &session_context_);
+    return new ComponentsFactory(GetSwitches(), scheduler_, &session_context_,
+                                 &storage_used_);
   }
 
   MockSyncScheduler* scheduler() { return scheduler_; }
@@ -3177,7 +3185,9 @@ class SyncManagerInitInvalidStorageTest : public SyncManagerTest {
   }
 
   virtual InternalComponentsFactory* GetFactory() OVERRIDE {
-    return new TestInternalComponentsFactory(GetSwitches(), STORAGE_INVALID);
+    return new TestInternalComponentsFactory(
+        GetSwitches(), InternalComponentsFactory::STORAGE_INVALID,
+        &storage_used_);
   }
 };
 

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;

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(

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);

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) {

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

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());

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',

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',

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

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_

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