[cros Aura] Added modal container for screen lock.

[cros Aura] Added modal container for screen lock.

BUG=104133, 104135
TEST=ShellTest.CreateLockScreenModalWindow, ShellTest.CreateModalWindow.


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

[Nikita (slow), 2011-11-22 14:37:10]

Ben,
This is the most straightforward approach to display modal dialogs at login
screen (Lock screen layer).

I personally don't like 
if (!lock_container->children().size()) check but not sure there's easier way.

As far as I know we're not planning to show modal dialogs on lock screen, only
tooltips.
Login screen is a bit different story since it has several modal dialogs.

Moving modal dialogs layer on top of lock layer when lock layer becomes active
wouldn't work as one could have modal dialog open and then activate screen lock.
It would work for login screen case but I think we should not distinguish these
cases. In this particular example I'm not sure that input event filtering will
behave properly with current implementation but that's another issue.

[Ben Goodger (Google), 2011-11-22 15:53:24]

Can you add a test... probably to shell_unittest?

1. open something in the lock container.
2. open a modal window parented to that something.
3. verify it got put in the right container.

[Nikita (slow), 2011-11-22 15:56:42]

Sure. Just wanted to confirm direction.
22.11.2011 19:53 пользователь <ben@chromium.org> написал:

> Can you add a test... probably to shell_unittest?
>
> 1. open something in the lock container.
> 2. open a modal window parented to that something.
> 3. verify it got put in the right container.
>
>
http://codereview.chromium.**org/8638013/<http://codereview.chromium.org/8638...
>

[Nikita (slow), 2011-11-23 16:33:12]

Added tests.

[Nikita (slow), 2011-11-24 02:42:21]

Please take a look.
23.11.2011 20:33 пользователь <nkostylev@chromium.org> написал:

> Added tests.
>
>
http://codereview.chromium.**org/8638013/<http://codereview.chromium.org/8638...
>

[Ben Goodger (Google), 2011-11-28 17:12:56]

http://codereview.chromium.org/8638013/diff/6001/ui/aura_shell/stacking_contr...
File ui/aura_shell/stacking_controller.cc (right):

http://codereview.chromium.org/8638013/diff/6001/ui/aura_shell/stacking_contr...
ui/aura_shell/stacking_controller.cc:82: if (!lock_container->children().size())
I realized there is a flaw here that will require some more complexity.

If the modal window is coming from a window behind the lock screen (consider a
page showing a js modal alert), then showing the modal dialog above the lock
screen would violate the privacy of the user whose session was locked.

Thus the following makes sense to me:

When a lock screen is active, modal dialogs opened from content in containers
behind the lock screen should still open in the regular modal container. The
event filter there would be superceded by the event filter attached to the lock
screen since the lock screen one is in front of it in the z-index and is thus
consulted first.

Modal windows opened from content in the lock container, or any container in
front of it, would be opened in the lock modal container.

So, I think you'll have to write a little more code here (perhaps move out to a
separate function since this function is going to be pretty long otherwise)...
and adjust your tests to consider both these cases.

[Nikita (slow), 2011-11-28 17:51:03]

Ok, that makes sense. I wasn't sure about this part too.
I'll rewrite the patch.

[Nikita (slow), 2011-11-29 10:06:34]

Please take a look.

http://codereview.chromium.org/8638013/diff/6001/ui/aura_shell/stacking_contr...
File ui/aura_shell/stacking_controller.cc (right):

http://codereview.chromium.org/8638013/diff/6001/ui/aura_shell/stacking_contr...
ui/aura_shell/stacking_controller.cc:82: if (!lock_container->children().size())
On 2011/11/28 17:12:56, Ben Goodger (Google) wrote:
> I realized there is a flaw here that will require some more complexity.
> 
> If the modal window is coming from a window behind the lock screen (consider a
> page showing a js modal alert), then showing the modal dialog above the lock
> screen would violate the privacy of the user whose session was locked.
> 
> Thus the following makes sense to me:
> 
> When a lock screen is active, modal dialogs opened from content in containers
> behind the lock screen should still open in the regular modal container. The
> event filter there would be superceded by the event filter attached to the
lock
> screen since the lock screen one is in front of it in the z-index and is thus
> consulted first.
> 
> Modal windows opened from content in the lock container, or any container in
> front of it, would be opened in the lock modal container.
> 
> So, I think you'll have to write a little more code here (perhaps move out to
a
> separate function since this function is going to be pretty long otherwise)...
> and adjust your tests to consider both these cases.

Done.

[Ben Goodger (Google), 2011-11-29 16:42:26]

lgtm

http://codereview.chromium.org/8638013/diff/15001/ui/aura_shell/stacking_cont...
File ui/aura_shell/stacking_controller.cc (right):

http://codereview.chromium.org/8638013/diff/15001/ui/aura_shell/stacking_cont...
ui/aura_shell/stacking_controller.cc:136: else
nit: no else after return

[Nikita (slow), 2011-11-29 17:12:32]

http://codereview.chromium.org/8638013/diff/15001/ui/aura_shell/stacking_cont...
File ui/aura_shell/stacking_controller.cc (right):

http://codereview.chromium.org/8638013/diff/15001/ui/aura_shell/stacking_cont...
ui/aura_shell/stacking_controller.cc:136: else
On 2011/11/29 16:42:26, Ben Goodger (Google) wrote:
> nit: no else after return

Done.

[commit-bot: I haz the power, 2011-11-29 18:40:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nkostylev@chromium.org/8638013/13012

[commit-bot: I haz the power, 2011-11-29 20:26:26]

Change committed as 112015