[Sync] Make OnDiskDirectoryBackingStore base class for deferred DBS

DODDBS now derives from ODDBS so it can access the backing file path.

Remove backing file path from DODDBS and use ODDBS's accessor method
instead.

Rename db_is_on_disk_ to created_on_disk_.

Rename backing_filepath_ to backing_file_path_ to match base::FilePath.

Move duplicated empty change detection logic out of DBS and into
SaveChangesSnapshot.

Format CL with "git cl format".

BUG=475557

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

Cr-Commit-Position: refs/heads/master@{#325908}

8 files changed, 86 insertions, 44 deletions

diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.cc b/sync/syncable/deferred_on_disk_directory_backing_store.cc
index e4f8a1e..5e960d2 100644
--- a/sync/syncable/deferred_on_disk_directory_backing_store.cc
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.cc
@@ -13,10 +13,10 @@ 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) {
+    const std::string& dir_name,
+    const base::FilePath& backing_file_path)
+    : OnDiskDirectoryBackingStore(dir_name, backing_file_path),
+      created_on_disk_(false) {
 }
 
 DeferredOnDiskDirectoryBackingStore::~DeferredOnDiskDirectoryBackingStore() {}
@@ -26,25 +26,24 @@ bool DeferredOnDiskDirectoryBackingStore::SaveChanges(
   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()) {
+  if (!snapshot.HasUnsavedMetahandleChanges()) {
     return true;
   }
+  if (!created_on_disk_ && !CreateOnDisk())
+    return false;
+  return OnDiskDirectoryBackingStore::SaveChanges(snapshot);
+}
 
-  if (!db_is_on_disk_) {
-    if (!base::DeleteFile(backing_filepath_, false))
-      return false;
-
-    // Reopen DB on disk.
-    ResetAndCreateConnection();
-    if (!Open(backing_filepath_) || !InitializeTables())
-      return false;
-
-    db_is_on_disk_ = true;
-  }
-
-  return DirectoryBackingStore::SaveChanges(snapshot);
+bool DeferredOnDiskDirectoryBackingStore::CreateOnDisk() {
+  DCHECK(CalledOnValidThread());
+  DCHECK(!created_on_disk_);
+  ResetAndCreateConnection();
+  if (!base::DeleteFile(backing_file_path(), false))
+    return false;
+  if (!Open(backing_file_path()) || !InitializeTables())
+    return false;
+  created_on_disk_ = true;
+  return true;
 }
 
 DirOpenResult DeferredOnDiskDirectoryBackingStore::Load(
@@ -52,6 +51,7 @@ DirOpenResult DeferredOnDiskDirectoryBackingStore::Load(
     JournalIndex* delete_journals,
     MetahandleSet* metahandles_to_purge,
     Directory::KernelLoadInfo* kernel_load_info) {
+  DCHECK(CalledOnValidThread());
   // Open an in-memory database at first to create initial sync data needed by
   // Directory.
   CHECK(!IsOpen());
diff --git a/sync/syncable/deferred_on_disk_directory_backing_store.h b/sync/syncable/deferred_on_disk_directory_backing_store.h
index b50da38..388e1f9 100644
--- a/sync/syncable/deferred_on_disk_directory_backing_store.h
+++ b/sync/syncable/deferred_on_disk_directory_backing_store.h
@@ -7,7 +7,7 @@
 
 #include "base/files/file_path.h"
 #include "sync/base/sync_export.h"
-#include "sync/syncable/directory_backing_store.h"
+#include "sync/syncable/on_disk_directory_backing_store.h"
 
 namespace syncer {
 namespace syncable {
@@ -18,10 +18,10 @@ namespace syncable {
 // 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 OnDiskDirectoryBackingStore {
  public:
   DeferredOnDiskDirectoryBackingStore(const std::string& dir_name,
-                                      const base::FilePath& backing_filepath);
+                                      const base::FilePath& backing_file_path);
   ~DeferredOnDiskDirectoryBackingStore() override;
   DirOpenResult Load(Directory::MetahandlesMap* handles_map,
                      JournalIndex* delete_journals,
@@ -30,10 +30,12 @@ class SYNC_EXPORT_PRIVATE DeferredOnDiskDirectoryBackingStore
   bool SaveChanges(const Directory::SaveChangesSnapshot& snapshot) override;
 
  private:
-  base::FilePath backing_filepath_;
+  // Create an on-disk directory backing store. Returns true on success, false
+  // on error.
+  bool CreateOnDisk();
 
-  // Whether current DB is on disk.
-  bool db_is_on_disk_;
+  // Whether an on-disk directory backing store has been created.
+  bool created_on_disk_;
 
   DISALLOW_COPY_AND_ASSIGN(DeferredOnDiskDirectoryBackingStore);
 };
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index 45e0798..767b807 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -75,6 +75,11 @@ Directory::SaveChangesSnapshot::~SaveChangesSnapshot() {
   STLDeleteElements(&delete_journals);
 }
 
+bool Directory::SaveChangesSnapshot::HasUnsavedMetahandleChanges() const {
+  return !dirty_metas.empty() || !metahandles_to_purge.empty() ||
+         !delete_journals.empty() || !delete_journals_to_purge.empty();
+}
+
 Directory::Kernel::Kernel(
     const std::string& name,
     const KernelLoadInfo& info,
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index e7c59a0..7f6ae0e 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -137,6 +137,9 @@ class SYNC_EXPORT Directory {
     SaveChangesSnapshot();
     ~SaveChangesSnapshot();
 
+    // Returns true if this snapshot has any unsaved metahandle changes.
+    bool HasUnsavedMetahandleChanges() const;
+
     KernelShareInfoStatus kernel_info_status;
     PersistedKernelInfo kernel_info;
     EntryKernelSet dirty_metas;
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index 29d2e8a..4b3d3b5 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -254,10 +254,8 @@ bool DirectoryBackingStore::SaveChanges(
 
   // Back out early if there is nothing to write.
   bool save_info =
-    (Directory::KERNEL_SHARE_INFO_DIRTY == snapshot.kernel_info_status);
-  if (snapshot.dirty_metas.empty() && snapshot.metahandles_to_purge.empty() &&
-      snapshot.delete_journals.empty() &&
-      snapshot.delete_journals_to_purge.empty() && !save_info) {
+      (Directory::KERNEL_SHARE_INFO_DIRTY == snapshot.kernel_info_status);
+  if (!snapshot.HasUnsavedMetahandleChanges() && !save_info) {
     return true;
   }
 
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index 3737ca0..d802d4f 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -2006,6 +2006,30 @@ TEST_F(SyncableDirectoryTest, MutableEntry_ImplicitParentId_Siblings) {
   }
 }
 
+TEST_F(SyncableDirectoryTest, SaveChangesSnapshot_HasUnsavedMetahandleChanges) {
+  EntryKernel kernel;
+  Directory::SaveChangesSnapshot snapshot;
+  EXPECT_FALSE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.dirty_metas.insert(&kernel);
+  EXPECT_TRUE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.dirty_metas.clear();
+
+  EXPECT_FALSE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.metahandles_to_purge.insert(1);
+  EXPECT_TRUE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.metahandles_to_purge.clear();
+
+  EXPECT_FALSE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.delete_journals.insert(&kernel);
+  EXPECT_TRUE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.delete_journals.clear();
+
+  EXPECT_FALSE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.delete_journals_to_purge.insert(1);
+  EXPECT_TRUE(snapshot.HasUnsavedMetahandleChanges());
+  snapshot.delete_journals_to_purge.clear();
+}
+
 }  // namespace syncable
 
 }  // namespace syncer
diff --git a/sync/syncable/on_disk_directory_backing_store.cc b/sync/syncable/on_disk_directory_backing_store.cc
index e6f284d..918dc07 100644
--- a/sync/syncable/on_disk_directory_backing_store.cc
+++ b/sync/syncable/on_disk_directory_backing_store.cc
@@ -24,13 +24,15 @@ enum HistogramResultEnum {
 }  // namespace
 
 OnDiskDirectoryBackingStore::OnDiskDirectoryBackingStore(
-    const std::string& dir_name, const base::FilePath& backing_filepath)
+    const std::string& dir_name,
+    const base::FilePath& backing_file_path)
     : DirectoryBackingStore(dir_name),
       allow_failure_for_test_(false),
-      backing_filepath_(backing_filepath) {
+      backing_file_path_(backing_file_path) {
 }
 
-OnDiskDirectoryBackingStore::~OnDiskDirectoryBackingStore() { }
+OnDiskDirectoryBackingStore::~OnDiskDirectoryBackingStore() {
+}
 
 DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
     Directory::MetahandlesMap* handles_map,
@@ -40,7 +42,7 @@ DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
   DCHECK(CalledOnValidThread());
 
   if (!IsOpen()) {
-    if (!Open(backing_filepath_))
+    if (!Open(backing_file_path_))
       return FAILED_OPEN_DATABASE;
   }
 
@@ -57,7 +59,6 @@ DirOpenResult OnDiskDirectoryBackingStore::TryLoad(
     return FAILED_DATABASE_CORRUPT;
 
   return OPENED;
-
 }
 
 DirOpenResult OnDiskDirectoryBackingStore::Load(
@@ -65,11 +66,12 @@ DirOpenResult OnDiskDirectoryBackingStore::Load(
     JournalIndex* delete_journals,
     MetahandleSet* metahandles_to_purge,
     Directory::KernelLoadInfo* kernel_load_info) {
+  DCHECK(CalledOnValidThread());
   DirOpenResult result = TryLoad(handles_map, delete_journals,
                                  metahandles_to_purge, kernel_load_info);
   if (result == OPENED) {
-    UMA_HISTOGRAM_ENUMERATION(
-        "Sync.DirectoryOpenResult", FIRST_TRY_SUCCESS, RESULT_COUNT);
+    UMA_HISTOGRAM_ENUMERATION("Sync.DirectoryOpenResult", FIRST_TRY_SUCCESS,
+                              RESULT_COUNT);
     return OPENED;
   }
 
@@ -82,16 +84,16 @@ DirOpenResult OnDiskDirectoryBackingStore::Load(
 
   ResetAndCreateConnection();
 
-  base::DeleteFile(backing_filepath_, false);
+  base::DeleteFile(backing_file_path_, false);
 
   result = TryLoad(handles_map, delete_journals, metahandles_to_purge,
                    kernel_load_info);
   if (result == OPENED) {
-    UMA_HISTOGRAM_ENUMERATION(
-        "Sync.DirectoryOpenResult", SECOND_TRY_SUCCESS, RESULT_COUNT);
+    UMA_HISTOGRAM_ENUMERATION("Sync.DirectoryOpenResult", SECOND_TRY_SUCCESS,
+                              RESULT_COUNT);
   } else {
-    UMA_HISTOGRAM_ENUMERATION(
-        "Sync.DirectoryOpenResult", SECOND_TRY_FAILURE, RESULT_COUNT);
+    UMA_HISTOGRAM_ENUMERATION("Sync.DirectoryOpenResult", SECOND_TRY_FAILURE,
+                              RESULT_COUNT);
   }
 
   return result;
@@ -109,5 +111,10 @@ void OnDiskDirectoryBackingStore::ReportFirstTryOpenFailure() {
   NOTREACHED() << "Crashing to preserve corrupt sync database";
 }
 
+const base::FilePath& OnDiskDirectoryBackingStore::backing_file_path() const {
+  DCHECK(CalledOnValidThread());
+  return backing_file_path_;
+}
+
 }  // namespace syncable
 }  // namespace syncer
diff --git a/sync/syncable/on_disk_directory_backing_store.h b/sync/syncable/on_disk_directory_backing_store.h
index 9002eb0..8c604d0 100644
--- a/sync/syncable/on_disk_directory_backing_store.h
+++ b/sync/syncable/on_disk_directory_backing_store.h
@@ -18,7 +18,7 @@ class SYNC_EXPORT_PRIVATE OnDiskDirectoryBackingStore
     : public DirectoryBackingStore {
  public:
   OnDiskDirectoryBackingStore(const std::string& dir_name,
-                              const base::FilePath& backing_filepath);
+                              const base::FilePath& backing_file_path);
   ~OnDiskDirectoryBackingStore() override;
   DirOpenResult Load(Directory::MetahandlesMap* handles_map,
                      JournalIndex* delete_journals,
@@ -29,6 +29,9 @@ class SYNC_EXPORT_PRIVATE OnDiskDirectoryBackingStore
   // Subclasses may override this to avoid a possible DCHECK.
   virtual void ReportFirstTryOpenFailure();
 
+  // Returns the file path of the back store.
+  const base::FilePath& backing_file_path() const;
+
  private:
   // A helper function that will make one attempt to load the directory.
   // Unlike Load(), it does not attempt to recover from failure.
@@ -38,7 +41,7 @@ class SYNC_EXPORT_PRIVATE OnDiskDirectoryBackingStore
                         Directory::KernelLoadInfo* kernel_load_info);
 
   bool allow_failure_for_test_;
-  base::FilePath backing_filepath_;
+  base::FilePath backing_file_path_;
 
   DISALLOW_COPY_AND_ASSIGN(OnDiskDirectoryBackingStore);
 };