[Sync] Cache encrypted types info in ProfileSyncService

[Sync] Cache encrypted types info in ProfileSyncService

Make Cryptographer emit notifications whenever the set of encrypted types changes and also when encryption is complete.  Propagate that up to the ProfileSyncService.

Since retrieving the encrypted types info requires holding a transaction,
that may lead to deadlocks.  This prevents that.

Remove some unnecessary functions and append ForTest to some other ones.

BUG=95619, 100698
TEST=

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

[akalin, 2011-10-19 23:32:33]

+zea for review

+tim fyi

[akalin, 2011-10-20 02:58:52]

On 2011/10/19 23:32:33, akalin wrote:
> +zea for review
> 
> +tim fyi

Oh, look integration test failures, what a surprise.

Looking.

[akalin, 2011-10-20 04:07:51]

Issues fixed, PTAL

On 2011/10/20 02:58:52, akalin wrote:
> On 2011/10/19 23:32:33, akalin wrote:
> > +zea for review
> > 
> > +tim fyi
> 
> Oh, look integration test failures, what a surprise.
> 
> Looking.

[Nicolas Zea, 2011-10-20 18:40:41]

Those integration test failures are a bit troublesome, seeing as how they're the
same on linux and win and dont seem to show up on previous runs.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/sync_manager.cc:1642: VLOG(1) << "Sending
OnEncryptionComplete";
Should probably be VLOG(2), since this is going to happen on nearly every sync
cycle.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1343: encrypt_everything_ = false;
presumably encrypt_everything_ should already be false here right? perhaps
change this to a DCHECK?

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1361: *encrypted_types =
encrypted_types_;
Is it valid to call this before the backend is initialized? Perhaps add a
DCHECK? We won't crash anymore, but it's still not the correct set of encrypted
types.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.h (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:210:
sync_api::PassphraseRequiredReason reason) OVERRIDE;
I think we may need to send the set of encrypted types here too. Namely, if
another users encrypts new types, we'll receive OnPassphraseRequired but not
OnEncryptionComplete. So we'll think we still have the old set of encrypted
types.

[akalin, 2011-10-21 02:24:17]

Reworked this patch substantially to emit the encryption notifications from the
Cryptographer.  That turned out to be necessary.

PTAL.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/sync_manager.cc:1642: VLOG(1) << "Sending
OnEncryptionComplete";
On 2011/10/20 18:40:41, nzea wrote:
> Should probably be VLOG(2), since this is going to happen on nearly every sync
> cycle.

Removed.  Probably not necessary.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1343: encrypt_everything_ = false;
On 2011/10/20 18:40:41, nzea wrote:
> presumably encrypt_everything_ should already be false here right? perhaps
> change this to a DCHECK?

Done.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1361: *encrypted_types =
encrypted_types_;
On 2011/10/20 18:40:41, nzea wrote:
> Is it valid to call this before the backend is initialized? Perhaps add a
> DCHECK? We won't crash anymore, but it's still not the correct set of
encrypted
> types. 

Yeah, the UI functions call it.  Sigh.

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.h (right):

http://codereview.chromium.org/8356026/diff/2002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:210:
sync_api::PassphraseRequiredReason reason) OVERRIDE;
On 2011/10/20 18:40:41, nzea wrote:
> I think we may need to send the set of encrypted types here too. Namely, if
> another users encrypts new types, we'll receive OnPassphraseRequired but not
> OnEncryptionComplete. So we'll think we still have the old set of encrypted
> types.

Right.  Added new notification OnEncryptedTypesChanged.

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

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:501: NewRunnableMethod(this,
&Core::NotifyEncryptedTypesChanged,
base::bind

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:100: // OnEncryptedTypesChanged.
I'm not sure I like enforcing this. See comments in cryptographer

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:456: // become aware of new
datatypes requiring encryption.
remove the "has become aware of new datatypes..."

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/sync_manager.cc:811: // The following call
checks that initialized_ is true.
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.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/syncapi_unittest.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/syncapi_unittest.cc:1218:
GetAllRealModelTypes(), true)).Times(3);
Enforcing the number of times this gets hit (here and elsewhere) seems a bit
brittle. It also seems odd that you'd get this hit three times, when the
encrypted types presumably only changes once.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/js/js_sy...
File chrome/browser/sync/js/js_sync_manager_observer_unittest.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/js/js_sy...
chrome/browser/sync/js/js_sync_manager_observer_unittest.cc:215:
HasDetailsAsDictionary(expected_details)));
indent

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:846: VLOG(2) << "Encrypted types
changed to "
Since these now happen relatively rarely, I'd prefer VLOG(1) for this and
encryption complete.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:861: for
(syncable::ModelTypeSet::const_iterator it = registered_types.begin();
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).

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1343: // We may be called during the
setup process before we're
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.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1353: // initialized.
Comment that this is because it's used by IsEncryptedDatatypeEnabled, which is
itself used by IsPassphraseRequiredForDecryption. :-/

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1366: // initialized.
Add something like "In this case, we'll default to the sensitive types."

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:676: // everything.  Initialized on
OnBackendInitialized and updated by
Perhaps include a comment that the Cryptographer::SensitiveTypes will always be
part of the set of encrypted types.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.cc:328:
EmitEncryptedTypesChangeNotification();
Perhaps only emit if the types actually changed? (add and compare against
old_encrypted_types) This applies to here and in set_encrypt_everything and
SetEncryptedTypes

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:57: // Guaranteed to be called
immeduately after a call of
immediately

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:177: void
SetEncryptedTypes(syncable::ModelTypeSet new_types);
This isn't really used any more (outside of tests), although it does get called
unnecessarily from SyncInternal::EncryptDataTypes (which only ever gets called
with an empty set now as a way of forcing an update of the nigori and triggering
reencryption). Cleaning that up is probably outside the scope of this patch
though, so perhaps just add a todo there and assign it to me.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:183: // notifiation.
Is it necessary to always trigger the OnEncryptedTypesChanged as well (I suppose
this is why the unit tests are based on receiving it 3 times)? If they had
changed, we would have triggered it organically. If they hadn't, why force the
notification?

In the case of locally turning on encryption, we're guaranteed to have triggered
an OnEncryptedTypesChanged (assuming they did actually change) before sending
the OnEncryptionComplete. If they hadn't changed, then the PSS must already have
the current encryption info (by way of RefreshEncryption). Either way, the PSS
will have the set of encrypted types/encrypt everything that matches the one
OnEncryptionComplete used, so the setup flow can proceed accordingly.

Similarly, when we change the passphrase, we trigger a reencryption, but the
encrypted types aren't changing. I'm not sure there's any benefit to forcing the
notification in that case.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:191: void
EmitEncryptedTypesChangeNotification();
EncryptedTypesChange... -> EncryptedTypesChanged...

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

All comments addressed, PTAL

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.cc:501: NewRunnableMethod(this,
&Core::NotifyEncryptedTypesChanged,
On 2011/10/21 14:29:07, nzea wrote:
> base::bind

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
File chrome/browser/sync/glue/sync_backend_host.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:100: // OnEncryptedTypesChanged.
On 2011/10/21 14:29:07, nzea wrote:
> I'm not sure I like enforcing this. See comments in cryptographer

Yeah, me neither.  Removed that condition.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/glue/syn...
chrome/browser/sync/glue/sync_backend_host.h:456: // become aware of new
datatypes requiring encryption.
On 2011/10/21 14:29:07, nzea wrote:
> remove the "has become aware of new datatypes..."

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/sync_manager.cc:811: // The following call
checks that initialized_ is true.
On 2011/10/21 14:29:07, nzea wrote:
> 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.

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
File chrome/browser/sync/internal_api/syncapi_unittest.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/internal...
chrome/browser/sync/internal_api/syncapi_unittest.cc:1218:
GetAllRealModelTypes(), true)).Times(3);
On 2011/10/21 14:29:07, nzea wrote:
> Enforcing the number of times this gets hit (here and elsewhere) seems a bit
> brittle. It also seems odd that you'd get this hit three times, when the
> encrypted types presumably only changes once.

Should be fixed by the change to only emit the notification when it actually
changes.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/js/js_sy...
File chrome/browser/sync/js/js_sync_manager_observer_unittest.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/js/js_sy...
chrome/browser/sync/js/js_sync_manager_observer_unittest.cc:215:
HasDetailsAsDictionary(expected_details)));
On 2011/10/21 14:29:07, nzea wrote:
> indent

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:846: VLOG(2) << "Encrypted types
changed to "
On 2011/10/21 14:29:07, nzea wrote:
> Since these now happen relatively rarely, I'd prefer VLOG(1) for this and
> encryption complete.

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:861: for
(syncable::ModelTypeSet::const_iterator it = registered_types.begin();
On 2011/10/21 14:29:07, nzea wrote:
> 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).

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

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1343: // We may be called during the
setup process before we're
On 2011/10/21 14:29:07, nzea wrote:
> 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.

Called by some unittests; see comment. :/

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1353: // initialized.
On 2011/10/21 14:29:07, nzea wrote:
> Comment that this is because it's used by IsEncryptedDatatypeEnabled, which is
> itself used by IsPassphraseRequiredForDecryption. :-/

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1366: // initialized.
On 2011/10/21 14:29:07, nzea wrote:
> Add something like "In this case, we'll default to the sensitive types."

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
File chrome/browser/sync/profile_sync_service.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.h:676: // everything.  Initialized on
OnBackendInitialized and updated by
On 2011/10/21 14:29:07, nzea wrote:
> Perhaps include a comment that the Cryptographer::SensitiveTypes will always
be
> part of the set of encrypted types.

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.cc (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.cc:328:
EmitEncryptedTypesChangeNotification();
On 2011/10/21 14:29:07, nzea wrote:
> Perhaps only emit if the types actually changed? (add and compare against
> old_encrypted_types) This applies to here and in set_encrypt_everything and
> SetEncryptedTypes

Done.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
File chrome/browser/sync/util/cryptographer.h (right):

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:57: // Guaranteed to be called
immeduately after a call of
On 2011/10/21 14:29:07, nzea wrote:
> immediately

Rendered moot

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:177: void
SetEncryptedTypes(syncable::ModelTypeSet new_types);
On 2011/10/21 14:29:07, nzea wrote:
> This isn't really used any more (outside of tests), although it does get
called
> unnecessarily from SyncInternal::EncryptDataTypes (which only ever gets called
> with an empty set now as a way of forcing an update of the nigori and
triggering
> reencryption). Cleaning that up is probably outside the scope of this patch
> though, so perhaps just add a todo there and assign it to me.

Actually, I ended up needing to clean it up, so I did.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:183: // notifiation.
On 2011/10/21 14:29:07, nzea wrote:
> Is it necessary to always trigger the OnEncryptedTypesChanged as well (I
suppose
> this is why the unit tests are based on receiving it 3 times)? If they had
> changed, we would have triggered it organically. If they hadn't, why force the
> notification?
> 
> In the case of locally turning on encryption, we're guaranteed to have
triggered
> an OnEncryptedTypesChanged (assuming they did actually change) before sending
> the OnEncryptionComplete. If they hadn't changed, then the PSS must already
have
> the current encryption info (by way of RefreshEncryption). Either way, the PSS
> will have the set of encrypted types/encrypt everything that matches the one
> OnEncryptionComplete used, so the setup flow can proceed accordingly.
> 
> Similarly, when we change the passphrase, we trigger a reencryption, but the
> encrypted types aren't changing. I'm not sure there's any benefit to forcing
the
> notification in that case.

Yeah, fixed.  It should only trigger when it actually changes.

http://codereview.chromium.org/8356026/diff/6002/chrome/browser/sync/util/cry...
chrome/browser/sync/util/cryptographer.h:191: void
EmitEncryptedTypesChangeNotification();
On 2011/10/21 14:29:07, nzea wrote:
> EncryptedTypesChange... -> EncryptedTypesChanged...

Done.

[akalin, 2011-10-24 18:36:05]

Ping!

I think the mac_sync integration test failure is a flake; retrying to see.

[Nicolas Zea, 2011-10-24 19:18:16]

Yeah, any MigratePrefs failure is migrate prefs failure is flake.

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/glue/sy...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/glue/sy...
chrome/browser/sync/glue/sync_backend_host.cc:930: // Triggers
OnEncryptedTypesChanged() if necessary.
and OnEncryptionComplete

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/interna...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/interna...
chrome/browser/sync/internal_api/sync_manager.cc:227: // Encrypt any new data
types or other nodes that should be
Mention that it triggers OnPassphraseRequired if the cryptographer isn't ready,
otherwise it updates the nigori node and reencrypts everything.

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/util/cr...
File chrome/browser/sync/util/cryptographer_unittest.cc (left):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/util/cr...
chrome/browser/sync/util/cryptographer_unittest.cc:207: // Ensure encrypted
types are never unset.
Any reason this was removed?

[akalin, 2011-10-24 19:34:13]

All comments addressed, PTAL

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/glue/sy...
File chrome/browser/sync/glue/sync_backend_host.cc (right):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/glue/sy...
chrome/browser/sync/glue/sync_backend_host.cc:930: // Triggers
OnEncryptedTypesChanged() if necessary.
On 2011/10/24 19:18:17, nzea wrote:
> and OnEncryptionComplete

Done.

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/interna...
File chrome/browser/sync/internal_api/sync_manager.cc (right):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/interna...
chrome/browser/sync/internal_api/sync_manager.cc:227: // Encrypt any new data
types or other nodes that should be
On 2011/10/24 19:18:17, nzea wrote:
> Mention that it triggers OnPassphraseRequired if the cryptographer isn't
ready,
> otherwise it updates the nigori node and reencrypts everything.

Done.

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/util/cr...
File chrome/browser/sync/util/cryptographer_unittest.cc (left):

http://codereview.chromium.org/8356026/diff/15024/chrome/browser/sync/util/cr...
chrome/browser/sync/util/cryptographer_unittest.cc:207: // Ensure encrypted
types are never unset.
On 2011/10/24 19:18:17, nzea wrote:
> Any reason this was removed?

The old public SetEncryptedTypes was more like MergeEncryptedTypes.  The new
SetEncryptedTypes is either private, or used by test, so it didn't make sense to
me to make it merge.  So we don't need to test the merge behavior anymore.

[Nicolas Zea, 2011-10-24 19:36:52]

LGTM

[commit-bot: I haz the power, 2011-10-24 19:46:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/8356026/22002

[commit-bot: I haz the power, 2011-10-24 19:46:41]

Can't process patch for file chrome/browser/sync/util/cryptographer.h.
File's status is None, patchset upload is incomplete.