Commit cd858876 authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Fix sync Nigori cache GUID left empty upon mismatch

In the error-handling codepath where a mismatch is found on the cache
GUID, when sync is starting for Nigori, the new cache GUID should be
stored in the ModelTypeState. Otherwise, it would remain empty, which
means the next sync start the mismatch detection doesn't take place.

This can explain cache-GUID mismatches being ignored, and bad
consequences such as violating the sync protocol by sending a non-empty
progress marker despite the sync birthday being empty.

Change-Id: I48d4b35a61dd20f56c416779bee686e1d582606d
Bug: 1063021
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2111090
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarMaksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#751966}
parent a94ce1b9
...@@ -390,6 +390,11 @@ bool NigoriModelTypeProcessor::IsConnectedForTest() const { ...@@ -390,6 +390,11 @@ bool NigoriModelTypeProcessor::IsConnectedForTest() const {
return IsConnected(); return IsConnected();
} }
const sync_pb::ModelTypeState&
NigoriModelTypeProcessor::GetModelTypeStateForTest() {
return model_type_state_;
}
bool NigoriModelTypeProcessor::IsTrackingMetadata() { bool NigoriModelTypeProcessor::IsTrackingMetadata() {
return model_type_state_.initial_sync_done(); return model_type_state_.initial_sync_done();
} }
...@@ -412,13 +417,14 @@ void NigoriModelTypeProcessor::ConnectIfReady() { ...@@ -412,13 +417,14 @@ void NigoriModelTypeProcessor::ConnectIfReady() {
return; return;
} }
if (!model_type_state_.has_cache_guid()) { if (model_type_state_.has_cache_guid() &&
model_type_state_.set_cache_guid(activation_request_.cache_guid); model_type_state_.cache_guid() != activation_request_.cache_guid) {
} else if (model_type_state_.cache_guid() != activation_request_.cache_guid) {
ClearMetadataAndReset(); ClearMetadataAndReset();
DCHECK(model_ready_to_sync_); DCHECK(model_ready_to_sync_);
} }
model_type_state_.set_cache_guid(activation_request_.cache_guid);
// Cache GUID verification earlier above guarantees the user is the same. // Cache GUID verification earlier above guarantees the user is the same.
model_type_state_.set_authenticated_account_id( model_type_state_.set_authenticated_account_id(
activation_request_.authenticated_account_id.ToString()); activation_request_.authenticated_account_id.ToString());
......
...@@ -59,6 +59,7 @@ class NigoriModelTypeProcessor : public ModelTypeProcessor, ...@@ -59,6 +59,7 @@ class NigoriModelTypeProcessor : public ModelTypeProcessor,
bool IsTrackingMetadata() override; bool IsTrackingMetadata() override;
bool IsConnectedForTest() const; bool IsConnectedForTest() const;
const sync_pb::ModelTypeState& GetModelTypeStateForTest();
private: private:
// Returns true if the handshake with sync thread is complete. // Returns true if the handshake with sync thread is complete.
......
...@@ -527,6 +527,8 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldResetDataOnCacheGuidMismatch) { ...@@ -527,6 +527,8 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldResetDataOnCacheGuidMismatch) {
processor()->OnSyncStarting(request, base::DoNothing()); processor()->OnSyncStarting(request, base::DoNothing());
EXPECT_FALSE(processor()->IsTrackingMetadata()); EXPECT_FALSE(processor()->IsTrackingMetadata());
EXPECT_EQ(processor()->GetModelTypeStateForTest().cache_guid(),
kOtherCacheGuid);
EXPECT_FALSE(ProcessorHasEntity()); EXPECT_FALSE(ProcessorHasEntity());
......
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