Wrap the virtual keyboard in a container view.

Wrap the virtual keyboard in a container view.

This allows for:
 - adding UI that will distinguish the keyboard from the client view
 - controlling focus behaviour of the keyboard: it is made unfocusable
   here as well

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71130

[bryeung, 2011-01-05 04:06:20]

Scott,

Let me know if you have ideas for how to better handle preserving focus in the
ClientView.  I've tested this implementation against our prototype keyboard and
it works, but it seems a bit heavy handed.

Also, while I don't think the KeyboardContainer is strictly necessary I think it
imposes a better structure to the code and that we might want it to be separate
especially as we start decorating the keyboard.  Thoughts?

Thanks!

[sky, 2011-01-05 16:52:51]

Using a specific class rather than a view is fine, but might not be entirely
necessary yet.

I don't think I understand the problem well enough. Who ends up requesting focus
as the user types?

  -Scott

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
File chrome/browser/ui/touch/frame/keyboard_container_view.cc (right):

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:43:
dom_view_->SetBounds(0, 0, width(), height());
Use FillLayout instead of this.

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:48:
MakeViewHierarchyUnfocusable(this);
Can you do this once? Say in the constructor or in ViewHieararchyChanged?

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
File chrome/browser/ui/touch/frame/keyboard_container_view.h (right):

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.h:14: /**
Use C++ style comments, not Java.

[bryeung, 2011-01-10 22:32:19]

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
File chrome/browser/ui/touch/frame/keyboard_container_view.cc (right):

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:43:
dom_view_->SetBounds(0, 0, width(), height());
On 2011/01/05 16:52:51, sky wrote:
> Use FillLayout instead of this.

I switched to FillLayout but that causes rendering issues: namely the contents
of the DOMView don't show up.  As I'm going to need custom layout to put a
border on this anyway, are you okay with this going in as-is, or should I dig
into this more deeply?

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:48:
MakeViewHierarchyUnfocusable(this);
On 2011/01/05 16:52:51, sky wrote:
> Can you do this once? Say in the constructor or in ViewHieararchyChanged?

Seems to work in ViewHierarchyChanged.  Good suggestion: thanks!

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
File chrome/browser/ui/touch/frame/keyboard_container_view.h (right):

http://codereview.chromium.org/6020013/diff/2001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.h:14: /**
On 2011/01/05 16:52:51, sky wrote:
> Use C++ style comments, not Java.

Oops.  Sorry.

[sky, 2011-01-10 22:38:09]

LGTM with the following changes.

http://codereview.chromium.org/6020013/diff/8001/chrome/browser/ui/touch/fram...
File chrome/browser/ui/touch/frame/keyboard_container_view.cc (right):

http://codereview.chromium.org/6020013/diff/8001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:48: void
KeyboardContainerView::Layout() {
Order should match header.

http://codereview.chromium.org/6020013/diff/8001/chrome/browser/ui/touch/fram...
chrome/browser/ui/touch/frame/keyboard_container_view.cc:51:
dom_view_->Layout();
You shouldn't need this Layout. SetBounds invokes Layout as necessary.

[sky, 2011-01-10 22:43:18]

As discussed, I'm ok with leaving in Layout call and adding TODO to figure out
why it is needed.

  -Scott