Commit 22acb1ed authored by Mikel Astiz's avatar Mikel Astiz Committed by Chromium LUCI CQ

[sync] Relax failing DCHECK due to SyncData logging

SyncData::ToString() is used for logging and, starting with
https://crrev.com/c/2562659, it can DCHECK-fail if the sync entity's
title is empty.

This patch removes the DCHECK from SyncData::GetTitle(), since it
seems overly strict, and the main calling site in
SyncableServiceBasedBridge already verifies that the title is non-empty.
To make this more obvious, an additional DCHECKs is introduced (which
is redundant with pre-existing DCHECKs) and two functions renamed for
clarity.

Change-Id: Ib499e6a07132ab21033d83a241e5e380226913b0
Fixed: 1156130
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577468
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: default avatarMaksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#834673}
parent a93f94f7
...@@ -102,8 +102,6 @@ ClientTagHash SyncData::GetClientTagHash() const { ...@@ -102,8 +102,6 @@ ClientTagHash SyncData::GetClientTagHash() const {
} }
const std::string& SyncData::GetTitle() const { const std::string& SyncData::GetTitle() const {
// TODO(zea): set this for data coming from the syncer too.
DCHECK(immutable_entity_.Get().has_non_unique_name());
return immutable_entity_.Get().non_unique_name(); return immutable_entity_.Get().non_unique_name();
} }
......
...@@ -54,6 +54,7 @@ TEST_F(SyncDataTest, CreateLocalData) { ...@@ -54,6 +54,7 @@ TEST_F(SyncDataTest, CreateLocalData) {
EXPECT_EQ(kDatatype, data.GetDataType()); EXPECT_EQ(kDatatype, data.GetDataType());
EXPECT_EQ(kNonUniqueTitle, data.GetTitle()); EXPECT_EQ(kNonUniqueTitle, data.GetTitle());
EXPECT_TRUE(data.GetSpecifics().has_preference()); EXPECT_TRUE(data.GetSpecifics().has_preference());
EXPECT_FALSE(data.ToString().empty());
} }
TEST_F(SyncDataTest, CreateRemoteData) { TEST_F(SyncDataTest, CreateRemoteData) {
...@@ -65,6 +66,7 @@ TEST_F(SyncDataTest, CreateRemoteData) { ...@@ -65,6 +66,7 @@ TEST_F(SyncDataTest, CreateRemoteData) {
EXPECT_EQ(ClientTagHash::FromUnhashed(PREFERENCES, kSyncTag), EXPECT_EQ(ClientTagHash::FromUnhashed(PREFERENCES, kSyncTag),
data.GetClientTagHash()); data.GetClientTagHash());
EXPECT_TRUE(data.GetSpecifics().has_preference()); EXPECT_TRUE(data.GetSpecifics().has_preference());
EXPECT_FALSE(data.ToString().empty());
} }
} // namespace } // namespace
......
...@@ -42,7 +42,7 @@ std::unique_ptr<EntityData> ConvertPersistedToEntityData( ...@@ -42,7 +42,7 @@ std::unique_ptr<EntityData> ConvertPersistedToEntityData(
return entity_data; return entity_data;
} }
sync_pb::PersistedEntityData CreatePersistedFromEntityData( sync_pb::PersistedEntityData CreatePersistedFromRemoteData(
const EntityData& entity_data) { const EntityData& entity_data) {
sync_pb::PersistedEntityData persisted; sync_pb::PersistedEntityData persisted;
persisted.set_name(entity_data.name); persisted.set_name(entity_data.name);
...@@ -50,10 +50,12 @@ sync_pb::PersistedEntityData CreatePersistedFromEntityData( ...@@ -50,10 +50,12 @@ sync_pb::PersistedEntityData CreatePersistedFromEntityData(
return persisted; return persisted;
} }
sync_pb::PersistedEntityData CreatePersistedFromSyncData( sync_pb::PersistedEntityData CreatePersistedFromLocalData(
const SyncData& sync_data) { const SyncData& sync_data) {
DCHECK(sync_data.IsLocal()); DCHECK(sync_data.IsLocal());
DCHECK(sync_data.IsValid());
DCHECK(!sync_data.GetTitle().empty()); DCHECK(!sync_data.GetTitle().empty());
sync_pb::PersistedEntityData persisted; sync_pb::PersistedEntityData persisted;
persisted.set_name(sync_data.GetTitle()); persisted.set_name(sync_data.GetTitle());
*persisted.mutable_specifics() = sync_data.GetSpecifics(); *persisted.mutable_specifics() = sync_data.GetSpecifics();
...@@ -149,7 +151,7 @@ class LocalChangeProcessor : public SyncChangeProcessor { ...@@ -149,7 +151,7 @@ class LocalChangeProcessor : public SyncChangeProcessor {
(*in_memory_store_)[storage_key] = change.sync_data().GetSpecifics(); (*in_memory_store_)[storage_key] = change.sync_data().GetSpecifics();
sync_pb::PersistedEntityData persisted_entity = sync_pb::PersistedEntityData persisted_entity =
CreatePersistedFromSyncData(change.sync_data()); CreatePersistedFromLocalData(change.sync_data());
batch->WriteData(storage_key, persisted_entity.SerializeAsString()); batch->WriteData(storage_key, persisted_entity.SerializeAsString());
other_->Put(storage_key, other_->Put(storage_key,
...@@ -564,7 +566,7 @@ SyncChangeList SyncableServiceBasedBridge::StoreAndConvertRemoteChanges( ...@@ -564,7 +566,7 @@ SyncChangeList SyncableServiceBasedBridge::StoreAndConvertRemoteChanges(
batch->WriteData( batch->WriteData(
storage_key, storage_key,
CreatePersistedFromEntityData(change->data()).SerializeAsString()); CreatePersistedFromRemoteData(change->data()).SerializeAsString());
in_memory_store_[storage_key] = change->data().specifics; in_memory_store_[storage_key] = change->data().specifics;
break; break;
} }
......
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