[Sync] Refactor passphrase state handling

[Sync] Refactor passphrase state handling

We now have an enum describing passphrase state, and cache the current value
on the UI thread via a new OnPassphraseStateChanged method.

Additionally, we log the OnPassphraseStateChanged event in the debug log info.

BUG=139848, 129665
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153397

[Nicolas Zea, 2012-08-21 22:46:26]

PTAL

[akalin, 2012-08-22 20:10:39]

lgtm

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
File sync/internal_api/public/sync_encryption_handler.h (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:44: // This class is not
thread safe. All methods must be invoked on the sync
thread safe -> thread-safe

also, since this is an interface, reword as:

Implementations of this class must be assumed to be non-thread-safe.  All
methods must be ...

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:117: // Add/Remove
SyncEncryptionHandler::Observer's.
no apostrophe

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:155: // bag encryption keys
in the nigori node.
bag -> bag of (?)

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_enc...
File sync/internal_api/sync_encryption_handler_impl.cc (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_enc...
sync/internal_api/sync_encryption_handler_impl.cc:562: (passphrase_state_ ==
CUSTOM_PASSPHRASE) ||
might be clearer to have an intermediate variable 'explicit_passphrase =
passphrase_state_ == CUSTOM_PASSPHRASE'

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_man...
File sync/internal_api/sync_manager_impl_unittest.cc (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_man...
sync/internal_api/sync_manager_impl_unittest.cc:2336:
EXPECT_EQ(CUSTOM_PASSPHRASE,
can you write a test that uses any of the other two states?  or will those come
in a future CL?

[Nicolas Zea, 2012-08-22 20:18:33]

All comments addressed. Committing after
http://codereview.chromium.org/10844005/ lands.

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
File sync/internal_api/public/sync_encryption_handler.h (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:44: // This class is not
thread safe. All methods must be invoked on the sync
On 2012/08/22 20:10:39, akalin wrote:
> thread safe -> thread-safe
> 
> also, since this is an interface, reword as:
> 
> Implementations of this class must be assumed to be non-thread-safe.  All
> methods must be ...

Done.

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:117: // Add/Remove
SyncEncryptionHandler::Observer's.
On 2012/08/22 20:10:39, akalin wrote:
> no apostrophe

Done.

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/public/s...
sync/internal_api/public/sync_encryption_handler.h:155: // bag encryption keys
in the nigori node.
On 2012/08/22 20:10:39, akalin wrote:
> bag -> bag of (?)

Done.

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_enc...
File sync/internal_api/sync_encryption_handler_impl.cc (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_enc...
sync/internal_api/sync_encryption_handler_impl.cc:562: (passphrase_state_ ==
CUSTOM_PASSPHRASE) ||
On 2012/08/22 20:10:39, akalin wrote:
> might be clearer to have an intermediate variable 'explicit_passphrase =
> passphrase_state_ == CUSTOM_PASSPHRASE'

Done.

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_man...
File sync/internal_api/sync_manager_impl_unittest.cc (right):

http://codereview.chromium.org/10824410/diff/18001/sync/internal_api/sync_man...
sync/internal_api/sync_manager_impl_unittest.cc:2336:
EXPECT_EQ(CUSTOM_PASSPHRASE,
On 2012/08/22 20:10:39, akalin wrote:
> can you write a test that uses any of the other two states?  or will those
come
> in a future CL?

Those will be in future CL's once the migration logic is introduced.

[Nicolas Zea, 2012-08-23 22:49:55]

ping?

[commit-bot: I haz the power, 2012-08-24 22:55:26]

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

[commit-bot: I haz the power, 2012-08-26 11:54:28]

Change committed as 153397