Disable VK overscroll for login/out-of-box.

Disabling virtual keyboard overscroll (at least for now) for login/out-of-box since scroll
into view seems to be not working. Implement user pod scroll into view depending
on the number of user pod rows available.

Login pods scroll into view is triggered only on window resize and when keyboard is shown.
This means that if keyboard is already shown (i.e. no resize is happening) and user selects
another user pod in different row no scrolling happens i.e. relying on user to scroll pods manually.

Also updating outer-container min-height based on primary display resolution.

Note: Lock screen has overscroll enabled since default scroll into view implementation is working fine there.

Main changes are in
login_display_host_impl.cc
display_manager.js
user_pod_row.js
keyboard_util.cc
The rest is just adding 2 new common methods into the interfaces.

To disable this feature: --disable-login-scroll-into-view

Screenshots: https://folio.googleplex.com/loginvk

BUG=363635
TEST=Manual: 1..6 rows of user pods, bring up VK. Both landscape and portrait mode, incl. screen rotation.

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

[Nikita (slow), 2014-05-20 18:05:06]

dzioev@ - chrome/*/login

kevers@ - ui/keyboard

[Nikita (slow), 2014-05-21 04:00:47]

> kevers@ - ui/keyboard

+bshe@ for ui/keyboard

[kevers, 2014-05-21 13:56:11]

https://codereview.chromium.org/292233002/diff/90001/ui/keyboard/keyboard_uti...
File ui/keyboard/keyboard_util.cc (right):

https://codereview.chromium.org/292233002/diff/90001/ui/keyboard/keyboard_uti...
ui/keyboard/keyboard_util.cc:50: bool
g_keyboard_overscroll_enabled_override_value = true;
Feels a bit messy using 2 booleans here. Can we replace with a tri-valued
enumeration (NONE, ENABLED, DISABLED)?

[Nikita (slow), 2014-05-21 17:21:03]

https://codereview.chromium.org/292233002/diff/90001/ui/keyboard/keyboard_uti...
File ui/keyboard/keyboard_util.cc (right):

https://codereview.chromium.org/292233002/diff/90001/ui/keyboard/keyboard_uti...
ui/keyboard/keyboard_util.cc:50: bool
g_keyboard_overscroll_enabled_override_value = true;
On 2014/05/21 13:56:11, kevers wrote:
> Feels a bit messy using 2 booleans here. Can we replace with a tri-valued
> enumeration (NONE, ENABLED, DISABLED)?

Done.

[dzhioev (left Google), 2014-05-21 17:30:31]

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/ui/login_display_host_impl.cc (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/ui/login_display_host_impl.cc:910: if
(webui_login_display_) {
Shouldn't we also set keyboard state once webui_login_display_ became available
(in CreateLoginDisplay method)?

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
File chrome/browser/resources/login/display_manager.js (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/display_manager.js:730: if (currentScreen &&
currentScreen.onWindowResize)
Added check is not needed, because we have default implementation of
|onWindowResize| in Screen prototype.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
File chrome/browser/resources/login/user_pod_row.js (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:56: var USER_POD_ROW_SCROLL_STOPS
= [[0.5], [0.25, 0.75], [0, 0.5, 1]];
We can have more than 3 rows of pods. What happens then?

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:1417: get focusedPod() {
Why did you add this getter? As I can see, you only use it in
scrollFocusedPodIntoView where |focusedPod_| property can be used.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:1789:
this.scrollFocusedPodIntoView();
Did you try to make the same in |focusPod|, where we change focused pod? It can
help with "if keyboard is already shown (i.e. no resize is happening) and user 
selects another user pod in different row no scrolling happens" problem.

[kevers, 2014-05-21 17:54:12]

ui/keyboard lgtm

[bshe, 2014-05-21 19:12:40]

On 2014/05/21 17:54:12, kevers wrote:
> ui/keyboard lgtm

lgtm

[Nikita (slow), 2014-05-22 15:11:49]

Please take a look, added code that updates scroll-container min-height based on
primary screen resolution.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
File chrome/browser/chromeos/login/ui/login_display_host_impl.cc (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
chrome/browser/chromeos/login/ui/login_display_host_impl.cc:910: if
(webui_login_display_) {
On 2014/05/21 17:30:32, dzhioev wrote:
> Shouldn't we also set keyboard state once webui_login_display_ became
available
> (in CreateLoginDisplay method)?

When CreateLoginDisplay() is called neither handler is created nor JS side is
initialized.

Added to CoreOobeHandler::Initialize().

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
File chrome/browser/resources/login/display_manager.js (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/display_manager.js:730: if (currentScreen &&
currentScreen.onWindowResize)
On 2014/05/21 17:30:32, dzhioev wrote:
> Added check is not needed, because we have default implementation of
> |onWindowResize| in Screen prototype.

Cool, removed.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
File chrome/browser/resources/login/user_pod_row.js (right):

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:56: var USER_POD_ROW_SCROLL_STOPS
= [[0.5], [0.25, 0.75], [0, 0.5, 1]];
On 2014/05/21 17:30:32, dzhioev wrote:
> We can have more than 3 rows of pods. What happens then?

Added dynamic calculation.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:1417: get focusedPod() {
On 2014/05/21 17:30:32, dzhioev wrote:
> Why did you add this getter? As I can see, you only use it in
> scrollFocusedPodIntoView where |focusedPod_| property can be used.

Right, not needed in latest impl, removed.

https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
chrome/browser/resources/login/user_pod_row.js:1789:
this.scrollFocusedPodIntoView();
On 2014/05/21 17:30:32, dzhioev wrote:
> Did you try to make the same in |focusPod|, where we change focused pod? It
can
> help with "if keyboard is already shown (i.e. no resize is happening) and user

> selects another user pod in different row no scrolling happens" problem.

Since virtual keyboard shows up after some delay first pod is focused and then
resize happens.

As for user pod scrolling when keyboard is already visible there's a bug which
resets focus for some reason, investigating.

[dzhioev (left Google), 2014-05-23 15:17:51]

On 2014/05/22 15:11:49, Nikita Kostylev wrote:
> Please take a look, added code that updates scroll-container min-height based
on
> primary screen resolution.
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
> File chrome/browser/chromeos/login/ui/login_display_host_impl.cc (right):
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/chromeos/...
> chrome/browser/chromeos/login/ui/login_display_host_impl.cc:910: if
> (webui_login_display_) {
> On 2014/05/21 17:30:32, dzhioev wrote:
> > Shouldn't we also set keyboard state once webui_login_display_ became
> available
> > (in CreateLoginDisplay method)?
> 
> When CreateLoginDisplay() is called neither handler is created nor JS side is
> initialized.
> 
> Added to CoreOobeHandler::Initialize().
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> File chrome/browser/resources/login/display_manager.js (right):
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> chrome/browser/resources/login/display_manager.js:730: if (currentScreen &&
> currentScreen.onWindowResize)
> On 2014/05/21 17:30:32, dzhioev wrote:
> > Added check is not needed, because we have default implementation of
> > |onWindowResize| in Screen prototype.
> 
> Cool, removed.
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> File chrome/browser/resources/login/user_pod_row.js (right):
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> chrome/browser/resources/login/user_pod_row.js:56: var
USER_POD_ROW_SCROLL_STOPS
> = [[0.5], [0.25, 0.75], [0, 0.5, 1]];
> On 2014/05/21 17:30:32, dzhioev wrote:
> > We can have more than 3 rows of pods. What happens then?
> 
> Added dynamic calculation.
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> chrome/browser/resources/login/user_pod_row.js:1417: get focusedPod() {
> On 2014/05/21 17:30:32, dzhioev wrote:
> > Why did you add this getter? As I can see, you only use it in
> > scrollFocusedPodIntoView where |focusedPod_| property can be used.
> 
> Right, not needed in latest impl, removed.
> 
>
https://codereview.chromium.org/292233002/diff/90001/chrome/browser/resources...
> chrome/browser/resources/login/user_pod_row.js:1789:
> this.scrollFocusedPodIntoView();
> On 2014/05/21 17:30:32, dzhioev wrote:
> > Did you try to make the same in |focusPod|, where we change focused pod? It
> can
> > help with "if keyboard is already shown (i.e. no resize is happening) and
user
> 
> > selects another user pod in different row no scrolling happens" problem.
> 
> Since virtual keyboard shows up after some delay first pod is focused and then
> resize happens.
> 
> As for user pod scrolling when keyboard is already visible there's a bug which
> resets focus for some reason, investigating.

LGTM

[Nikita (slow), 2014-05-23 16:04:38]

The CQ bit was checked by nkostylev@chromium.org

[commit-bot: I haz the power, 2014-05-23 16:05:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nkostylev@chromium.org/292233002/230001

[commit-bot: I haz the power, 2014-05-23 20:07:12]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)

[commit-bot: I haz the power, 2014-05-23 22:03:50]

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

[commit-bot: I haz the power, 2014-05-23 22:03:51]

Try jobs failed on following builders:
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)

[Nikita (slow), 2014-05-25 22:23:54]

The CQ bit was checked by nkostylev@chromium.org

[commit-bot: I haz the power, 2014-05-25 22:24:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nkostylev@chromium.org/292233002/250001

[commit-bot: I haz the power, 2014-05-26 00:15:31]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-26 00:40:00]

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

[commit-bot: I haz the power, 2014-05-26 00:40:01]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[Nikita (slow), 2014-05-26 04:15:41]

The CQ bit was checked by nkostylev@chromium.org

[commit-bot: I haz the power, 2014-05-26 04:16:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nkostylev@chromium.org/292233002/250001

[commit-bot: I haz the power, 2014-05-26 16:17:32]

Change committed as 272843