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

Revert "Fix sync Nigori cache GUID left empty upon mismatch"

This reverts commit cd858876.

Reason for revert: a more aggressive logic can be implemented and
is safer against users that are already in a broken state (empty
cache GUID).

Original change's description:
> 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: Maksim Moskvitin <mmoskvitin@google.com>
> Cr-Commit-Position: refs/heads/master@{#751966}

TBR=mastiz@chromium.org,mmoskvitin@google.com,rushans@google.com

Change-Id: I2de46c2cd71e30f99b3a953c815de8f78d3a568c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1063021
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2112631Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#752028}
parent 4c198109
...@@ -390,11 +390,6 @@ bool NigoriModelTypeProcessor::IsConnectedForTest() const { ...@@ -390,11 +390,6 @@ 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();
} }
...@@ -417,14 +412,13 @@ void NigoriModelTypeProcessor::ConnectIfReady() { ...@@ -417,14 +412,13 @@ void NigoriModelTypeProcessor::ConnectIfReady() {
return; return;
} }
if (model_type_state_.has_cache_guid() && if (!model_type_state_.has_cache_guid()) {
model_type_state_.cache_guid() != activation_request_.cache_guid) { model_type_state_.set_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,7 +59,6 @@ class NigoriModelTypeProcessor : public ModelTypeProcessor, ...@@ -59,7 +59,6 @@ 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,8 +527,6 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldResetDataOnCacheGuidMismatch) { ...@@ -527,8 +527,6 @@ 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