[Sync::USS] Don't populate client tags for bookmark tombstones
Before this CL:
NonBlockingTypeCommitContribution used to populate the client
defined unique tag for bookmark tombstones with an empty string. This
is because it used the check if specifics has bookmarks to identify
the bookmark commits. However, for tombstones, specifics are empty and
hence the check fails.
After this CL:
The model type is plumbed through and checked explicitly instead of
checking the specifics.
In addition, this CL also makes sure the client tag isn't populated for
Nigori data type.
Bug: 963895
Change-Id: I2c04e758fc341f92cd3ffe46b38579b5d98fead6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1615202
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660389}
diff --git a/components/sync/engine_impl/non_blocking_type_commit_contribution.cc b/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
index 93c9bfe4..1440bf5 100644
--- a/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
+++ b/components/sync/engine_impl/non_blocking_type_commit_contribution.cc
@@ -58,7 +58,7 @@
sync_entity->mutable_specifics()->CopyFrom(
commit_request->entity->specifics);
} else {
- PopulateCommitProto(*commit_request, sync_entity);
+ PopulateCommitProto(type_, *commit_request, sync_entity);
AdjustCommitProto(sync_entity);
}
@@ -126,7 +126,7 @@
response_list.push_back(response_data);
status->increment_num_successful_commits();
- if (commit_request.entity->specifics.has_bookmark()) {
+ if (type_ == BOOKMARKS) {
status->increment_num_successful_bookmark_commits();
}
@@ -178,12 +178,14 @@
// static
void NonBlockingTypeCommitContribution::PopulateCommitProto(
+ ModelType type,
const CommitRequestData& commit_entity,
sync_pb::SyncEntity* commit_proto) {
const EntityData& entity_data = *commit_entity.entity;
commit_proto->set_id_string(entity_data.id);
- // Populate client_defined_unique_tag only for non-bookmark data types.
- if (!entity_data.specifics.has_bookmark()) {
+ // Populate client_defined_unique_tag only for non-bookmark and non-Nigori
+ // data types.
+ if (type != BOOKMARKS && type != NIGORI) {
commit_proto->set_client_defined_unique_tag(entity_data.client_tag_hash);
}
commit_proto->set_version(commit_entity.base_version);
@@ -193,7 +195,7 @@
if (!entity_data.is_deleted()) {
// Handle bookmarks separately.
- if (entity_data.specifics.has_bookmark()) {
+ if (type == BOOKMARKS) {
// position_in_parent field is set only for legacy reasons. See comments
// in sync.proto for more information.
commit_proto->set_position_in_parent(
diff --git a/components/sync/engine_impl/non_blocking_type_commit_contribution.h b/components/sync/engine_impl/non_blocking_type_commit_contribution.h
index 7e9ec461..c6bdb0c 100644
--- a/components/sync/engine_impl/non_blocking_type_commit_contribution.h
+++ b/components/sync/engine_impl/non_blocking_type_commit_contribution.h
@@ -49,7 +49,8 @@
// Public for testing.
// Copies data to be committed from CommitRequestData into SyncEntity proto.
- static void PopulateCommitProto(const CommitRequestData& commit_entity,
+ static void PopulateCommitProto(ModelType type,
+ const CommitRequestData& commit_entity,
sync_pb::SyncEntity* commit_proto);
private:
diff --git a/components/sync/engine_impl/non_blocking_type_commit_contribution_unittest.cc b/components/sync/engine_impl/non_blocking_type_commit_contribution_unittest.cc
index 285a3799..f0552ea1 100644
--- a/components/sync/engine_impl/non_blocking_type_commit_contribution_unittest.cc
+++ b/components/sync/engine_impl/non_blocking_type_commit_contribution_unittest.cc
@@ -71,7 +71,8 @@
request_data.entity = std::move(data);
SyncEntity entity;
- NonBlockingTypeCommitContribution::PopulateCommitProto(request_data, &entity);
+ NonBlockingTypeCommitContribution::PopulateCommitProto(PREFERENCES,
+ request_data, &entity);
// Exhaustively verify the populated SyncEntity.
EXPECT_TRUE(entity.id_string().empty());
@@ -119,7 +120,8 @@
request_data.entity = std::move(data);
SyncEntity entity;
- NonBlockingTypeCommitContribution::PopulateCommitProto(request_data, &entity);
+ NonBlockingTypeCommitContribution::PopulateCommitProto(BOOKMARKS,
+ request_data, &entity);
// Exhaustively verify the populated SyncEntity.
EXPECT_FALSE(entity.id_string().empty());
