Virtual Keyboard: Update insets on change to window bounds.

Update insets on change to window bounds.

In overscroll mode, any window with web contents that overlaps the virtual keyboard has insets in order to facilitate scrolling to the bottom of the document.  These insets are updated whenever the keyboard is shown or hidden.  This patch adds another update in the event that a window moves while the keyboard is visible.  This case can occur after a screen rotation while the keyboard is shown.


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

[kevers, 2014-07-03 20:55:31]

Hi Biao,

Can you please take a look at this CL.

[bshe, 2014-07-04 13:03:57]

lgtm

I am not familiar with RWHV related code. It looks right to me but you may want
another person to take a look if this is not too urgent.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:409: proxy_->HasKeyboardWindow() &&
proxy_->GetKeyboardWindow()->IsVisible();
nit: Do you need HasKeyboardWindow? It should always be true after you call
GetKeyboardWindow above.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:423: view->SetInsets(gfx::Insets(0, 0, 0,
0));
nit: probably gfx::Insets()?

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:529: while(window->parent() &&
window->parent()->id() < 0) {
nit: do you mind to add a comment on why do we check "id < 0"? Or maybe add an
anonymous function since this while loop doesn't depend on KeyboardController
and is used in more than one place.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:540: if (window &&
window->HasObserver(window_bounds_observer_.get()))
nit: you probably dont need to check if window is null in this if check or you
should probably check it before the while loop.

[kevers, 2014-07-17 20:04:15]

The CQ bit was checked by kevers@chromium.org

[commit-bot: I haz the power, 2014-07-17 20:06:17]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kevers@chromium.org/368323002/60001

[commit-bot: I haz the power, 2014-07-17 21:12:45]

Change committed as 283873

[kevers, 2014-07-18 14:06:23]

Whoops.  Neglected to submit my earlier comments.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:409: proxy_->HasKeyboardWindow() &&
proxy_->GetKeyboardWindow()->IsVisible();
On 2014/07/04 13:03:57, bshe wrote:
> nit: Do you need HasKeyboardWindow? It should always be true after you call
> GetKeyboardWindow above.

Done.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:423: view->SetInsets(gfx::Insets(0, 0, 0,
0));
On 2014/07/04 13:03:57, bshe wrote:
> nit: probably gfx::Insets()?

Done.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:529: while(window->parent() &&
window->parent()->id() < 0) {
On 2014/07/04 13:03:57, bshe wrote:
> nit: do you mind to add a comment on why do we check "id < 0"? Or maybe add an
> anonymous function since this while loop doesn't depend on KeyboardController
> and is used in more than one place.

Done.

https://codereview.chromium.org/368323002/diff/20001/ui/keyboard/keyboard_con...
ui/keyboard/keyboard_controller.cc:540: if (window &&
window->HasObserver(window_bounds_observer_.get()))
On 2014/07/04 13:03:57, bshe wrote:
> nit: you probably dont need to check if window is null in this if check or you
> should probably check it before the while loop.

Done.