[Sync] Cache encrypted types info in ProfileSyncService

chrome/browser/sync/profile_sync_service.cc

Index: chrome/browser/sync/profile_sync_service.cc
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 500ec0910b7cd9fdebfcd47073c033a2c8064401..ec7f33fd80d581670a619cf2cafa6f319798b9f7 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -111,6 +111,8 @@ ProfileSyncService::ProfileSyncService(ProfileSyncFactory* factory,
       scoped_runnable_method_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       expect_sync_configuration_aborted_(false),
       clear_server_data_state_(CLEAR_NOT_STARTED),
+      encrypted_types_(browser_sync::Cryptographer::SensitiveTypes()),
+      encrypt_everything_(false),
       encryption_pending_(false),
       auto_start_enabled_(false),
       failed_datatypes_handler_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
@@ -423,6 +425,8 @@ void ProfileSyncService::Shutdown(bool sync_disabled) {
   backend_initialized_ = false;
   cached_passphrases_ = CachedPassphrases();
   encryption_pending_ = false;
+  encrypt_everything_ = false;
+  encrypted_types_ = browser_sync::Cryptographer::SensitiveTypes();
   passphrase_required_reason_ = sync_api::REASON_PASSPHRASE_NOT_REQUIRED;
   last_attempted_user_email_.clear();
   last_auth_error_ = GoogleServiceAuthError::None();
@@ -834,8 +838,22 @@ void ProfileSyncService::ResolvePassphraseRequired() {
   NotifyObservers();
 }
 
-void ProfileSyncService::OnEncryptionComplete(
-    const syncable::ModelTypeSet& encrypted_types) {
+void ProfileSyncService::OnEncryptedTypesChanged(
+    const syncable::ModelTypeSet& encrypted_types,
+    bool encrypt_everything) {
+  encrypted_types_ = encrypted_types;
+  encrypt_everything_ = encrypt_everything;
+  VLOG(2) << "Encrypted types changed to "

[Nicolas Zea, 2011/10/21 14:29:07]

Since these now happen relatively rarely, I'd prefer VLOG(1) for this and
encryption complete.

[akalin, 2011/10/22 03:28:38]

Done.

+          << syncable::ModelTypeSetToString(encrypted_types_)
+          << " (encrypt everything is set to "
+          << (encrypt_everything_ ? "true" : "false") << ")";
+  DCHECK_GT(encrypted_types_.count(syncable::PASSWORDS), 0u);
+}
+
+void ProfileSyncService::OnEncryptionComplete() {
+  VLOG(2) << "Encryption complete for "
+          << syncable::ModelTypeSetToString(encrypted_types_);
+  DCHECK_GT(encrypted_types_.count(syncable::PASSWORDS), 0u);
   if (encryption_pending_) {
     syncable::ModelTypeSet registered_types;
     GetRegisteredDataTypes(&registered_types);
@@ -843,7 +861,7 @@ void ProfileSyncService::OnEncryptionComplete(
     for (syncable::ModelTypeSet::const_iterator it = registered_types.begin();

[Nicolas Zea, 2011/10/21 14:29:07]

I think now that we're receiving the encrypt_everything value in
OnEncryptedTypesChanged, we can key of that instead of doing this comparison. If
encryption_pending_ is true, we're waiting until encrypt_everything becomes
true. Once that is true, we can be sure all registered types will have been
encrypted (ReEncryptEverything will always get the current set of encrypted
types and encrypt all registered types).

[akalin, 2011/10/22 03:28:38]

Yeah.  But we still need to wait for encryption to finish, right?  I implemented
that.

          it != registered_types.end();
          ++it) {
-      if (encrypted_types.count(*it) == 0) {
+      if (encrypted_types_.count(*it) == 0) {
         // One of our types is not yet encrypted - keep waiting.
         encryption_complete = false;
         break;
@@ -854,9 +872,11 @@ void ProfileSyncService::OnEncryptionComplete(
       // The user had chosen to encrypt datatypes. This is the last thing to
       // complete, so now that we're done notify the UI.
       wizard_.Step(SyncSetupWizard::DONE);
+      // This is to nudge the integration tests when encryption is
+      // finished.
+      NotifyObservers();
     }
   }
-  NotifyObservers();
 }
 
 void ProfileSyncService::OnMigrationNeededForTypes(
@@ -1320,37 +1340,32 @@ void ProfileSyncService::SetPassphrase(const std::string& passphrase,
 }
 
 void ProfileSyncService::SetEncryptEverything(bool encrypt_everything) {
+  // We may be called during the setup process before we're

[Nicolas Zea, 2011/10/21 14:29:07]

I understand GetEncryptedTypes being called by the UI, but SetEncryptEverything
is only called once we're already configuring, which is dependent on having been
initialized (OnBackendInitialized triggers the Step(SYNC_EVERYTHING)), so I
don't think this holds true.

[akalin, 2011/10/22 03:28:38]

Called by some unittests; see comment. :/

+  // initialized.
   encryption_pending_ = encrypt_everything;
+  // Callers shouldn't try to disable encrypt everything once it has
+  // already succeeded.
+  DCHECK(!encrypt_everything_);
 }
 
 bool ProfileSyncService::encryption_pending() const {
+  // We may be called during the setup process before we're
+  // initialized.

[Nicolas Zea, 2011/10/21 14:29:07]

Comment that this is because it's used by IsEncryptedDatatypeEnabled, which is
itself used by IsPassphraseRequiredForDecryption. :-/

[akalin, 2011/10/22 03:28:38]

Done.

   return encryption_pending_;
 }
 
 bool ProfileSyncService::EncryptEverythingEnabled() const {
-  if (!backend_.get() || !backend_initialized_) {
-    NOTREACHED() << "Cannot check encryption without initialized backend.";
-    return false;
-  }
-  return backend_->EncryptEverythingEnabled();
+  DCHECK(backend_initialized_);
+  return encrypt_everything_;
 }
 
-// This will open a transaction to get the encrypted types. Do not call this
-// if you already have a transaction open.
 void ProfileSyncService::GetEncryptedDataTypes(
     syncable::ModelTypeSet* encrypted_types) const {
   CHECK(encrypted_types);
-  if (backend_.get()) {
-    *encrypted_types = backend_->GetEncryptedDataTypes();
-    DCHECK(encrypted_types->count(syncable::PASSWORDS));
-  } else {
-    // Either we are in an unrecoverable error or the sync is not yet done
-    // initializing. In either case just return the sensitive types. During
-    // sync initialization the UI might need to know what our encrypted
-    // types are.
-    *encrypted_types = browser_sync::Cryptographer::SensitiveTypes();
-    DCHECK(encrypted_types->count(syncable::PASSWORDS));
-  }
+  // We may be called during the setup process before we're
+  // initialized.

[Nicolas Zea, 2011/10/21 14:29:07]

Add something like "In this case, we'll default to the sensitive types."

[akalin, 2011/10/22 03:28:38]

Done.

+  *encrypted_types = encrypted_types_;
+  DCHECK_GT(encrypted_types->count(syncable::PASSWORDS), 0u);
 }
 
 void ProfileSyncService::OnSyncManagedPrefChange(bool is_sync_managed) {