Commit 47c2232d authored by pnoland's avatar pnoland Committed by Commit bot

[sync] Purge directory data when migrating to USS

Make use of the existing PurgeEntriesWithTypeIn to purge the directory
of entries of the type we just migrated. Add a test that ensures this
happens.

R=skym@chromium.org

BUG=716156

Review-Url: https://codereview.chromium.org/2842373004
Cr-Commit-Position: refs/heads/master@{#468170}
parent f8f753f6
...@@ -82,8 +82,6 @@ void ModelTypeRegistry::ConnectNonBlockingType( ...@@ -82,8 +82,6 @@ void ModelTypeRegistry::ConnectNonBlockingType(
activation_context->model_type_state.initial_sync_done(); activation_context->model_type_state.initial_sync_done();
// Attempt migration if the USS initial sync hasn't been done, there is a // Attempt migration if the USS initial sync hasn't been done, there is a
// migrator function, and directory has data for this type. // migrator function, and directory has data for this type.
// Note: The injected migrator function is currently null outside of testing
// until issues with triggering initial sync correctly are addressed.
bool do_migration = !initial_sync_done && !uss_migrator_.is_null() && bool do_migration = !initial_sync_done && !uss_migrator_.is_null() &&
directory()->InitialSyncEndedForType(type); directory()->InitialSyncEndedForType(type);
bool trigger_initial_sync = !initial_sync_done && !do_migration; bool trigger_initial_sync = !initial_sync_done && !do_migration;
...@@ -124,17 +122,24 @@ void ModelTypeRegistry::ConnectNonBlockingType( ...@@ -124,17 +122,24 @@ void ModelTypeRegistry::ConnectNonBlockingType(
// TODO(crbug.com/658002): Store a pref before attempting migration // TODO(crbug.com/658002): Store a pref before attempting migration
// indicating that it was attempted so we can avoid failure loops. // indicating that it was attempted so we can avoid failure loops.
if (uss_migrator_.Run(type, user_share_, worker_ptr)) { if (uss_migrator_.Run(type, user_share_, worker_ptr)) {
UMA_HISTOGRAM_ENUMERATION("Sync.USSMigrationSuccess", type, UMA_HISTOGRAM_ENUMERATION("Sync.USSMigrationSuccess",
ModelTypeToHistogramInt(type),
MODEL_TYPE_COUNT); MODEL_TYPE_COUNT);
// If we succesfully migrated, purge the directory of data for the type.
// Purging removes the directory's local copy of the data only.
directory()->PurgeEntriesWithTypeIn(ModelTypeSet(type), ModelTypeSet(),
ModelTypeSet());
} else { } else {
UMA_HISTOGRAM_ENUMERATION("Sync.USSMigrationFailure", type, UMA_HISTOGRAM_ENUMERATION("Sync.USSMigrationFailure",
ModelTypeToHistogramInt(type),
MODEL_TYPE_COUNT); MODEL_TYPE_COUNT);
} }
} }
// TODO(crbug.com/658002): Delete directory data here if initial_sync_done and // We want to check that we haven't accidentally enabled both the non-blocking
// has_directory_data are both true. // and directory implementations for a given model type. This is true even if
// migration fails; our fallback in this case is to do an initial GetUpdates,
// not to use the directory implementation.
DCHECK(Intersection(GetEnabledDirectoryTypes(), GetEnabledNonBlockingTypes()) DCHECK(Intersection(GetEnabledDirectoryTypes(), GetEnabledNonBlockingTypes())
.Empty()); .Empty());
} }
......
...@@ -75,8 +75,18 @@ class ModelTypeRegistryTest : public ::testing::Test { ...@@ -75,8 +75,18 @@ class ModelTypeRegistryTest : public ::testing::Test {
directory()->MarkInitialSyncEndedForType(&trans, type); directory()->MarkInitialSyncEndedForType(&trans, type);
} }
void SetDummyProgressMarkerForType(ModelType type) {
sync_pb::DataTypeProgressMarker progress_marker;
progress_marker.set_token("dummy");
directory()->SetDownloadProgress(type, progress_marker);
}
bool migration_attempted() { return migration_attempted_; } bool migration_attempted() { return migration_attempted_; }
syncable::MetahandleSet metahandles_to_purge() {
return directory()->kernel()->metahandles_to_purge;
}
private: private:
bool MigrateDirectory(ModelType type, bool MigrateDirectory(ModelType type,
UserShare* user_share, UserShare* user_share,
...@@ -208,15 +218,20 @@ TEST_F(ModelTypeRegistryTest, GetInitialSyncEndedTypes) { ...@@ -208,15 +218,20 @@ TEST_F(ModelTypeRegistryTest, GetInitialSyncEndedTypes) {
} }
// Tests that when directory data is present for type ConnectNonBlockingType // Tests that when directory data is present for type ConnectNonBlockingType
// triggers USS migration. // triggers USS migration and purges old directory data.
TEST_F(ModelTypeRegistryTest, UssMigrationAttempted) { TEST_F(ModelTypeRegistryTest, UssMigration) {
EXPECT_FALSE(migration_attempted()); EXPECT_FALSE(migration_attempted());
MarkInitialSyncEndedForDirectoryType(THEMES); MarkInitialSyncEndedForDirectoryType(THEMES);
// Purge only proceeds in the presence of a progress marker for the type(s)
// being purged.
SetDummyProgressMarkerForType(THEMES);
EXPECT_EQ(0u, metahandles_to_purge().size());
registry()->ConnectNonBlockingType( registry()->ConnectNonBlockingType(
THEMES, MakeActivationContext(MakeInitialModelTypeState(THEMES))); THEMES, MakeActivationContext(MakeInitialModelTypeState(THEMES)));
EXPECT_TRUE(migration_attempted()); EXPECT_TRUE(migration_attempted());
EXPECT_NE(0u, metahandles_to_purge().size());
} }
} // namespace syncer } // namespace syncer
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment