[Sync] Rely on directory to decide which types need to be purged

Preivously we relied on the SyncBackendRegistrar's last configured types to
decide which types have been recently disabled. Now we just purge all disabled
types that exist in the directory, as recorded by their progress markers (which
should never be deleted unless the type has been purged).

BUG=386778

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284283 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 44 insertions, 8 deletions

diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 6f891e8d..160ed9c 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -1037,6 +1037,10 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
   // Similar to above, but throw a purge operation into the mix. Bug 49278.
   syncable::Id pref_node_id = TestIdFactory::MakeServer("Tim");
   {
+    directory()->SetDownloadProgress(BOOKMARKS,
+                                     syncable::BuildProgress(BOOKMARKS));
+    directory()->SetDownloadProgress(PREFERENCES,
+                                     syncable::BuildProgress(PREFERENCES));
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "Pete");
     ASSERT_TRUE(parent.good());
@@ -1086,6 +1090,8 @@ TEST_F(SyncerTest, TestPurgeWhileUnsynced) {
 TEST_F(SyncerTest, TestPurgeWhileUnapplied) {
   // Similar to above, but for unapplied items. Bug 49278.
   {
+    directory()->SetDownloadProgress(BOOKMARKS,
+                                     syncable::BuildProgress(BOOKMARKS));
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&wtrans, CREATE, BOOKMARKS, wtrans.root_id(), "Pete");
     ASSERT_TRUE(parent.good());
@@ -1111,6 +1117,8 @@ TEST_F(SyncerTest, TestPurgeWhileUnapplied) {
 
 TEST_F(SyncerTest, TestPurgeWithJournal) {
   {
+    directory()->SetDownloadProgress(BOOKMARKS,
+                                     syncable::BuildProgress(BOOKMARKS));
     WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
     MutableEntry parent(&wtrans, syncable::CREATE, BOOKMARKS, wtrans.root_id(),
                         "Pete");
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc
index be489b2..1af5474 100644
--- a/sync/syncable/directory.cc
+++ b/sync/syncable/directory.cc
@@ -58,6 +58,13 @@ void Directory::PersistedKernelInfo::ResetDownloadProgress(
   download_progress[model_type].set_token("");
 }
 
+bool Directory::PersistedKernelInfo::HasEmptyDownloadProgress(
+    ModelType model_type) {
+  const sync_pb::DataTypeProgressMarker& progress_marker =
+      download_progress[model_type];
+  return progress_marker.token().empty();
+}
+
 Directory::SaveChangesSnapshot::SaveChangesSnapshot()
     : kernel_info_status(KERNEL_SHARE_INFO_INVALID) {
 }
@@ -738,6 +745,18 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet disabled_types,
     {
       ScopedKernelLock lock(this);
 
+      bool found_progress = false;
+      for (ModelTypeSet::Iterator iter = disabled_types.First(); iter.Good();
+           iter.Inc()) {
+        if (!kernel_->persisted_info.HasEmptyDownloadProgress(iter.Get()))
+          found_progress = true;
+      }
+
+      // If none of the disabled types have progress markers, there's nothing to
+      // purge.
+      if (!found_progress)
+        return true;
+
       // We iterate in two passes to avoid a bug in STLport (which is used in
       // the Android build).  There are some versions of that library where a
       // hash_map's iterators can be invalidated when an item is erased from the
diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h
index 2830eec..01baac7 100644
--- a/sync/syncable/directory.h
+++ b/sync/syncable/directory.h
@@ -109,6 +109,9 @@ class SYNC_EXPORT Directory {
     // a full download of all objects of the model will be initiated.
     void ResetDownloadProgress(ModelType model_type);
 
+    // Whether a valid progress marker exists for |model_type|.
+    bool HasEmptyDownloadProgress(ModelType model_type);
+
     // Last sync timestamp fetched from the server.
     sync_pb::DataTypeProgressMarker download_progress[MODEL_TYPE_COUNT];
     // Sync-side transaction version per data type. Monotonically incremented
diff --git a/sync/syncable/directory_unittest.cc b/sync/syncable/directory_unittest.cc
index efc97e6..fabdc47 100644
--- a/sync/syncable/directory_unittest.cc
+++ b/sync/syncable/directory_unittest.cc
@@ -232,6 +232,8 @@ TEST_F(SyncableDirectoryTest, TakeSnapshotGetsMetahandlesToPurge) {
   MetahandleSet expected_purges;
   MetahandleSet all_handles;
   {
+    dir()->SetDownloadProgress(BOOKMARKS, BuildProgress(BOOKMARKS));
+    dir()->SetDownloadProgress(PREFERENCES, BuildProgress(PREFERENCES));
     WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
     for (int i = 0; i < metas_to_create; i++) {
       MutableEntry e(&trans, CREATE, BOOKMARKS, trans.root_id(), "foo");
diff --git a/sync/syncable/syncable_unittest.cc b/sync/syncable/syncable_unittest.cc
index ef0c9dd..7e1aad5 100644
--- a/sync/syncable/syncable_unittest.cc
+++ b/sync/syncable/syncable_unittest.cc
@@ -191,13 +191,6 @@ class OnDiskSyncableDirectoryTest : public SyncableDirectoryTest {
   base::FilePath file_path_;
 };
 
-sync_pb::DataTypeProgressMarker BuildProgress(ModelType type) {
-  sync_pb::DataTypeProgressMarker progress;
-  progress.set_token("token");
-  progress.set_data_type_id(GetSpecificsFieldNumberFromModelType(type));
-  return progress;
-}
-
 sync_pb::DataTypeContext BuildContext(ModelType type) {
   sync_pb::DataTypeContext context;
   context.set_context("context");
@@ -518,7 +511,8 @@ TEST_F(OnDiskSyncableDirectoryTest, TestSaveChangesFailure) {
 
 TEST_F(OnDiskSyncableDirectoryTest, TestSaveChangesFailureWithPurge) {
   int64 handle1 = 0;
-  // Set up an item using a regular, saveable directory.
+  // Set up an item and progress marker using a regular, saveable directory.
+  dir()->SetDownloadProgress(BOOKMARKS, BuildProgress(BOOKMARKS));
   {
     WriteTransaction trans(FROM_HERE, UNITTEST, dir().get());
 
diff --git a/sync/test/engine/test_syncable_utils.cc b/sync/test/engine/test_syncable_utils.cc
index 8eb247f..aeb4c66 100644
--- a/sync/test/engine/test_syncable_utils.cc
+++ b/sync/test/engine/test_syncable_utils.cc
@@ -90,5 +90,12 @@ void CreateTypeRoot(WriteTransaction* trans,
   node.PutSpecifics(specifics);
 }
 
+sync_pb::DataTypeProgressMarker BuildProgress(ModelType type) {
+  sync_pb::DataTypeProgressMarker progress;
+  progress.set_token("token");
+  progress.set_data_type_id(GetSpecificsFieldNumberFromModelType(type));
+  return progress;
+}
+
 }  // namespace syncable
 }  // namespace syncer
diff --git a/sync/test/engine/test_syncable_utils.h b/sync/test/engine/test_syncable_utils.h
index 7f162f7..5975e12 100644
--- a/sync/test/engine/test_syncable_utils.h
+++ b/sync/test/engine/test_syncable_utils.h
@@ -11,6 +11,7 @@
 #include <string>
 
 #include "sync/internal_api/public/base/model_type.h"
+#include "sync/protocol/sync.pb.h"
 
 namespace syncer {
 namespace syncable {
@@ -42,6 +43,8 @@ void CreateTypeRoot(WriteTransaction* trans,
                     syncable::Directory *dir,
                     ModelType type);
 
+sync_pb::DataTypeProgressMarker BuildProgress(ModelType type);
+
 }  // namespace syncable
 }  // namespace syncer