Easy Unlock: Add support for hard locking

Easy Unlock: Add support for hard locking

When user clicks on the easy-unlock icon on "Click to enter" lock screen
(i.e. when lock screen auth type is USER_CLICK), the lock screen changes to
state in which password is required to unlock the screen.
This adds new auth state FORCE_OFFLINE_PASSWORD which, unlike OFFLINE_PASSWORD,
can't be changed to any other.

BUG=397363
TBR=asargent@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290096

[tbarzic, 2014-08-13 23:06:35]

depends on: https://codereview.chromium.org/446743003/

rogerta: owner for chrome/browser/signin/screenlock_bridge
dzhioev: owner for ui/login/
asargent: owner for chrome/browser/extensions/api/screenock_private (fairly
trivial)

[dzhioev (left Google), 2014-08-14 10:46:29]

LGTM with a couple of remarks

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:291: // When the icon is focused using
mouse, there should be no outline shown.
All you need is:
this.iconElement.addEventListener('mousedown', function(e) {
  e.preventDefault();
});

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
What if we want to replace callback?

[xiyuan, 2014-08-14 17:41:07]

lgtm

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 10:46:28, dzhioev wrote:
> What if we want to replace callback?

Looks like we should just do this.hardlockUserPod_.bind() here instead of using
|opt_callback| since we don't have other action handler.

[tbarzic, 2014-08-14 18:15:51]

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:291: // When the icon is focused using
mouse, there should be no outline shown.
On 2014/08/14 10:46:28, dzhioev wrote:
> All you need is:
> this.iconElement.addEventListener('mousedown', function(e) {
>   e.preventDefault();
> });

yes, that's much cleaner, thanks

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 10:46:28, dzhioev wrote:
> What if we want to replace callback?

Done.

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 17:41:07, xiyuan wrote:
> On 2014/08/14 10:46:28, dzhioev wrote:
> > What if we want to replace callback?
> 
> Looks like we should just do this.hardlockUserPod_.bind() here instead of
using
> |opt_callback| since we don't have other action handler.

hardlockUserPod_ is defined on UserPod, not UserPodCustomIcon (i.e. |this|).

I could move the logic here, but I'd rather keep the icon element agnostic of
the user associated with the user pod that contains the icon.

Though, if you feel strongly about this, I'll reshuffle the code a bit :)

[dzhioev (left Google), 2014-08-14 19:38:21]

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 18:15:51, tbarzic wrote:
> On 2014/08/14 17:41:07, xiyuan wrote:
> > On 2014/08/14 10:46:28, dzhioev wrote:
> > > What if we want to replace callback?
> > 
> > Looks like we should just do this.hardlockUserPod_.bind() here instead of
> using
> > |opt_callback| since we don't have other action handler.
> 
> hardlockUserPod_ is defined on UserPod, not UserPodCustomIcon (i.e. |this|).
> 
> I could move the logic here, but I'd rather keep the icon element agnostic of
> the user associated with the user pod that contains the icon.
> 
> Though, if you feel strongly about this, I'll reshuffle the code a bit :)
No I just meant, that if you call 'setInteractive(true, callback)', you expect
that your callback will be called on click. However, if some callback already
set in icon, your callback will be just silently ignored. I think we should
replace existing callback with a new one in this case.

[xiyuan, 2014-08-14 19:40:01]

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 18:15:51, tbarzic wrote:
> On 2014/08/14 17:41:07, xiyuan wrote:
> > On 2014/08/14 10:46:28, dzhioev wrote:
> > > What if we want to replace callback?
> > 
> > Looks like we should just do this.hardlockUserPod_.bind() here instead of
> using
> > |opt_callback| since we don't have other action handler.
> 
> hardlockUserPod_ is defined on UserPod, not UserPodCustomIcon (i.e. |this|).
> 
> I could move the logic here, but I'd rather keep the icon element agnostic of
> the user associated with the user pod that contains the icon.
> 
> Though, if you feel strongly about this, I'll reshuffle the code a bit :)

I see. In this case, passing the callback is right.

My other comment is that |value| and |opt_callback| seem a bit overlapping. Is
there a case that we need to set a callback when |value| is false (i.e.
non-interactive)? If not, could we just use !!opt_callback as an indication of
whether be interactive?

[tbarzic, 2014-08-14 20:35:07]

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 19:38:21, dzhioev wrote:
> On 2014/08/14 18:15:51, tbarzic wrote:
> > On 2014/08/14 17:41:07, xiyuan wrote:
> > > On 2014/08/14 10:46:28, dzhioev wrote:
> > > > What if we want to replace callback?
> > > 
> > > Looks like we should just do this.hardlockUserPod_.bind() here instead of
> > using
> > > |opt_callback| since we don't have other action handler.
> > 
> > hardlockUserPod_ is defined on UserPod, not UserPodCustomIcon (i.e. |this|).
> > 
> > I could move the logic here, but I'd rather keep the icon element agnostic
of
> > the user associated with the user pod that contains the icon.
> > 
> > Though, if you feel strongly about this, I'll reshuffle the code a bit :)
> No I just meant, that if you call 'setInteractive(true, callback)', you expect
> that your callback will be called on click. However, if some callback already
> set in icon, your callback will be just silently ignored. I think we should
> replace existing callback with a new one in this case.

yeah, I got there (and fixed it). I was replying to xiyuan there :)

[tbarzic, 2014-08-14 21:03:07]

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
File ui/login/account_picker/user_pod_row.js (right):

https://codereview.chromium.org/456943003/diff/260001/ui/login/account_picker...
ui/login/account_picker/user_pod_row.js:464: return;
On 2014/08/14 19:40:01, xiyuan wrote:
> On 2014/08/14 18:15:51, tbarzic wrote:
> > On 2014/08/14 17:41:07, xiyuan wrote:
> > > On 2014/08/14 10:46:28, dzhioev wrote:
> > > > What if we want to replace callback?
> > > 
> > > Looks like we should just do this.hardlockUserPod_.bind() here instead of
> > using
> > > |opt_callback| since we don't have other action handler.
> > 
> > hardlockUserPod_ is defined on UserPod, not UserPodCustomIcon (i.e. |this|).
> > 
> > I could move the logic here, but I'd rather keep the icon element agnostic
of
> > the user associated with the user pod that contains the icon.
> > 
> > Though, if you feel strongly about this, I'll reshuffle the code a bit :)
> 
> I see. In this case, passing the callback is right.
> 
> My other comment is that |value| and |opt_callback| seem a bit overlapping. Is
> there a case that we need to set a callback when |value| is false (i.e.
> non-interactive)? If not, could we just use !!opt_callback as an indication of
> whether be interactive?
> 

Done.

[xiyuan, 2014-08-14 21:04:33]

LGTM++

[tbarzic, 2014-08-15 18:18:47]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-08-15 18:20:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tbarzic@chromium.org/456943003/340001

[commit-bot: I haz the power, 2014-08-15 22:51:56]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/42501)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/47698)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[tbarzic, 2014-08-15 22:52:36]

The CQ bit was unchecked by tbarzic@chromium.org

[tbarzic, 2014-08-15 23:21:54]

The CQ bit was checked by tbarzic@chromium.org

[commit-bot: I haz the power, 2014-08-15 23:28:28]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tbarzic@chromium.org/456943003/360001

[commit-bot: I haz the power, 2014-08-16 05:50:29]

Committed patchset #19 (360001) as 290096