[USS] Do not expect GC clear all directive for initial sync

Before this CL, migration to USS for wallet data USS bridge fails
because
 - this bridge does not support incremental updates,
 - a clear-all gc directive is expected for every (non-empty) update
   for such bridges, and
 - the uss migrator does not provide the directive (as it has no clue
   about which data types expect which directives).

This CL exempts initial updates from this rule and allows them to
arrive without any gc directive.

Bug: 876308
Change-Id: I2dd91f077ee3602d88e96c5532c3bb910b48c224
Reviewed-on: https://chromium-review.googlesource.com/1194366
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587206}

components/sync/model_impl/client_tag_based_model_type_processor.cc

@@ -558,24 +558,7 @@ void ClientTagBasedModelTypeProcessor::OnUpdateReceived(
     const UpdateResponseDataList& updates) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (HasClearAllDirective(model_type_state) &&
-      bridge_->SupportsIncrementalUpdates()) {
-    ReportError(ModelError(FROM_HERE,
-                           "Received an update with version watermark for "
-                           "bridge that supports incremental updates"));
-
-    return;
-  } else if (!HasClearAllDirective(model_type_state) &&
-             !bridge_->SupportsIncrementalUpdates()) {
-    // We receive empty updates (without clear all directive) from the server to
-    // indicate nothing changed. We can just ignore these updates for bridges
-    // that don't support incremental updates.
-    if (!updates.empty()) {
-      ReportError(ModelError(FROM_HERE,
-                             "Received an update without version watermark for "
-                             "bridge that does not support incremental "
-                             "updates"));
-    }
+  if (!ValidateUpdate(model_type_state, updates)) {
     return;
   }
 
@@ -788,6 +771,40 @@ void ClientTagBasedModelTypeProcessor::RecommitAllForEncryption(
   }
 }
 
+bool ClientTagBasedModelTypeProcessor::ValidateUpdate(
+    const sync_pb::ModelTypeState& model_type_state,
+    const UpdateResponseDataList& updates) {
+  if (!model_type_state_.initial_sync_done()) {
+    // Due to uss_migrator, initial sync (when migrating from non-USS) does not
+    // contain any gc directives. Thus, we cannot expect the conditions below to
+    // be satisfied. It is okay to skip the check as for an initial sync, the gc
+    // directive does not make any semantical difference.
+    return true;
+  }
+
+  if (HasClearAllDirective(model_type_state) &&
+      bridge_->SupportsIncrementalUpdates()) {
+    ReportError(ModelError(FROM_HERE,
+                           "Received an update with version watermark for "
+                           "bridge that supports incremental updates"));
+
+    return false;
+  } else if (!HasClearAllDirective(model_type_state) &&
+             !bridge_->SupportsIncrementalUpdates()) {
+    // We receive empty updates (without clear all directive) from the server to
+    // indicate nothing changed. We can just ignore these updates for bridges
+    // that don't support incremental updates.
+    if (!updates.empty()) {
+      ReportError(ModelError(FROM_HERE,
+                             "Received an update without version watermark for "
+                             "bridge that does not support incremental "
+                             "updates"));
+    }
+    return false;
+  }
+  return true;
+}
+
 base::Optional<ModelError>
 ClientTagBasedModelTypeProcessor::OnFullUpdateReceived(
     const sync_pb::ModelTypeState& model_type_state,

components/sync/model_impl/client_tag_based_model_type_processor.h

@@ -129,6 +129,12 @@ class ClientTagBasedModelTypeProcessor : public ModelTypeProcessor,
   void RecommitAllForEncryption(std::unordered_set<std::string> already_updated,
                                 MetadataChangeList* metadata_changes);
 
+  // Validates the update specified by the input parameters and returns whether
+  // it should get further processed. If the update is incorrect, this function
+  // also reports an error.
+  bool ValidateUpdate(const sync_pb::ModelTypeState& model_type_state,
+                      const UpdateResponseDataList& updates);
+
   // Handle the first update received from the server after being enabled. If
   // the data type does not support incremental updates, this will be called for
   // any server update.

components/sync/model_impl/client_tag_based_model_type_processor_unittest.cc

@@ -1731,6 +1731,18 @@ TEST_F(FullUpdateClientTagBasedModelTypeProcessorTest,
   EXPECT_EQ(0, bridge()->apply_call_count());
 }
 
+// Tests that the processor correctly handles an initial (non-empty) update
+// without any gc directives (as it happens in the migration to USS).
+TEST_F(FullUpdateClientTagBasedModelTypeProcessorTest,
+       InitialUpdateForFullUpdateBridge) {
+  // Do not set any model type state to emulate that initial sync has not been
+  // done yet.
+  ModelReadyToSync();
+  OnSyncStarting();
+
+  worker()->UpdateFromServer(kHash1, GenerateSpecifics(kKey1, kValue1));
+}
+
 // Tests that the processor reports an error for updates with a version GC
 // directive that are received for types that support incremental updates.
 TEST_F(ClientTagBasedModelTypeProcessorTest,