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

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

Due to allowing users to configure without a passphrase if they don't have
passwords enabled, we can get into a situation where we require encryption of
datatypes, but don't have the cryptographer enabled. When in this situation
we need to ensure we don't corrupt any data. Once the user sets their
passphrase, we will reencrypt everything, which will overwrite unsynced entries
with their encrypted versions, allowing the sync cycle to finish.

BUG=93100
TEST=sync_unit_tests --gtest_filter="*GetCommitIdsFilterEntries*"
Also to manually test: set up sync on a machine with an explicit passphrase. Set up sync without passwords enabled on a separate machine and don't enter the passphrase. Enable encryption on the first client. Attempt to add a bookmark on the second client.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98758

[Nicolas Zea, 2011-08-29 21:35:16]

Please take a look. I had to also slightly change the behavior of
ReEncryptEverything, as we were only previously calling it when setting a new 
passphrase, not providing a required one.

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

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:72: for (iter =
unsynced_handles->begin();
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).

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:79: type ==
syncable::PASSWORDS ||
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]

Comments addressed, PTAL

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
File chrome/browser/sync/engine/get_commit_ids_command.cc (right):

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:72: for (iter =
unsynced_handles->begin();
On 2011/08/29 23:15:31, timsteele wrote:
> 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).

Added TODO, as discussed offline.

http://codereview.chromium.org/7795002/diff/1003/chrome/browser/sync/engine/g...
chrome/browser/sync/engine/get_commit_ids_command.cc:79: type ==
syncable::PASSWORDS ||
On 2011/08/29 23:15:31, timsteele wrote:
> this code looks vaguely familiar... do we run a similar check anywhere else
and
> might benefit from factoring it into a method?  

Done.

[tim (not reviewing), 2011-08-30 00:48:14]

LGTM