diff options
author | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-11 08:14:14 +0000 |
---|---|---|
committer | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-04-11 08:14:14 +0000 |
commit | b5e5a42df4b992425cb8692d418dfc1aca1bc316 (patch) | |
tree | c1cabba64eff1823e74b343ed361afbdd04c4edd | |
parent | f5c9dbcb852de1cd6ce1bd293920c8c64a7ad9d9 (diff) | |
download | chromium_src-b5e5a42df4b992425cb8692d418dfc1aca1bc316.zip chromium_src-b5e5a42df4b992425cb8692d418dfc1aca1bc316.tar.gz chromium_src-b5e5a42df4b992425cb8692d418dfc1aca1bc316.tar.bz2 |
[Sync] Properly purge context/progress markers for disabled types
We were not properly purging all the context or progress marker info,
and it looks like we were even marking the kernel as dirty to ensure it
got saved into the directory. This fixes that, and tests it properly.
BUG=362307
Review URL: https://codereview.chromium.org/234403003
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@263193 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | sync/syncable/directory.cc | 21 | ||||
-rw-r--r-- | sync/syncable/directory.h | 2 | ||||
-rw-r--r-- | sync/syncable/syncable_unittest.cc | 33 |
3 files changed, 48 insertions, 8 deletions
diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc index 11bc956..e92f456 100644 --- a/sync/syncable/directory.cc +++ b/sync/syncable/directory.cc @@ -39,19 +39,22 @@ Directory::PersistedKernelInfo::PersistedKernelInfo() ModelTypeSet protocol_types = ProtocolTypes(); for (ModelTypeSet::Iterator iter = protocol_types.First(); iter.Good(); iter.Inc()) { - reset_download_progress(iter.Get()); + ResetDownloadProgress(iter.Get()); transaction_version[iter.Get()] = 0; } } Directory::PersistedKernelInfo::~PersistedKernelInfo() {} -void Directory::PersistedKernelInfo::reset_download_progress( +void Directory::PersistedKernelInfo::ResetDownloadProgress( ModelType model_type) { + // Clear everything except the data type id field. + download_progress[model_type].Clear(); download_progress[model_type].set_data_type_id( GetSpecificsFieldNumberFromModelType(model_type)); - // An empty-string token indicates no prior knowledge. - download_progress[model_type].set_token(std::string()); + + // Explicitly set an empty token field to denote no progress. + download_progress[model_type].set_token(""); } Directory::SaveChangesSnapshot::SaveChangesSnapshot() @@ -716,10 +719,14 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet disabled_types, it.Good(); it.Inc()) { kernel_->persisted_info.transaction_version[it.Get()] = 0; - // Don't discard progress markers for unapplied types. - if (!types_to_unapply.Has(it.Get())) - kernel_->persisted_info.reset_download_progress(it.Get()); + // Don't discard progress markers or context for unapplied types. + if (!types_to_unapply.Has(it.Get())) { + kernel_->persisted_info.ResetDownloadProgress(it.Get()); + kernel_->persisted_info.datatype_context[it.Get()].Clear(); + } } + + kernel_->info_status = KERNEL_SHARE_INFO_DIRTY; } } return true; diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h index 6b91a5a..438c11c 100644 --- a/sync/syncable/directory.h +++ b/sync/syncable/directory.h @@ -100,7 +100,7 @@ class SYNC_EXPORT Directory { // Set the |download_progress| entry for the given model to a // "first sync" start point. When such a value is sent to the server, // a full download of all objects of the model will be initiated. - void reset_download_progress(ModelType model_type); + void ResetDownloadProgress(ModelType model_type); // Last sync timestamp fetched from the server. sync_pb::DataTypeProgressMarker download_progress[MODEL_TYPE_COUNT]; diff --git a/sync/syncable/syncable_unittest.cc b/sync/syncable/syncable_unittest.cc index 8f261c7..01176c2 100644 --- a/sync/syncable/syncable_unittest.cc +++ b/sync/syncable/syncable_unittest.cc @@ -489,6 +489,14 @@ class SyncableDirectoryTest : public testing::Test { for (ModelTypeSet::Iterator it = types_to_purge.First(); it.Good(); it.Inc()) { EXPECT_FALSE(dir_->InitialSyncEndedForType(it.Get())); + sync_pb::DataTypeProgressMarker progress; + dir_->GetDownloadProgress(it.Get(), &progress); + EXPECT_EQ("", progress.token()); + + ReadTransaction trans(FROM_HERE, dir_.get()); + sync_pb::DataTypeContext context; + dir_->GetDataTypeContext(&trans, it.Get(), &context); + EXPECT_TRUE(context.SerializeAsString().empty()); } EXPECT_FALSE(types_to_purge.Has(BOOKMARKS)); EXPECT_TRUE(dir_->InitialSyncEndedForType(BOOKMARKS)); @@ -1686,6 +1694,20 @@ 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"); + context.set_data_type_id(GetSpecificsFieldNumberFromModelType(type)); + return context; +} + TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) { sync_pb::EntitySpecifics bookmark_specs; sync_pb::EntitySpecifics autofill_specs; @@ -1696,11 +1718,22 @@ TEST_F(OnDiskSyncableDirectoryTest, TestPurgeEntriesWithTypeIn) { ModelTypeSet types_to_purge(PREFERENCES, AUTOFILL); + dir_->SetDownloadProgress(BOOKMARKS, + BuildProgress(BOOKMARKS)); + dir_->SetDownloadProgress(PREFERENCES, + BuildProgress(PREFERENCES)); + dir_->SetDownloadProgress(AUTOFILL, + BuildProgress(AUTOFILL)); + TestIdFactory id_factory; // Create some items for each type. { WriteTransaction trans(FROM_HERE, UNITTEST, dir_.get()); + dir_->SetDataTypeContext(&trans, BOOKMARKS, BuildContext(BOOKMARKS)); + dir_->SetDataTypeContext(&trans, PREFERENCES, BuildContext(PREFERENCES)); + dir_->SetDataTypeContext(&trans, AUTOFILL, BuildContext(AUTOFILL)); + // Make it look like these types have completed initial sync. CreateTypeRoot(&trans, dir_.get(), BOOKMARKS); CreateTypeRoot(&trans, dir_.get(), PREFERENCES); |