0

Allow bridges without support for computing client tags

We ancitipate future changes where certain bridges will lack the support
for client tag hashes. Hence, let's accomodate that in the bridge
interface and update the processor to honor the new contract.

Bug: 870624,881289
Change-Id: Ic871d7a9fd7cf9c9a989624e1691409aed400e39
Reviewed-on: https://chromium-review.googlesource.com/1249023
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Florian Uunk <feuunk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594885}
This commit is contained in:
Mikel Astiz
2018-09-27 21:58:37 +00:00
committed by Commit Bot
parent 2f7cc59335
commit 715c7e1847
7 changed files with 45 additions and 16 deletions

@ -4,8 +4,6 @@
#include "components/sync/model/model_type_change_processor.h"
#include "components/sync/model_impl/client_tag_based_model_type_processor.h"
namespace syncer {
ModelTypeChangeProcessor::ModelTypeChangeProcessor() {}

@ -30,7 +30,8 @@ class ModelTypeChangeProcessor {
// Inform the processor of a new or updated entity. The |entity_data| param
// does not need to be fully set, but it should at least have specifics and
// non-unique name. The processor will fill in the rest if the bridge does
// not have a reason to care.
// not have a reason to care. For example, if |client_tag_hash| is not set,
// the bridge's GetClientTag() will be exercised (and must be supported).
virtual void Put(const std::string& storage_key,
std::unique_ptr<EntityData> entity_data,
MetadataChangeList* metadata_change_list) = 0;

@ -24,6 +24,10 @@ ModelTypeSyncBridge::~ModelTypeSyncBridge() {}
void ModelTypeSyncBridge::OnSyncStarting(
const DataTypeActivationRequest& request) {}
bool ModelTypeSyncBridge::SupportsGetClientTag() const {
return true;
}
bool ModelTypeSyncBridge::SupportsGetStorageKey() const {
return true;
}

@ -103,6 +103,8 @@ class ModelTypeSyncBridge {
// Used for getting all data in Sync Node Browser of chrome://sync-internals.
virtual void GetAllDataForDebugging(DataCallback callback) = 0;
// Must not be called unless SupportsGetClientTag() returns true.
//
// Get or generate a client tag for |entity_data|. This must be the same tag
// that was/would have been generated in the SyncableService/Directory world
// for backward compatibility with pre-USS clients. The only time this
@ -112,6 +114,8 @@ class ModelTypeSyncBridge {
// GetStorageKey(). Only the hash of this value is kept.
virtual std::string GetClientTag(const EntityData& entity_data) = 0;
// Must not be called unless SupportsGetStorageKey() returns true.
//
// Get or generate a storage key for |entity_data|. This will only ever be
// called once when first encountering a remote entity. Local changes will
// provide their storage keys directly to Put instead of using this method.
@ -121,6 +125,12 @@ class ModelTypeSyncBridge {
// type should strive to keep these keys as small as possible.
virtual std::string GetStorageKey(const EntityData& entity_data) = 0;
// Whether or not the bridge is capable of producing a client tag from
// |EntityData| (usually remote changes), via GetClientTag(). Most bridges do,
// but in rare cases including commit-only types and read-only types, it may
// not.
virtual bool SupportsGetClientTag() const;
// By returning true in this function datatype indicates that it can generate
// storage key from EntityData. In this case for all new entities received
// from server, change processor will call GetStorageKey and update

@ -370,9 +370,10 @@ void ClientTagBasedModelTypeProcessor::Put(
// |data->client_tag_hash|, so let's ask for the client tag if needed.
if (data->client_tag_hash.empty()) {
data->client_tag_hash = GetClientTagHash(storage_key, *data);
} else {
} else if (bridge_->SupportsGetClientTag()) {
// If the Put() call already included the client tag, let's verify that
// it's consistent with the bridge's regular GetClientTag() function.
// it's consistent with the bridge's regular GetClientTag() function (if
// supported by the bridge).
DCHECK_EQ(data->client_tag_hash,
GenerateSyncableHash(type_, bridge_->GetClientTag(*data)));
}
@ -427,6 +428,9 @@ void ClientTagBasedModelTypeProcessor::UpdateStorageKey(
MetadataChangeList* metadata_change_list) {
const std::string& client_tag_hash = entity_data.client_tag_hash;
DCHECK(!client_tag_hash.empty());
DCHECK(!storage_key.empty());
DCHECK(!bridge_->SupportsGetStorageKey());
ProcessorEntityTracker* entity = GetEntityForTagHash(client_tag_hash);
DCHECK(entity);
@ -585,6 +589,7 @@ UpdateResponseDataList PopulateClientTagsForWalletData(
const ModelType& type,
ModelTypeSyncBridge* bridge,
const UpdateResponseDataList& updates) {
DCHECK(bridge->SupportsGetClientTag());
UpdateResponseDataList updates_with_client_tags;
for (const UpdateResponseData& update : updates) {
if (update.entity->parent_id == "0") {
@ -677,7 +682,7 @@ ProcessorEntityTracker* ClientTagBasedModelTypeProcessor::ProcessUpdate(
}
// Filter out unexpected client tag hashes.
if (!data.is_deleted() &&
if (!data.is_deleted() && bridge_->SupportsGetClientTag() &&
client_tag_hash !=
GenerateSyncableHash(type_, bridge_->GetClientTag(data))) {
DLOG(WARNING) << "Received unexpected client tag hash: " << client_tag_hash;
@ -1096,8 +1101,9 @@ void ClientTagBasedModelTypeProcessor::CommitLocalChanges(
std::string ClientTagBasedModelTypeProcessor::GetClientTagHash(
const std::string& storage_key,
const EntityData& data) {
const EntityData& data) const {
auto iter = storage_key_to_tag_hash_.find(storage_key);
DCHECK(bridge_->SupportsGetClientTag());
return iter == storage_key_to_tag_hash_.end()
? GenerateSyncableHash(type_, bridge_->GetClientTag(data))
: iter->second;
@ -1138,8 +1144,10 @@ ProcessorEntityTracker* ClientTagBasedModelTypeProcessor::CreateEntity(
ProcessorEntityTracker* ClientTagBasedModelTypeProcessor::CreateEntity(
const EntityData& data) {
DCHECK_EQ(data.client_tag_hash,
GenerateSyncableHash(type_, bridge_->GetClientTag(data)));
if (bridge_->SupportsGetClientTag()) {
DCHECK_EQ(data.client_tag_hash,
GenerateSyncableHash(type_, bridge_->GetClientTag(data)));
}
std::string storage_key;
if (bridge_->SupportsGetStorageKey())
storage_key = bridge_->GetStorageKey(data);

@ -37,9 +37,11 @@ class ProcessorEntityTracker;
// A sync component embedded on the model type's thread that tracks entity
// metadata in the model store and coordinates communication between sync and
// model type threads. See
// //docs/sync/uss/client_tag_based_model_type_processor.md for a more thorough
// description.
// model type threads. All changes in flight (either incoming from the server
// or local changes reported by the bridge) must specify a client tag.
//
// See //docs/sync/uss/client_tag_based_model_type_processor.md for a more
// thorough description.
class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
public ModelTypeChangeProcessor,
public ModelTypeControllerDelegate {
@ -175,7 +177,7 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
// with |data| if the lookup finds nothing. Does not update the storage key to
// client tag hash mapping.
std::string GetClientTagHash(const std::string& storage_key,
const EntityData& data);
const EntityData& data) const;
// Gets the entity for the given storage key, or null if there isn't one.
ProcessorEntityTracker* GetEntityForStorageKey(

@ -1,13 +1,19 @@
# ClientTagBasedModelTypeProcessor
The [`ClientTagBasedModelTypeProcessor`][SMTP] is a crucial piece of the USS codepath.
It lives on the model thread and performs the tracking of sync metadata for the
[`ModelTypeSyncBridge`][MTSB] that owns it by implementing the
The [`ClientTagBasedModelTypeProcessor`][SMTP] is a crucial piece of the USS
codepath. It lives on the model thread and performs the tracking of sync
metadata for the [`ModelTypeSyncBridge`][MTSB] that owns it by implementing the
[`ModelTypeChangeProcessor`][MTCP] interface, as well as sending commit requests
to the [`ModelTypeWorker`][MTW] on the sync thread via the [`CommitQueue`][CQ]
interface and receiving updates from the same worker via the
[`ModelTypeProcessor`][MTP] interface.
This processor supports types that use a client tag, which is currently
includes all except bookmarks. This means all changes in flight (either incoming
remote changes provided via the [`ModelTypeWorker`][MTW], or local changes
reported by the [`ModelTypeSyncBridge`][MTSB]) must specify a client tag, which
is considered (after being hashed) the main global identifier of a sync entity.
[SMTP]: https://cs.chromium.org/chromium/src/components/sync/model_impl/client_tag_based_model_type_processor.h
[MTSB]: https://cs.chromium.org/chromium/src/components/sync/model/model_type_sync_bridge.h
[MTCP]: https://cs.chromium.org/chromium/src/components/sync/model/model_type_change_processor.h