Remove redundant SetBounds calls to the keyboard window.

Remove redundant SetBounds calls to the keyboard window.

These additional calls were preempting the initial load animation from
finishing. Keyboard show animations would start, then a SetBounds call
would skip the animation resulting in an abrupt first-load experience of
the keyboard jumping right to the end position and being blank (still
loading). After this fix the keyboard nicely animates till the end and
by the time the animation is finished the keyboard is loaded as well.

BUG=356753
TEST=Verified on linux chromeos.

Committed: https://crrev.com/c9bb939b8326bf72cb241b3eefa5d428906a043c
Cr-Commit-Position: refs/heads/master@{#297930}

[Peter Wen, 2014-09-30 19:15:18]

wnwen@chromium.org changed reviewers:
+ bshe@chromium.org, kevers@chromium.org

[Peter Wen, 2014-09-30 19:15:19]

This change includes resetting opacity to 0 as it does not crash with the value
at 0.

[kevers, 2014-09-30 19:35:15]

lgtm

[bshe, 2014-09-30 21:57:16]

lgtm

Do you mind to add a unit test in a follow up CL?

https://codereview.chromium.org/617013002/diff/1/ui/keyboard/keyboard_control...
File ui/keyboard/keyboard_controller_proxy.cc (right):

https://codereview.chromium.org/617013002/diff/1/ui/keyboard/keyboard_control...
ui/keyboard/keyboard_controller_proxy.cc:63: if (bounds != keyboard->bounds())
nit: It would be nice to add a comment on why this check is necessary. And just
out of curiosity, do we call move contents multiple times with one
window.resizeTo or window.resizeTo will trigger this function being called
multiple times?

[Peter Wen, 2014-10-01 21:48:22]

On 2014/09/30 21:57:16, bshe wrote:
> lgtm
> 
> Do you mind to add a unit test in a follow up CL?
> 
>
https://codereview.chromium.org/617013002/diff/1/ui/keyboard/keyboard_control...
> File ui/keyboard/keyboard_controller_proxy.cc (right):
> 
>
https://codereview.chromium.org/617013002/diff/1/ui/keyboard/keyboard_control...
> ui/keyboard/keyboard_controller_proxy.cc:63: if (bounds != keyboard->bounds())
> nit: It would be nice to add a comment on why this check is necessary. And
just
> out of curiosity, do we call move contents multiple times with one
> window.resizeTo or window.resizeTo will trigger this function being called
> multiple times?

Thanks for the review. Added comment and TODO to investigate window.resizeTo <=>
MoveContents calls.

Not sure how to add a unit test that tests this case specifically. I'd like to
ideally call MoveContents multiple times with the same arguments and confirm
that SetBounds on the keyboard window is only called once. I'll learn the
necessary moving parts of writing such a unit test and add it as a follow-up CL.

[Peter Wen, 2014-10-01 21:48:31]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-10-01 21:49:59]

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

[commit-bot: I haz the power, 2014-10-01 22:58:53]

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

[commit-bot: I haz the power, 2014-10-01 22:58:54]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming 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_...)

[Peter Wen, 2014-10-02 21:44:06]

It turns out that JS was calling window.resizeTo repeatedly with the same
arguments. I will make the changes there to prevent that, but keeping the change
here as MoveContents should be smart too. Adding unit test on JS side.

@rsadam - figured out reason why setting opacity to 0 crashes. Adding comments
in this CL.

[Peter Wen, 2014-10-02 22:07:10]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2014-10-02 22:08:26]

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

[commit-bot: I haz the power, 2014-10-02 22:36:25]

Committed patchset #7 (id:120001) as f3970531623cbaf9da9a3124684b3a11a05a4ec2

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

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