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

Relax ModelTypeWorker DCHECK when processing updates

Nothing seems to guarantee, strictly speaking, that the split between
ApplyUpdates() vs PassiveApplyUpdates() maps to initial sync having been
done.

In practice, that's almost certainly the case, we some flaky tests do
suggest the assertion gets violated. Circumstances that could contribute
to this failure include polling sync cycles and failing configuration
cycles, which would all lead to a non-configuration cycle downloading
the initial data for a sync datatype, and that being passed via
ApplyUpdates().

Since the DCHECK is not guaranteed to hold, let's just relax it.

TBR=treib@chromium.org

Bug: 1002549
Change-Id: I901886cb34b68266a024c259328be204a6f88fa5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807294Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#696903}
parent 3d2be4ff
......@@ -39,6 +39,9 @@ class GetUpdatesDelegate {
virtual std::unique_ptr<ProtocolEvent> GetNetworkRequestEvent(
base::Time timestamp,
const sync_pb::ClientToServerMessage& request) const = 0;
private:
DISALLOW_COPY_AND_ASSIGN(GetUpdatesDelegate);
};
// Functionality specific to the normal GetUpdate request.
......
......@@ -379,21 +379,21 @@ ModelTypeWorker::DecryptionStatus ModelTypeWorker::PopulateUpdateResponseData(
void ModelTypeWorker::ApplyUpdates(StatusController* status) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// This should only ever be called after one PassiveApplyUpdates.
DCHECK(model_type_state_.initial_sync_done())
<< "ApplyUpdates() called without initial sync being done for "
<< ModelTypeToString(type_);
// Indicate to the processor that the initial download is done. The initial
// sync technically isn't done yet but by the time this value is persisted to
// disk on the model thread it will be.
//
// This should be mostly relevant for the call from PassiveApplyUpdates(), but
// in rare cases we may end up receiving initial updates outside configuration
// cycles (e.g. polling cycles).
model_type_state_.set_initial_sync_done(true);
// Download cycle is done, pass all updates to the processor.
ApplyPendingUpdates();
}
void ModelTypeWorker::PassiveApplyUpdates(StatusController* status) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Indicate to the processor that the initial download is done. The initial
// sync technically isn't done yet but by the time this value is persisted to
// disk on the model thread it will be.
model_type_state_.set_initial_sync_done(true);
ApplyPendingUpdates();
ApplyUpdates(status);
}
void ModelTypeWorker::EncryptionAcceptedMaybeApplyUpdates() {
......
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