[Sync] Gracefully handle writing to a node when the cryptographer is not ready.

chrome/browser/sync/engine/get_commit_ids_command.cc

Index: chrome/browser/sync/engine/get_commit_ids_command.cc
diff --git a/chrome/browser/sync/engine/get_commit_ids_command.cc b/chrome/browser/sync/engine/get_commit_ids_command.cc
index 903d0e98f71d9149a24769b12017c40e09e0ccc1..4cf3485e45bc13709e62cccb728a1280db3fcb1f 100644
--- a/chrome/browser/sync/engine/get_commit_ids_command.cc
+++ b/chrome/browser/sync/engine/get_commit_ids_command.cc
@@ -9,7 +9,9 @@
 #include <vector>
 
 #include "chrome/browser/sync/engine/syncer_util.h"
+#include "chrome/browser/sync/syncable/directory_manager.h"
 #include "chrome/browser/sync/syncable/syncable.h"
+#include "chrome/browser/sync/util/cryptographer.h"
 
 using std::set;
 using std::vector;
@@ -31,6 +33,15 @@ void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
   syncable::Directory::UnsyncedMetaHandles all_unsynced_handles;
   SyncerUtil::GetUnsyncedEntries(session->write_transaction(),
                                  &all_unsynced_handles);
+
+  Cryptographer *cryptographer =
+      session->context()->directory_manager()->GetCryptographer(
+          session->write_transaction());
+  if (cryptographer) {
+    FilterEntriesNeedingEncryption(cryptographer->GetEncryptedTypes(),
+                                   session->write_transaction(),
+                                   &all_unsynced_handles);
+  }
   StatusController* status = session->status_controller();
   status->set_unsynced_handles(all_unsynced_handles);
 
@@ -46,6 +57,39 @@ void GetCommitIdsCommand::ExecuteImpl(SyncSession* session) {
   status->set_commit_set(*ordered_commit_set_.get());
 }
 
+// We create a new list of unsynced handles which omits all handles to entries
+// that require encryption but are written in plaintext. If any were found we
+// overwrite |unsynced_handles| with this new list, else no change is made.
+// Static.
+void GetCommitIdsCommand::FilterEntriesNeedingEncryption(
+    const syncable::ModelTypeSet& encrypted_types,
+    syncable::BaseTransaction* trans,
+    syncable::Directory::UnsyncedMetaHandles* unsynced_handles) {
+  bool removed_handles = false;
+  syncable::Directory::UnsyncedMetaHandles::iterator iter;
+  syncable::Directory::UnsyncedMetaHandles new_unsynced_handles;
+  new_unsynced_handles.reserve(unsynced_handles->size());
+  for (iter = unsynced_handles->begin();

[tim (not reviewing), 2011/08/29 23:15:31]

Hm. I think we should embed this filter in the existing BuildCommitIds loop
(AddCreates/Deletes/etc)... it kind of sucks to have to iterate through every
unsynced handle again (e.g. on initial sync).

[Nicolas Zea, 2011/08/30 00:44:22]

Added TODO, as discussed offline.

+       iter != unsynced_handles->end();
+       ++iter) {
+    syncable::Entry entry(trans, syncable::GET_BY_HANDLE, *iter);
+    syncable::ModelType type = entry.GetModelType();
+    if (encrypted_types.count(type) == 0 ||
+        !entry.Get(syncable::UNIQUE_SERVER_TAG).empty() ||
+        type == syncable::PASSWORDS ||

[tim (not reviewing), 2011/08/29 23:15:31]

this code looks vaguely familiar... do we run a similar check anywhere else and
might benefit from factoring it into a method?  

[Nicolas Zea, 2011/08/30 00:44:22]

Done.

+        entry.Get(syncable::SPECIFICS).has_encrypted()) {
+      new_unsynced_handles.push_back(*iter);
+    } else {
+      // This entry requires encryption but is not encrypted (possibly due to
+      // the cryptographer not being initialized). Don't add it to our new list
+      // of unsynced handles.
+      removed_handles = true;
+    }
+  }
+  if (removed_handles)
+    *unsynced_handles = new_unsynced_handles;
+}
+
 void GetCommitIdsCommand::AddUncommittedParentsAndTheirPredecessors(
     syncable::BaseTransaction* trans,
     syncable::Id parent_id,