[Easy Unlock] Update handling of the trial easy unlock/signin run

[Easy Unlock] Update handling of the trial easy unlock/signin run

Consider Easy Unlock behaviour the trial one only if initiated
from Easy Unlock
app during setup.

Autoshows all tooltips on the first run.

BUG=410143, 422084
Committed: https://crrev.com/ab8d5b5509a011a59c7c0791bef0f1444448dc67
Cr-Commit-Position: refs/heads/master@{#299010}

[tbarzic, 2014-10-08 18:07:07]

tbarzic@chromium.org changed reviewers:
+ xiyuan@chromium.org

[xiyuan, 2014-10-08 19:57:25]

lgtm

https://codereview.chromium.org/628193003/diff/1/chrome/browser/signin/easy_u...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/628193003/diff/1/chrome/browser/signin/easy_u...
chrome/browser/signin/easy_unlock_service.cc:542:
profile_->GetPrefs()->SetBoolean(prefs::kEasyUnlockShowTutorial, true);
nit: How about ClearPref() here?

[tbarzic, 2014-10-09 02:43:24]

Can you take another look?

I noticed that pairing data changes were not handled particularly well after
easy unlock has been set up(i.e. if user turns easy unlock off and sets it up
again; or when phone public keys change). The lock screen would end up in
PAIRING_CHANGED/NO_PAIRING hardlock state when the screen is locked after setup
completes.

Let me know what you think about the approach in the newest patchset:
propagating NO_PAIRING and PAIRING_CHANGED states to signin screen only?

https://codereview.chromium.org/628193003/diff/1/chrome/browser/signin/easy_u...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/628193003/diff/1/chrome/browser/signin/easy_u...
chrome/browser/signin/easy_unlock_service.cc:542:
profile_->GetPrefs()->SetBoolean(prefs::kEasyUnlockShowTutorial, true);
On 2014/10/08 19:57:25, xiyuan wrote:
> nit: How about ClearPref() here?

Done.

[xiyuan, 2014-10-09 03:18:04]

On 2014/10/09 02:43:24, tbarzic wrote:
> Can you take another look?
> 
> I noticed that pairing data changes were not handled particularly well after
> easy unlock has been set up(i.e. if user turns easy unlock off and sets it up
> again; or when phone public keys change). The lock screen would end up in
> PAIRING_CHANGED/NO_PAIRING hardlock state when the screen is locked after
setup
> completes.
> 

Trial run complete state is sticky. That is, only the very first time is counted
as trial run. After that, even if we turn off Easy unlock and go through setup
again, it is not actually consider trial run (i.e. IsTrialRun returns false).
Maybe the fix should be clear the trial run state during setup flow?

> Let me know what you think about the approach in the newest patchset:
> propagating NO_PAIRING and PAIRING_CHANGED states to signin screen only?
> 

I prefer to keep lock screen consistent with signin screen.

[tbarzic, 2014-10-09 20:35:01]

updated logic as discussed before

[xiyuan, 2014-10-09 21:03:55]

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
File chrome/browser/signin/easy_unlock_screenlock_state_handler.cc (right):

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
chrome/browser/signin/easy_unlock_screenlock_state_handler.cc:279:
ScreenlockBridge::LockHandler::USER_CLICK) {
nit: Should we move this logic to Lockhandler::SetAuthType?

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
File chrome/browser/signin/easy_unlock_service.cc (right):

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
chrome/browser/signin/easy_unlock_service.cc:546: GetHardlockState() ==
EasyUnlockScreenlockStateHandler::NO_PAIRING) {
Why do we need to map NO_PAIRING to PAIRING_CHANGED?

[xiyuan, 2014-10-09 21:08:58]

Discussed offline. So LGTM

[tbarzic, 2014-10-09 21:33:27]

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
File chrome/browser/signin/easy_unlock_screenlock_state_handler.cc (right):

https://codereview.chromium.org/628193003/diff/120001/chrome/browser/signin/e...
chrome/browser/signin/easy_unlock_screenlock_state_handler.cc:279:
ScreenlockBridge::LockHandler::USER_CLICK) {
On 2014/10/09 21:03:55, xiyuan wrote:
> nit: Should we move this logic to Lockhandler::SetAuthType?

I plan to make a cl that removes FORCED_OFFLINE_PASSWORD auth type, I'll include
this in that cl.

Though, the main reason I'm also inclined to keep this here for now is because
we have a test case covering it :)

[tbarzic, 2014-10-09 21:39:17]

tbarzic@chromium.org changed reviewers:
+ yoz@chromium.org

[tbarzic, 2014-10-09 21:39:18]

+yoz for chrome/browser/extensions/api/screenlock_private_api.cc

Note that I plan to move this logic to easyUnlockPrivate API (and make the
function name more descriptive), but want to do that separately mainly because
we plan to merge this back to M39 (and I'd like to avoid having to merge an app
change in addition to this)

[Yoyo Zhou, 2014-10-09 21:53:21]

screenlock_private_api LGTM

[tbarzic, 2014-10-09 22:24:17]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-10-09 22:25:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/628193003/120001

[commit-bot: I haz the power, 2014-10-09 23:53:11]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2014-10-09 23:54:13]

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