Thread-safe version of PassphraseType.

Thread-safe version of PassphraseType.

BUG=638963
Committed: https://crrev.com/a7c2a6c4ad0e6087a81f7624d47a875290b0dbb2
Cr-Commit-Position: refs/heads/master@{#415653}

[melandory, 2016-08-26 16:01:24]

melandory@chromium.org changed reviewers:
+ zea@chromium.org

[melandory, 2016-08-26 16:01:24]

PTAL.

This CL is part of CLs chain described in last mail here:
https://codereview.chromium.org/2260953002

[Nicolas Zea, 2016-08-26 18:25:46]

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl.cc (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:722: PassphraseType*
passphrase_type = &UnlockVaultMutable(trans)->passphrase_type;
given ShouldTriggerMigration doesn't modify the value, this can jsut be
UnlockVault

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:994: bool
passphrase_typematches = true;
nit: this looks like a search and replace bug, keep the old name.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:1323: PassphraseType*
passphrase_type) const {
this method never modifies the passphrase right? Prefer passing by value then.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl.h (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.h:80: PassphraseType
GetPassphraseType(
nit: remove newline above (so it's grouped together with the other NigoriHandler
methods)

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl_unittest.cc (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl_unittest.cc:349: void
VerifyPassphraseType(PassphraseType passphrase_type) {
I'm kind of surprised this compiles given it's defined below where it's used?
Perhaps that's change with C++11?

[melandory, 2016-08-26 21:36:12]

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl.cc (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:722: PassphraseType*
passphrase_type = &UnlockVaultMutable(trans)->passphrase_type;
On 2016/08/26 18:25:45, Nicolas Zea wrote:
> given ShouldTriggerMigration doesn't modify the value, this can jsut be
> UnlockVault

Done.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:994: bool
passphrase_typematches = true;
On 2016/08/26 18:25:45, Nicolas Zea wrote:
> nit: this looks like a search and replace bug, keep the old name.

Done.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.cc:1323: PassphraseType*
passphrase_type) const {
On 2016/08/26 18:25:46, Nicolas Zea wrote:
> this method never modifies the passphrase right? Prefer passing by value then.

Done.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl.h (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl.h:80: PassphraseType
GetPassphraseType(
On 2016/08/26 18:25:46, Nicolas Zea wrote:
> nit: remove newline above (so it's grouped together with the other
NigoriHandler
> methods)

Done.

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
File components/sync/core_impl/sync_encryption_handler_impl_unittest.cc (right):

https://codereview.chromium.org/2278043003/diff/1/components/sync/core_impl/s...
components/sync/core_impl/sync_encryption_handler_impl_unittest.cc:349: void
VerifyPassphraseType(PassphraseType passphrase_type) {
On 2016/08/26 18:25:46, Nicolas Zea wrote:
> I'm kind of surprised this compiles given it's defined below where it's used?
> Perhaps that's change with C++11?

It is within class bounds, so order doesn't matter, but moved anyway.

[Nicolas Zea, 2016-08-26 21:40:42]

LGTM

https://codereview.chromium.org/2278043003/diff/20001/components/sync/core_im...
File components/sync/core_impl/sync_encryption_handler_impl.h (right):

https://codereview.chromium.org/2278043003/diff/20001/components/sync/core_im...
components/sync/core_impl/sync_encryption_handler_impl.h:236: const
PassphraseType& passphrase_type) const;
nit: given it's just an enum class, no need for const ref I think?

[melandory, 2016-08-31 00:49:11]

The CQ bit was checked by melandory@chromium.org

[melandory, 2016-08-31 00:49:12]

The patchset sent to the CQ was uploaded after l-g-t-m from zea@chromium.org
Link to the patchset: https://codereview.chromium.org/2278043003/#ps40001
(title: "Thread-safe version of PassphraseType.")

[commit-bot: I haz the power, 2016-08-31 00:49:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 02:00:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-31 02:00:09]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[melandory, 2016-08-31 15:40:48]

The CQ bit was checked by melandory@chromium.org

[commit-bot: I haz the power, 2016-08-31 15:41:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-31 16:29:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-31 16:30:44]

Description was changed from

==========
Thread-safe version of PassphraseType.

BUG=638963
==========

to

==========
Thread-safe version of PassphraseType.

BUG=638963

Committed: https://crrev.com/a7c2a6c4ad0e6087a81f7624d47a875290b0dbb2
Cr-Commit-Position: refs/heads/master@{#415653}
==========

[commit-bot: I haz the power, 2016-08-31 16:30:45]

Patchset 3 (id:??) landed as
https://crrev.com/a7c2a6c4ad0e6087a81f7624d47a875290b0dbb2
Cr-Commit-Position: refs/heads/master@{#415653}