[Sync] The cryptographer should be able to handle empty or old nigori nodes.

[Sync] The cryptographer should be able to handle empty or old nigori nodes.

This includes never unsetting the encryption status of a type once it has been
set.

BUG=104218
TEST=unit_tests


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

[Nicolas Zea, 2011-11-16 19:45:47]

ping!

[akalin, 2011-11-17 03:09:20]

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
File chrome/browser/sync/util/cryptographer.cc (right):

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
chrome/browser/sync/util/cryptographer.cc:384: void
Cryptographer::SetEncryptedTypes(
Rename this to "MergeEncryptedTypes"

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
chrome/browser/sync/util/cryptographer.cc:386: if (encrypted_types_ ==
encrypted_types) {
should be:

if (std::includes(encrypted_types_.begin(), ..., encrypted_types.begin() ...)) {
  return;
}

[Nicolas Zea, 2011-11-17 05:09:59]

PTAL

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
File chrome/browser/sync/util/cryptographer.cc (right):

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
chrome/browser/sync/util/cryptographer.cc:384: void
Cryptographer::SetEncryptedTypes(
On 2011/11/17 03:09:20, akalin wrote:
> Rename this to "MergeEncryptedTypes"

Done.

http://codereview.chromium.org/8564032/diff/4/chrome/browser/sync/util/crypto...
chrome/browser/sync/util/cryptographer.cc:386: if (encrypted_types_ ==
encrypted_types) {
On 2011/11/17 03:09:20, akalin wrote:
> should be:
> 
> if (std::includes(encrypted_types_.begin(), ..., encrypted_types.begin() ...))
{
>   return;
> }

Done.

[akalin, 2011-11-17 11:29:18]

LGTM, assuming you fix the diff

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.h (left):

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:193: // Changes the set of encrypted
types and emits a notification if
Update comment, maybe:

// Merges the given set with the current set of encrypted types and emits a
notification if necessary.

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer_unittest.cc (left):

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer_unittest.cc:236: EXPECT_CALL(observer,
What's up with this diff?  You added this new empty nigori test in this CL,
right?

[Nicolas Zea, 2011-11-17 19:03:15]

Done. Committing once trybots go green.

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.h (left):

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:193: // Changes the set of encrypted
types and emits a notification if
On 2011/11/17 11:29:18, akalin wrote:
> Update comment, maybe:
> 
> // Merges the given set with the current set of encrypted types and emits a
> notification if necessary.

Done.

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer_unittest.cc (left):

http://codereview.chromium.org/8564032/diff/9001/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer_unittest.cc:236: EXPECT_CALL(observer,
On 2011/11/17 11:29:18, akalin wrote:
> What's up with this diff?  You added this new empty nigori test in this CL,
> right?
Weird. Think git got confused as to which branch it was tracking. Fixed.

[commit-bot: I haz the power, 2011-11-17 21:05:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/zea@chromium.org/8564032/10004

[commit-bot: I haz the power, 2011-11-17 22:41:27]

Change committed as 110578