[Sync] Fix encryption/passphrase handling.

[Sync] Fix encryption/passphrase handling.

We now ensure cached passphrases are always used. In addition, encryption now
happens after configuration, allowing us to make use of the
pending_encrypted_types to know if we're waiting for encryption (which
affects how we prompt for passphrase). This is dependent on sync_setup_flow.cc
always calling EncryptDataTypes before configuring the new datatypes and setting
pending_encrypted_types_ appropriately, which then gets consumed on
SYNC_CONFIGURE_DONE.

BUG=91314
TEST=see bug

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

[Nicolas Zea, 2011-08-02 21:18:26]

Please take a look. I added a couple codereview comments into the first patchset
to make it clear why some of these changes work.

http://codereview.chromium.org/7551024/diff/1/chrome/browser/sync/profile_syn...
File chrome/browser/sync/profile_sync_service.cc (left):

http://codereview.chromium.org/7551024/diff/1/chrome/browser/sync/profile_syn...
chrome/browser/sync/profile_sync_service.cc:1244: // If the user had entered a
custom passphrase use it now.
This all now happens in OnPassphraseRequired and OnPassphraseAccepted.

http://codereview.chromium.org/7551024/diff/1/chrome/browser/sync/profile_syn...
File chrome/browser/sync/profile_sync_service.cc (right):

http://codereview.chromium.org/7551024/diff/1/chrome/browser/sync/profile_syn...
chrome/browser/sync/profile_sync_service.cc:760:
wizard_.Step(SyncSetupWizard::DONE);
This works because encryption only occurs in the SYNC_CONFIGURE_DONE handling
now.

http://codereview.chromium.org/7551024/diff/1/chrome/browser/sync/profile_syn...
chrome/browser/sync/profile_sync_service.cc:1204:
pending_types_for_encryption_.clear();
This call will be combined with a configuredatatypes, which will trigger the UI
done.

[tim (not reviewing), 2011-08-04 16:57:56]

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

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:699: std::string pass =
gaia_password_;
Comment why we need to do this move-aside?

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1199: void
ProfileSyncService::EncryptDataTypes(
W.r.t your comment below, lets call this set_pending_types_for_encryption() so
it's clear to callers it doesn't do anything fancy aside from set the value.

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1276: // TODO(sync): Less wizard,
more toast.
nit - just nuke this todo, it's obsolete.

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1282: // matches the encrypted
types.
if they match, does it still clear?

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
File chrome/browser/sync/sync_setup_flow.cc (right):

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
chrome/browser/sync/sync_setup_flow.cc:273: // example due to needing a
passphrase).
hmm.. this seems odd.. but if we must can we pull the ModelTypeSet as well as
the EncryptDataTypes call outside of the if (above and below, respectively)

[Nicolas Zea, 2011-08-04 18:43:54]

PTAL

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

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:699: std::string pass =
gaia_password_;
On 2011/08/04 16:57:56, timsteele wrote:
> Comment why we need to do this move-aside?

Done.

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1199: void
ProfileSyncService::EncryptDataTypes(
On 2011/08/04 16:57:56, timsteele wrote:
> W.r.t your comment below, lets call this set_pending_types_for_encryption() so
> it's clear to callers it doesn't do anything fancy aside from set the value.

Done.

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1276: // TODO(sync): Less wizard,
more toast.
On 2011/08/04 16:57:56, timsteele wrote:
> nit - just nuke this todo, it's obsolete.

Done.

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
chrome/browser/sync/profile_sync_service.cc:1282: // matches the encrypted
types.
Yes, added comment.

On 2011/08/04 16:57:56, timsteele wrote:
> if they match, does it still clear?

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
File chrome/browser/sync/sync_setup_flow.cc (right):

http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
chrome/browser/sync/sync_setup_flow.cc:273: // example due to needing a
passphrase).
On 2011/08/04 16:57:56, timsteele wrote:
> hmm.. this seems odd.. but if we must can we pull the ModelTypeSet as well as
> the EncryptDataTypes call outside of the if (above and below, respectively)

Done.

[tim (not reviewing), 2011-08-05 19:03:40]

On 2011/08/04 18:43:54, nzea wrote:
> PTAL
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
> File chrome/browser/sync/profile_sync_service.cc (right):
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
> chrome/browser/sync/profile_sync_service.cc:699: std::string pass =
> gaia_password_;
> On 2011/08/04 16:57:56, timsteele wrote:
> > Comment why we need to do this move-aside?
> 
> Done.
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
> chrome/browser/sync/profile_sync_service.cc:1199: void
> ProfileSyncService::EncryptDataTypes(
> On 2011/08/04 16:57:56, timsteele wrote:
> > W.r.t your comment below, lets call this set_pending_types_for_encryption()
so
> > it's clear to callers it doesn't do anything fancy aside from set the value.
> 
> Done.
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
> chrome/browser/sync/profile_sync_service.cc:1276: // TODO(sync): Less wizard,
> more toast.
> On 2011/08/04 16:57:56, timsteele wrote:
> > nit - just nuke this todo, it's obsolete.
> 
> Done.
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/profile_...
> chrome/browser/sync/profile_sync_service.cc:1282: // matches the encrypted
> types.
> Yes, added comment.
> 
> On 2011/08/04 16:57:56, timsteele wrote:
> > if they match, does it still clear?
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
> File chrome/browser/sync/sync_setup_flow.cc (right):
> 
>
http://codereview.chromium.org/7551024/diff/5001/chrome/browser/sync/sync_set...
> chrome/browser/sync/sync_setup_flow.cc:273: // example due to needing a
> passphrase).
> On 2011/08/04 16:57:56, timsteele wrote:
> > hmm.. this seems odd.. but if we must can we pull the ModelTypeSet as well
as
> > the EncryptDataTypes call outside of the if (above and below, respectively)
> 
> Done.

LGTM

[commit-bot: I haz the power, 2011-08-05 20:55:00]

Can't apply patch for file chrome/browser/sync/engine/syncapi.cc.
While running patch -p1 --forward --force;
patching file chrome/browser/sync/engine/syncapi.cc
Hunk #1 succeeded at 2096 (offset 10 lines).
Hunk #2 FAILED at 2108.
Hunk #3 succeeded at 2165 (offset 11 lines).
Hunk #4 succeeded at 2184 (offset 11 lines).
1 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/sync/engine/syncapi.cc.rej