Password in user pods clears after minute of idle state.

Password in user pods clears after minute of idle state.


BUG=155440
TBR=nkostylev

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

[ygorshenin1, 2013-02-26 16:43:01]

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/login/webui_login_display.cc (right):

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:27: const int
kPasswordClearTimeoutS = 60;
nit: s/kPasswordClearTimeoutS/kPasswordClearTimeoutSec

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:339: LOG(ERROR) << "User
activity";
Why do we need this log message?

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:340: if
(!password_clear_timer_.IsRunning()) {
nit: you can avoid curly braces there.

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:354: LOG(ERROR) <<
"Password timer expired";
Suggest to change log level from ERROR to WARNING.

[dzhioev (left Google), 2013-02-26 17:02:40]

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/login/webui_login_display.cc (right):

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:27: const int
kPasswordClearTimeoutS = 60;
On 2013/02/26 16:43:01, ygorshenin1 wrote:
> nit: s/kPasswordClearTimeoutS/kPasswordClearTimeoutSec

Done.

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:339: LOG(ERROR) << "User
activity";
On 2013/02/26 16:43:01, ygorshenin1 wrote:
> Why do we need this log message?
Just forgot to remove.
Done.

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:340: if
(!password_clear_timer_.IsRunning()) {
On 2013/02/26 16:43:01, ygorshenin1 wrote:
> nit: you can avoid curly braces there.

Done.

https://chromiumcodereview.appspot.com/12321136/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/login/webui_login_display.cc:354: LOG(ERROR) <<
"Password timer expired";
On 2013/02/26 16:43:01, ygorshenin1 wrote:
> Suggest to change log level from ERROR to WARNING.
I've forgot to remove it too.

[ygorshenin1, 2013-02-26 17:07:48]

lgtm

[commit-bot: I haz the power, 2013-02-26 17:16:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dzhioev@chromium.org/12321136/3002

[commit-bot: I haz the power, 2013-02-26 17:17:09]

Presubmit check for 12321136-3002 failed and returned exit status 1.

INFO:root:Found 8 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/resources/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/webui/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h
    chrome/browser/resources/chromeos/login/oobe.js
    chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
    chrome/browser/resources/chromeos/login/user_pod_row.js
    chrome/browser/chromeos/login/webui_login_display.cc
    chrome/browser/resources/chromeos/login/login.js
    chrome/browser/chromeos/login/webui_login_display.h
    chrome/browser/resources/chromeos/login/display_manager.js

Presubmit checks took 4.9s to calculate.

[dzhioev (left Google), 2013-02-26 17:20:46]

Dmitry, I need owner's LGTM.

[Dmitry Polukhin, 2013-02-27 14:47:42]

The code looks good but I have general question why you did not make everything
in JavaScript only? It looks like we can ignore user activity outside login
screen page...

[dzhioev (left Google), 2013-02-27 15:08:06]

On 2013/02/27 14:47:42, Dmitry Polukhin wrote:
> The code looks good but I have general question why you did not make
everything
> in JavaScript only? It looks like we can ignore user activity outside login
> screen page...

I've thought about JavaScript only implementation, but decided to do it in this
way:
* I'm not sure that JavaScript receives all user actions (functional keys, etc).
* I think JavaScript implementation is more complicated -- for example jQuery
plugin for detecting idle state (
https://github.com/mikesherov/jquery-idletimer/blob/master/src/idle-timer.js )
has 160 lines of code.

[Dmitry Polukhin, 2013-02-27 15:12:07]

OK, LGTM

[commit-bot: I haz the power, 2013-02-27 15:13:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dzhioev@chromium.org/12321136/3002

[commit-bot: I haz the power, 2013-02-27 15:13:45]

Presubmit check for 12321136-3002 failed and returned exit status 1.

INFO:root:Found 8 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/resources/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/browser/ui/webui/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc
    chrome/browser/ui/webui/chromeos/login/signin_screen_handler.h

Presubmit checks took 5.0s to calculate.

[commit-bot: I haz the power, 2013-02-27 15:19:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dzhioev@chromium.org/12321136/3002

[commit-bot: I haz the power, 2013-02-27 18:03:38]

Change committed as 184987