Prevent system modal dialog from blocking events to the virtual keyboard.

Prevent system modal dialog from blocking events to the virtual keyboard.

BUG=https://code.google.com/p/chrome-os-partner/issues/detail?id=27142

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

[kevers, 2014-03-28 15:03:47]

Hi Oshima,

Can you please take a look at this CL.

[oshima, 2014-03-28 16:55:14]

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc#newcode1094
ash/shell.cc:1094: if (keyboard_container &&
keyboard_container->Contains(window))
Will

keyboard_contorller->GetContainerWindow()->Contains(window)

work?

[oshima, 2014-03-28 16:55:29]

On 2014/03/28 16:55:14, oshima wrote:
> https://codereview.chromium.org/216743004/diff/1/ash/shell.cc
> File ash/shell.cc (right):
> 
> https://codereview.chromium.org/216743004/diff/1/ash/shell.cc#newcode1094
> ash/shell.cc:1094: if (keyboard_container &&
> keyboard_container->Contains(window))
> Will
> 
> keyboard_contorller->GetContainerWindow()->Contains(window)
> 
> work?

and can you add test?

[kevers, 2014-03-28 17:52:36]

Will add test.

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc#newcode1094
ash/shell.cc:1094: if (keyboard_container &&
keyboard_container->Contains(window))
On 2014/03/28 16:55:15, oshima wrote:
> Will
> 
> keyboard_contorller->GetContainerWindow()->Contains(window)
> 
> work?

keyboard_controller->GetContainerWindow() has side effects creating the keyboard
window if it does not already exist.

[oshima, 2014-03-28 20:02:41]

(sorry for delay. I thought I hit 'publish' but apparently did not)

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc#newcode1094
ash/shell.cc:1094: if (keyboard_container &&
keyboard_container->Contains(window))
On 2014/03/28 17:52:37, kevers wrote:
> On 2014/03/28 16:55:15, oshima wrote:
> > Will
> > 
> > keyboard_contorller->GetContainerWindow()->Contains(window)
> > 
> > work?
> 
> keyboard_controller->GetContainerWindow() has side effects creating the
keyboard
> window if it does not already exist.

If that's the case, can you check this in RWC (say
RWC::IsVirtualkeyboardWindow(aura::Window*)) and check it in the loop above?
Currently, only RWC knows about the virtual keyboard container and it'd be
better to keep it that way.

[kevers, 2014-03-28 21:11:00]

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/1/ash/shell.cc#newcode1094
ash/shell.cc:1094: if (keyboard_container &&
keyboard_container->Contains(window))
On 2014/03/28 20:02:42, oshima wrote:
> On 2014/03/28 17:52:37, kevers wrote:
> > On 2014/03/28 16:55:15, oshima wrote:
> > > Will
> > > 
> > > keyboard_contorller->GetContainerWindow()->Contains(window)
> > > 
> > > work?
> > 
> > keyboard_controller->GetContainerWindow() has side effects creating the
> keyboard
> > window if it does not already exist.
> 
> If that's the case, can you check this in RWC (say
> RWC::IsVirtualkeyboardWindow(aura::Window*)) and check it in the loop above?
> Currently, only RWC knows about the virtual keyboard container and it'd be
> better to keep it that way.

Done.

[oshima, 2014-03-28 21:49:56]

lgtm with a nit

https://codereview.chromium.org/216743004/diff/40001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/40001/ash/shell.cc#newcode1092
ash/shell.cc:1092: if
(GetPrimaryRootWindowController()->IsVirtualKeyboardWindow(window))
can you move this to the loop above? I think it's more robust because it'll work
even if the VK moved to another root window.

[kevers, 2014-03-31 14:46:18]

The CQ bit was checked by kevers@chromium.org

[commit-bot: I haz the power, 2014-03-31 14:46:43]

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

[commit-bot: I haz the power, 2014-03-31 17:07:35]

Change committed as 260576

[kevers, 2014-03-31 17:21:17]

https://codereview.chromium.org/216743004/diff/40001/ash/shell.cc
File ash/shell.cc (right):

https://codereview.chromium.org/216743004/diff/40001/ash/shell.cc#newcode1092
ash/shell.cc:1092: if
(GetPrimaryRootWindowController()->IsVirtualKeyboardWindow(window))
On 2014/03/28 21:49:56, oshima wrote:
> can you move this to the loop above? I think it's more robust because it'll
work
> even if the VK moved to another root window.

Done.