Support modal windows

Support modal windows

* New ContainerParams
 -  default_parent to specify the default parent when no
    transient parent is specified.
 - modal_container_priority now controls the modal container
   used for a modal window created for the given container.
 - if not specified, it will fallback downwards to find one.
 - if window is specified as always_on_top, it will use top
   most modal container.

* Changed network selector/shutdown dialog to use the new API.

* Other change:
 Separated test windows to athena/test/base/test_windows.h

BUG=410499
TBR=sky@chromium.org,reed@chromium.org
TEST=coverd by unit tests

Committed: https://crrev.com/4903fd36b2b36f00efeb1b7bba81b7de6e9457a3
Cr-Commit-Position: refs/heads/master@{#300192}

Committed: https://crrev.com/44bf2fd5fb11d3777e522fae68de024c8f6ce46f
Cr-Commit-Position: refs/heads/master@{#300288}

[oshima, 2014-10-17 15:47:21]

oshima@chromium.org changed reviewers:
+ mukai@chromium.org

[Jun Mukai, 2014-10-17 17:29:54]

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
File athena/screen/modal_window_controller.cc (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:70: if (IsChildWindow(window) && window
!= dimmer_window_)
IsChildWindow already checks window != dimmer_window_ right?

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:137: // immediately.
It seems not detaching at all?

https://codereview.chromium.org/662763002/diff/20001/athena/screen/public/scr...
File athena/screen/public/screen_manager.h (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:53: //   starting from this container.
I am not sure starting from which container.  Can you explain a bit more
verbose?

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:159: public aura::WindowObserver {
It doesn't have to be WindowObserver anymore?

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:180: for (; iter != containers.end();
++iter) {
You can simply using r_iter (but --r_iter) right?

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:255: scoped_ptr<ui::EventTargeter>(new
AthenaWindowTargeter(root_window_)));
make_scoped_ptr(...) is more concise

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:285: #if !defined(NDEBUG)
It's not related to this, but why it's enclosed by NDEBUG?  I suspect that's
because originally std::find_if or something is outside of DCHECK and don't want
to run unnecessary code.

If my suspicion is correct, this NDEBUG is not necessary anymore.

[oshima, 2014-10-17 18:20:06]

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
File athena/screen/modal_window_controller.cc (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:70: if (IsChildWindow(window) && window
!= dimmer_window_)
On 2014/10/17 17:29:53, Jun Mukai wrote:
> IsChildWindow already checks window != dimmer_window_ right?

thank you for catching this. Done.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:137: // immediately.
On 2014/10/17 17:29:53, Jun Mukai wrote:
> It seems not detaching at all?

ScopedLayerAnimationSettings does. Updated the comment.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/public/scr...
File athena/screen/public/screen_manager.h (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:53: //   starting from this container.
On 2014/10/17 17:29:54, Jun Mukai wrote:
> I am not sure starting from which container.  Can you explain a bit more
> verbose?

Updated the comment.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:159: public aura::WindowObserver {
On 2014/10/17 17:29:54, Jun Mukai wrote:
> It doesn't have to be WindowObserver anymore?

Good catch. Done.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:180: for (; iter != containers.end();
++iter) {
On 2014/10/17 17:29:54, Jun Mukai wrote:
> You can simply using r_iter (but --r_iter) right?

You're right. done.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:255: scoped_ptr<ui::EventTargeter>(new
AthenaWindowTargeter(root_window_)));
On 2014/10/17 17:29:54, Jun Mukai wrote:
> make_scoped_ptr(...) is more concise

Done.

https://codereview.chromium.org/662763002/diff/20001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:285: #if !defined(NDEBUG)
On 2014/10/17 17:29:54, Jun Mukai wrote:
> It's not related to this, but why it's enclosed by NDEBUG?  I suspect that's
> because originally std::find_if or something is outside of DCHECK and don't
want
> to run unnecessary code.
> 
> If my suspicion is correct, this NDEBUG is not necessary anymore.

Done.

[Jun Mukai, 2014-10-17 20:13:57]

lgtm

[oshima, 2014-10-17 20:49:41]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-10-17 20:50:31]

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

[commit-bot: I haz the power, 2014-10-17 20:58:37]

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

[commit-bot: I haz the power, 2014-10-17 20:58:39]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[oshima, 2014-10-17 21:59:05]

Patchset #4 (id:60001) has been deleted

[oshima, 2014-10-17 23:42:49]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-10-17 23:44:02]

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

[commit-bot: I haz the power, 2014-10-18 00:20:56]

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

[commit-bot: I haz the power, 2014-10-18 00:20:57]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[oshima, 2014-10-18 00:43:38]

oshima@chromium.org changed reviewers:
+ reed@chromium.org, sky@chromium.org

[oshima, 2014-10-18 00:44:34]

tbr'ing sky for ui/wm/core -> ui/wm change in athena/test/DEPS
and reed@ for third_party/skia/include in athena/screen/DEPS

[oshima, 2014-10-18 00:44:39]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-10-18 00:46:16]

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

[commit-bot: I haz the power, 2014-10-18 00:59:20]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2014-10-18 01:00:04]

Patchset 4 (id:??) landed as
https://crrev.com/4903fd36b2b36f00efeb1b7bba81b7de6e9457a3
Cr-Commit-Position: refs/heads/master@{#300192}

[pkotwicz, 2014-10-20 00:45:07]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/665803003/ by pkotwicz@chromium.org.

The reason for reverting is: Reverting because this CL because
WithOrWithoutTargeter.* introduced a memory leak and made all of the Athena Asan
tests fail
http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20...

Based on tools/lsan/suppressions.txt reverting is now preferred over adding a
suppression.

[pkotwicz, 2014-10-20 14:54:50]

pkotwicz@chromium.org changed reviewers:
+ pkotwicz@chromium.org

[pkotwicz, 2014-10-20 14:54:52]

https://codereview.chromium.org/662763002/diff/80001/athena/screen/modal_wind...
File athena/screen/modal_window_controller.cc (right):

https://codereview.chromium.org/662763002/diff/80001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:128: const int kDimmAnimationDurationMs
= 500;
kDimmAnimationDurationMs -> kDimAnimationDurationMs

https://codereview.chromium.org/662763002/diff/80001/athena/screen/modal_wind...
athena/screen/modal_window_controller.cc:136: // ScopedHidingAnimationSettings
will detach the animating and
animating -> animation

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
File athena/screen/public/screen_manager.h (right):

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:32: // True if the container will block
evnets from containers behind it.
evnets -> events

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:44: // use a trnasient parent, not a
direct parent, or no transient parent.)
trnasient -> transient

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:48: // that has the modal container
proiroty.
proiroty -> priority

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:53: // 2.a) If
aura::client::kAlwaysOnTopKey is NOT set, it uses the stand flow
stand -> standard ?

Random comment: I would have added GetSystemModalPriority(int default_priority)
to athena/util/container_priorities.h
I would pass in CP_END if the window has the kAlwaysOnTopKey key set in addition
to being system modal. The implementation of GetSystemModalPriority() would be a
switch statement.

https://codereview.chromium.org/662763002/diff/80001/athena/screen/public/scr...
athena/screen/public/screen_manager.h:55: // 2.b) If
aura::client::kAlwaysOnTopKey is set, it searches the top most
top most -> topmost

https://codereview.chromium.org/662763002/diff/80001/athena/screen/screen_man...
File athena/screen/screen_manager_impl.cc (right):

https://codereview.chromium.org/662763002/diff/80001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:28: // This is to avoid creating type
definitoin for kAlwaysOnTopKey.
definitoin -> definition

https://codereview.chromium.org/662763002/diff/80001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:40: // A functor to find a container that
has the higher priority.
How about: "A functor to find the highest priority container."

https://codereview.chromium.org/662763002/diff/80001/athena/screen/screen_man...
athena/screen/screen_manager_impl.cc:273: // mmodal container's priority must be
higher than the container's priority.
mmodal -> Modal

https://codereview.chromium.org/662763002/diff/80001/athena/system/shutdown_d...
File athena/system/shutdown_dialog.cc (right):

https://codereview.chromium.org/662763002/diff/80001/athena/system/shutdown_d...
athena/system/shutdown_dialog.cc:18: #include
"ui/views/window/dialog_delegate.h"
You meant to include widget_delegate.h ?

https://codereview.chromium.org/662763002/diff/80001/athena/system/shutdown_d...
File athena/system/shutdown_dialog.h (right):

https://codereview.chromium.org/662763002/diff/80001/athena/system/shutdown_d...
athena/system/shutdown_dialog.h:26: explicit ShutdownDialog();
Remove explicit

https://codereview.chromium.org/662763002/diff/80001/athena/system/status_ico...
File athena/system/status_icon_container_view.h (right):

https://codereview.chromium.org/662763002/diff/80001/athena/system/status_ico...
athena/system/status_icon_container_view.h:17:
StatusIconContainerView(SystemUI::ColorScheme color_scheme);
explicit

https://codereview.chromium.org/662763002/diff/80001/athena/system/system_ui_...
File athena/system/system_ui_impl.cc (right):

https://codereview.chromium.org/662763002/diff/80001/athena/system/system_ui_...
athena/system/system_ui_impl.cc:30: SystemInfoView(SystemUI::ColorScheme
color_scheme)
explicit

https://codereview.chromium.org/662763002/diff/80001/athena/system/system_ui_...
athena/system/system_ui_impl.cc:75: SystemUIImpl(scoped_refptr<base::TaskRunner>
blocking_task_runner)
explicit

[oshima, 2014-10-20 16:49:49]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-10-20 16:50:18]

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

[commit-bot: I haz the power, 2014-10-20 17:19:30]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2014-10-20 17:20:49]

Patchset 5 (id:??) landed as
https://crrev.com/44bf2fd5fb11d3777e522fae68de024c8f6ce46f
Cr-Commit-Position: refs/heads/master@{#300288}