[Sync] Don't require keystore migration time to be set in nigori node

sync/internal_api/sync_encryption_handler_impl.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "sync/internal_api/sync_encryption_handler_impl.h"
 
 #include <queue>
 #include <string>
 
 #include "base/base64.h"
@@ skipping 53 matching lines @@
   NOT_MIGRATED_CRYPTO_NOT_READY,
   NOT_MIGRATED_NO_KEYSTORE_KEY,
   NOT_MIGRATED_UNKNOWN_REASON,
   MIGRATION_STATE_SIZE,
 };
 
 // The new passphrase state is sufficient to determine whether a nigori node
 // is migrated to support keystore encryption. In addition though, we also
 // want to verify the conditions for proper keystore encryption functionality.
 // 1. Passphrase state is set.
 // 2. Migration time is set.

[pavely, 2015/07/21 18:08:46]

Item 2. from comment should be removed too.

[Nicolas Zea, 2015/07/21 19:41:26]

Done.

 // 3. Frozen keybag is true
 // 4. If passphrase state is keystore, keystore_decryptor_token is set.
 bool IsNigoriMigratedToKeystore(const sync_pb::NigoriSpecifics& nigori) {
   if (!nigori.has_passphrase_type())
     return false;
-  if (!nigori.has_keystore_migration_time())
-    return false;
   if (!nigori.keybag_is_frozen())
     return false;
   if (nigori.passphrase_type() ==
           sync_pb::NigoriSpecifics::IMPLICIT_PASSPHRASE)
     return false;
   if (nigori.passphrase_type() ==
           sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE &&
       nigori.keystore_decryptor_token().blob().empty())
     return false;
-  if (!nigori.has_keystore_migration_time())
-    return false;
   return true;
 }
 
 PassphraseType ProtoPassphraseTypeToEnum(
     sync_pb::NigoriSpecifics::PassphraseType type) {
   switch(type) {
     case sync_pb::NigoriSpecifics::IMPLICIT_PASSPHRASE:
       return IMPLICIT_PASSPHRASE;
     case sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE:
       return KEYSTORE_PASSPHRASE;
@@ skipping 786 matching lines @@
   DVLOG(1) << "Applying nigori node update.";
   bool nigori_types_need_update = !UpdateEncryptedTypesFromNigori(nigori,
                                                                   trans);
 
   if (nigori.custom_passphrase_time() != 0) {
     custom_passphrase_time_ =
         ProtoTimeToTime(nigori.custom_passphrase_time());
   }
   bool is_nigori_migrated = IsNigoriMigratedToKeystore(nigori);
   if (is_nigori_migrated) {
-    DCHECK(nigori.has_keystore_migration_time());
     migration_time_ = ProtoTimeToTime(nigori.keystore_migration_time());

[pavely, 2015/07/21 18:08:46]

Do we use migration_time_ anywhere other than displaying it in about:sync?
With this change migration_time_ might be null.

[Nicolas Zea, 2015/07/21 19:41:26]

It's only in about:sync. And yeah, I'm okay with it being null there.

     PassphraseType nigori_passphrase_type =
         ProtoPassphraseTypeToEnum(nigori.passphrase_type());
 
     // Only update the local passphrase state if it's a valid transition:
     // - implicit -> keystore
     // - implicit -> frozen implicit
     // - implicit -> custom
     // - keystore -> custom
     // Note: frozen implicit -> custom is not technically a valid transition,
     // but we let it through here as well in case future versions do add support
@@ skipping 795 matching lines @@
 void SyncEncryptionHandlerImpl::UpdateNigoriForTransitionToPassphraseEncryption(
     WriteTransaction* trans) {
   DCHECK(trans);
   if (clear_data_option_ != PASSPHRASE_TRANSITION_CLEAR_DATA)
     return;
   // TODO(maniscalco): Update the Nigori node to record the fact the user has
   // begun the transition to passphrase encryption (crbug.com/505917).
 }
 
 }  // namespace browser_sync