Avoid DCHECK failure during ModelTypeWorker decryption

It's unclear why the DCHECK removed in this patch is guaranteed to hold
true, and in fact some tests run into it. Instead, let's defer
application of updates until all pending ones can be decrypted.

According to an old (previously deprecated) UMA metric,
Sync.WorkerApplyHasEncryptedUpdates, the condition doesn't meet about
once per million times. If this only affects the small subset of users
with custom passphrase enabled, which is likely, then it's non-
negligible.

This may (in rare cases) differ from the cryptographer being ready,
specially during sync cyles where NIGORI updates are not requested.

Bug: 873902
Change-Id: Ic70efdad3b72e9387bac50e782a51da09e532857
Reviewed-on: https://chromium-review.googlesource.com/c/1335933Reviewed-by: Pavel Yatsuk <pavely@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609243}

components/sync/engine_impl/model_type_worker.cc

@@ -392,14 +392,13 @@ void ModelTypeWorker::EncryptionAcceptedMaybeApplyUpdates() {
 }
 
 void ModelTypeWorker::ApplyPendingUpdates() {
-  if (BlockForEncryption())
+  if (!entries_pending_decryption_.empty())
     return;
+
   DVLOG(1) << ModelTypeToString(type_) << ": "
            << base::StringPrintf("Delivering %" PRIuS " applicable updates.",
                                  pending_updates_.size());
 
-  DCHECK(entries_pending_decryption_.empty());
-
   const bool contains_duplicate_server_ids =
       ContainsDuplicateServerID(pending_updates_);
   const bool contains_duplicate_client_tag_hashes =

components/sync/engine_impl/model_type_worker_unittest.cc

@@ -1109,9 +1109,12 @@ TEST_F(ModelTypeWorkerTest, EncryptionBlocksUpdates) {
   AddPendingKey();
   EXPECT_EQ(0U, processor()->GetNumUpdateResponses());
 
-  // Update progress marker, should be blocked.
-  server()->SetProgressMarkerToken("token2");
-  DeliverRawUpdates(SyncEntityList());
+  // Receive an encrypted update with that new key, which we can't access.
+  SetUpdateEncryptionFilter(1);
+  TriggerUpdateFromServer(10, kTag1, kValue1);
+
+  // At this point, the cryptographer does not have access to the key, so the
+  // updates will be undecryptable. This should block all updates.
   EXPECT_EQ(0U, processor()->GetNumUpdateResponses());
 
   // Update local cryptographer, verify everything is pushed to processor.
@@ -1268,7 +1271,7 @@ TEST_F(ModelTypeWorkerTest, ReceiveUndecryptableEntries) {
   // Receive a new foreign encryption key that we can't decrypt.
   AddPendingKey();
 
-  // Receive an encrypted with that new key, which we can't access.
+  // Receive an encrypted update with that new key, which we can't access.
   SetUpdateEncryptionFilter(1);
   TriggerUpdateFromServer(10, kTag1, kValue1);
 
@@ -1802,7 +1805,7 @@ TEST_F(ModelTypeWorkerPasswordsTest, ReceiveUndecryptablePasswordEntries) {
   sync_pb::EntitySpecifics encrypted_specifics =
       EncryptPasswordSpecifics(GetNthKeyParams(1), unencrypted_password);
 
-  // Receive an encrypted with that new key, which we can't access.
+  // Receive an encrypted update with that new key, which we can't access.
   SyncEntity entity = server()->UpdateFromServer(
       /*version_offset=*/10, kHash1, encrypted_specifics);
   worker()->ProcessGetUpdatesResponse(server()->GetProgress(),