keyboard: Add a window delegate to the keyboard window container.

keyboard: Add a window delegate to the keyboard window container.

The delegate makes sure that the keyboard window does not get focus,
and that events are not consumed by the container window itself.

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

[bryeung, 2013-04-05 19:28:57]

lgtm

https://codereview.chromium.org/13726015/diff/4001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/4001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:39: SetChildBoundsDirect(keyboard_,
keyboard_bounds);
might as well:

SetChildBoundsDirect(keyboard_,
KeyboardBoundsFromWindowBounds(owner_->bounds()))

now

[sadrul, 2013-04-08 04:09:48]

+ben@ for OWNERS

https://codereview.chromium.org/13726015/diff/4001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/4001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:39: SetChildBoundsDirect(keyboard_,
keyboard_bounds);
On 2013/04/05 19:28:57, bryeung wrote:
> might as well:
> 
> SetChildBoundsDirect(keyboard_,
> KeyboardBoundsFromWindowBounds(owner_->bounds()))
> 
> now

Done.

[Ben Goodger (Google), 2013-04-08 21:19:14]

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:63: class KeyboardWindowDelegate : public
aura::WindowDelegate {
this seems kinda heavy handed. is there a way to do this just by using event
handlers? what is the specific method you needed to override here?

[sadrul, 2013-04-08 21:27:10]

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:63: class KeyboardWindowDelegate : public
aura::WindowDelegate {
On 2013/04/08 21:19:14, Ben Goodger (Google) wrote:
> this seems kinda heavy handed. is there a way to do this just by using event
> handlers? what is the specific method you needed to override here?

The main necessary bit here is the CanFocus() override. An alternating approach
could be to install an event-client (separately in ash, and other places that
use this) that stops the window from getting focus. Having the window-delegate
simplifies this in the sense that the client that uses the keyboard needs to do
less work to set things up correctly.

[Ben Goodger (Google), 2013-04-08 21:30:06]

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:87: virtual bool CanFocus() OVERRIDE { return
false; }
my next question is... why is it insufficient to rely on the embedded widget's
CanFocus impl?

[sadrul, 2013-04-08 21:38:53]

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_contr...
ui/keyboard/keyboard_controller.cc:87: virtual bool CanFocus() OVERRIDE { return
false; }
On 2013/04/08 21:30:06, Ben Goodger (Google) wrote:
> my next question is... why is it insufficient to rely on the embedded widget's
> CanFocus impl?

Right now, we do not use views Widgets for hosting the keyboard. We have a
WebContent, and put its aura::Window inside another aura::Window that plays the
role of its container.

[Ben Goodger (Google), 2013-04-08 21:40:56]

IC. OK. I guess I can live with this. LGTM :-)


On Mon, Apr 8, 2013 at 2:38 PM, <sadrul@chromium.org> wrote:

>
> https://codereview.chromium.**org/13726015/diff/8001/ui/**
>
keyboard/keyboard_controller.**cc<https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_controller.cc>
> File ui/keyboard/keyboard_**controller.cc (right):
>
> https://codereview.chromium.**org/13726015/diff/8001/ui/**
>
keyboard/keyboard_controller.**cc#newcode87<https://codereview.chromium.org/13726015/diff/8001/ui/keyboard/keyboard_controller.cc#newcode87>
> ui/keyboard/keyboard_**controller.cc:87: virtual bool CanFocus() OVERRIDE
> { return false; }
> On 2013/04/08 21:30:06, Ben Goodger (Google) wrote:
>
>> my next question is... why is it insufficient to rely on the embedded
>>
> widget's
>
>> CanFocus impl?
>>
>
> Right now, we do not use views Widgets for hosting the keyboard. We have
> a WebContent, and put its aura::Window inside another aura::Window that
> plays the role of its container.
>
>
https://codereview.chromium.**org/13726015/<https://codereview.chromium.org/1...
>

[sadrul, 2013-04-10 15:51:06]

Committed patchset #3 manually as r193392 (presubmit successful).