[Sync] Cache encrypted types info in ProfileSyncService

chrome/browser/sync/internal_api/sync_manager.cc

Index: chrome/browser/sync/internal_api/sync_manager.cc
diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index c8435b65b996a08fe7dab7efb01fca927e24a726..98ce5d70c16a6c7a93a32ccbd57735efbf605551 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -119,6 +119,7 @@ namespace sync_api {
 // SyncManager's implementation: SyncManager::SyncInternal
 class SyncManager::SyncInternal
     : public net::NetworkChangeNotifier::IPAddressObserver,
+      public browser_sync::Cryptographer::Observer,
       public sync_notifier::SyncNotifierObserver,
       public JsBackend,
       public SyncEngineEventListener,
@@ -259,6 +260,12 @@ class SyncManager::SyncInternal
   // Open the directory named with username_for_share
   bool OpenDirectory();
 
+  // Cryptographer::Observer implementation.
+  virtual void OnEncryptedTypesChanged(
+      const syncable::ModelTypeSet& encrypted_types,
+      bool encrypt_everything) OVERRIDE;
+  virtual void OnEncryptionComplete() OVERRIDE;
+
   // SyncNotifierObserver implementation.
   virtual void OnNotificationStateChange(
       bool notifications_enabled) OVERRIDE;
@@ -439,15 +446,6 @@ class SyncManager::SyncInternal
 
   void ReEncryptEverything(WriteTransaction* trans);
 
-  // Initializes (bootstraps) the Cryptographer if NIGORI has finished
-  // initial sync so that it can immediately start encrypting / decrypting.
-  // If the restored key is incompatible with the current version of the NIGORI
-  // node (which could happen if a restart occurred just after an update to
-  // NIGORI was downloaded and the user must enter a new passphrase to decrypt)
-  // then we will raise OnPassphraseRequired and set pending keys for
-  // decryption.  Otherwise, the cryptographer is made ready (is_ready()).
-  void BootstrapEncryption(const std::string& restored_key_for_bootstrapping);
-
   // Called for every notification. This updates the notification statistics
   // to be displayed in about:sync.
   void UpdateNotificationInfo(
@@ -688,7 +686,7 @@ void SyncManager::EnableEncryptEverything() {
   data_->EncryptDataTypes(syncable::ModelTypeSet());
 }
 
-bool SyncManager::EncryptEverythingEnabled() const {
+bool SyncManager::EncryptEverythingEnabledForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return trans.GetCryptographer()->encrypt_everything();
 }
@@ -810,8 +808,14 @@ bool SyncManager::SyncInternal::Init(
 
     initialized_ = true;
 
-    // The following calls check that initialized_ is true.
-    BootstrapEncryption(restored_key_for_bootstrapping);
+    // The following call checks that initialized_ is true.

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

Something like "Getting the user share to open a transaction DCHECKS that
initialized_ is true." Also, don't think there's really a need for the braces
since we're at the end of the if statement.

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

Done.

+    {
+      // Cryptographer should only be accessed while holding a
+      // transaction.
+      ReadTransaction trans(FROM_HERE, GetUserShare());
+      trans.GetCryptographer()->Bootstrap(restored_key_for_bootstrapping);
+      trans.GetCryptographer()->AddObserver(this);
+    }
   }
 
   // Notify that initialization is complete. Note: This should be the last to
@@ -832,22 +836,12 @@ bool SyncManager::SyncInternal::Init(
   return signed_in;
 }
 
-void SyncManager::SyncInternal::BootstrapEncryption(
-    const std::string& restored_key_for_bootstrapping) {
-  // Cryptographer should only be accessed while holding a transaction.
-  ReadTransaction trans(FROM_HERE, GetUserShare());
-  Cryptographer* cryptographer = trans.GetCryptographer();
-
-  // Set the bootstrap token before bailing out if nigori node is not there.
-  // This could happen if server asked us to migrate nigri.
-  cryptographer->Bootstrap(restored_key_for_bootstrapping);
-}
-
 bool SyncManager::SyncInternal::UpdateCryptographerFromNigori() {
   DCHECK(initialized_);
   syncable::ScopedDirLookup lookup(dir_manager(), username_for_share());
   if (!lookup.good()) {
-    NOTREACHED() << "BootstrapEncryption: lookup not good so bailing out";
+    NOTREACHED()
+        << "UpdateCryptographerFromNigori: lookup not good so bailing out";
     return false;
   }
   if (!lookup->initial_sync_ended_for_type(syncable::NIGORI))
@@ -1139,7 +1133,6 @@ void SyncManager::SyncInternal::EncryptDataTypes(
   // We reencrypt everything regardless of whether the set of encrypted
   // types changed to ensure that any stray unencrypted entries are overwritten.
   ReEncryptEverything(&trans);
-  return;
 }
 
 // TODO(zea): Add unit tests that ensure no sync changes are made when not
@@ -1216,8 +1209,7 @@ void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) {
     }
   }
 
-  FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
-                    OnEncryptionComplete(encrypted_types));
+  cryptographer->MarkEncryptionComplete();
 }
 
 SyncManager::~SyncManager() {
@@ -1294,12 +1286,14 @@ void SyncManager::SyncInternal::ShutdownOnSyncThread() {
   observing_ip_address_changes_ = false;
 
   if (dir_manager()) {
-    syncable::ScopedDirLookup lookup(dir_manager(), username_for_share());
-    if (lookup.good()) {
-      lookup->RemoveTransactionObserver(&js_mutation_event_observer_);
+    {
+      // Cryptographer should only be accessed while holding a
+      // transaction.
+      ReadTransaction trans(FROM_HERE, GetUserShare());
+      trans.GetCryptographer()->RemoveObserver(this);
+      trans.GetLookup()->
+          RemoveTransactionObserver(&js_mutation_event_observer_);
       RemoveChangeObserver(&js_mutation_event_observer_);
-    } else {
-      NOTREACHED();
     }
     dir_manager()->FinalSaveChangesForAll();
     dir_manager()->Close(username_for_share());
@@ -1622,11 +1616,6 @@ void SyncManager::SyncInternal::OnSyncEngineEvent(
       allstatus_.SetCryptographerReady(cryptographer->is_ready());
       allstatus_.SetCryptoHasPendingKeys(cryptographer->has_pending_keys());
       allstatus_.SetEncryptedTypes(cryptographer->GetEncryptedTypes());
-
-      // If everything is in order(we have the passphrase) then there is no
-      // need to inform the listeners. They will just wait for sync
-      // completion event and if no errors have been raised it means
-      // encryption was succesful.
     }
 
     if (!initialized_) {
@@ -1636,7 +1625,7 @@ void SyncManager::SyncInternal::OnSyncEngineEvent(
     }
 
     if (!event.snapshot->has_more_to_sync) {
-      VLOG(1) << "OnSyncCycleCompleted sent";
+      VLOG(1) << "Sending OnSyncCycleCompleted";
       FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
                         OnSyncCycleCompleted(event.snapshot));
     }
@@ -1876,6 +1865,21 @@ JsArgList SyncManager::SyncInternal::FindNodesContainingString(
   return JsArgList(&return_args);
 }
 
+void SyncManager::SyncInternal::OnEncryptedTypesChanged(
+    const syncable::ModelTypeSet& encrypted_types,
+    bool encrypt_everything) {
+  // NOTE: We're in a transaction.
+  FOR_EACH_OBSERVER(
+      SyncManager::Observer, observers_,
+      OnEncryptedTypesChanged(encrypted_types, encrypt_everything));
+}
+
+void SyncManager::SyncInternal::OnEncryptionComplete() {
+  // NOTE: We're in a transaction.
+  FOR_EACH_OBSERVER(
+      SyncManager::Observer, observers_, OnEncryptionComplete());
+}
+
 void SyncManager::SyncInternal::OnNotificationStateChange(
     bool notifications_enabled) {
   VLOG(1) << "P2P: Notifications enabled = "
@@ -2007,7 +2011,7 @@ void SyncManager::RefreshEncryption() {
     data_->EncryptDataTypes(syncable::ModelTypeSet());
 }
 
-syncable::ModelTypeSet SyncManager::GetEncryptedDataTypes() const {
+syncable::ModelTypeSet SyncManager::GetEncryptedDataTypesForTest() const {
   ReadTransaction trans(FROM_HERE, GetUserShare());
   return GetEncryptedTypes(&trans);
 }