Consolidate the code path when window becomes invisible. This fixes a couple of focus related issues.

* focus didn't move when a window becomes invisible.
* transient parent didn't get focus if the transient child is removed from root hierarchy.

BUG=none
TEST=new test cases is added to root_window_event_filter.

Review URL: http://codereview.chromium.org/9398004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@122302 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 52 insertions, 2 deletions

diff --git a/ash/wm/root_window_event_filter_unittest.cc b/ash/wm/root_window_event_filter_unittest.cc
index 9cd2829..41a37eb 100644
--- a/ash/wm/root_window_event_filter_unittest.cc
+++ b/ash/wm/root_window_event_filter_unittest.cc
@@ -9,6 +9,7 @@
 #include "ash/wm/activation_controller.h"
 #include "ash/wm/window_util.h"
 #include "ui/aura/client/activation_delegate.h"
+#include "ui/aura/client/activation_client.h"
 #include "ui/aura/cursor.h"
 #include "ui/aura/event.h"
 #include "ui/aura/root_window.h"
@@ -89,6 +90,10 @@ TEST_F(RootWindowEventFilterTest, Focus) {
       new aura::test::ColorTestWindowDelegate(SK_ColorRED);
   scoped_ptr<aura::Window> w122(aura::test::CreateTestWindowWithDelegate(
       w122delegate, -122, gfx::Rect(10, 5, 5, 5), w12.get()));
+  aura::test::ColorTestWindowDelegate* w123delegate =
+      new aura::test::ColorTestWindowDelegate(SK_ColorRED);
+  scoped_ptr<aura::Window> w123(aura::test::CreateTestWindowWithDelegate(
+      w123delegate, -123, gfx::Rect(15, 5, 5, 5), w12.get()));
   scoped_ptr<aura::Window> w13(aura::test::CreateTestWindow(
       SK_ColorGRAY, -13, gfx::Rect(5, 470, 50, 50), w1.get()));
 
@@ -116,11 +121,49 @@ TEST_F(RootWindowEventFilterTest, Focus) {
   root_window->DispatchKeyEvent(&keyev);
   EXPECT_EQ(ui::VKEY_E, w122delegate->last_key_code());
 
-  // Removing the focused window from parent should reset the focused window.
+  // Hiding the focused window will set the focus to its parent if
+  // it's focusable.
+  w122->Hide();
+  EXPECT_EQ(w12->GetFocusManager(), w122->GetFocusManager());
+  EXPECT_EQ(w12.get(), w12->GetFocusManager()->GetFocusedWindow());
+
+  // Sets the focus back to w122.
+  w122->Show();
+  w122->Focus();
+  EXPECT_EQ(w122.get(), w12->GetFocusManager()->GetFocusedWindow());
+
+  // Removing the focused window from parent should set the focus to
+  // its parent if it's focusable.
   w12->RemoveChild(w122.get());
   EXPECT_EQ(NULL, w122->GetFocusManager());
+  EXPECT_EQ(w12.get(), w12->GetFocusManager()->GetFocusedWindow());
+
+  // Set the focus to w123, but make the w1 not activatable.
+  TestActivationDelegate *activation_delegate = new
+      TestActivationDelegate(false);
+  w123->Focus();
+  EXPECT_EQ(w123.get(), w12->GetFocusManager()->GetFocusedWindow());
+  aura::client::SetActivationDelegate(w1.get(), activation_delegate);
+
+  // Hiding the focused window will set the focus to NULL because
+  // parent window is not focusable.
+  w123->Hide();
+  EXPECT_EQ(w12->GetFocusManager(), w123->GetFocusManager());
   EXPECT_EQ(NULL, w12->GetFocusManager()->GetFocusedWindow());
   EXPECT_FALSE(root_window->DispatchKeyEvent(&keyev));
+
+  // Set the focus back to w123
+  aura::client::SetActivationDelegate(w1.get(), NULL);
+  w123->Show();
+  w123->Focus();
+  EXPECT_EQ(w123.get(), w12->GetFocusManager()->GetFocusedWindow());
+  aura::client::SetActivationDelegate(w1.get(), activation_delegate);
+
+  // Removing the focused window will set the focus to NULL because
+  // parent window is not focusable.
+  w12->RemoveChild(w123.get());
+  EXPECT_EQ(NULL, w123->GetFocusManager());
+  EXPECT_FALSE(root_window->DispatchKeyEvent(&keyev));
 }
 
 // Various assertion testing for activating windows.
diff --git a/ash/wm/window_modality_controller.cc b/ash/wm/window_modality_controller.cc
index f459cf3..b60fb0b 100644
--- a/ash/wm/window_modality_controller.cc
+++ b/ash/wm/window_modality_controller.cc
@@ -39,7 +39,7 @@ aura::Window* WindowModalityController::GetWindowModalTransient(
   for (it = window->transient_children().begin();
        it != window->transient_children().end();
        ++it) {
-    if (TransientChildIsWindowModal(*it)) {
+    if (TransientChildIsWindowModal(*it) && (*it)->IsVisible()) {
       if (!(*it)->transient_children().empty())
         return GetWindowModalTransient(*it);
       return *it;
diff --git a/ash/wm/window_modality_controller_unittest.cc b/ash/wm/window_modality_controller_unittest.cc
index c1eb06a..4169880 100644
--- a/ash/wm/window_modality_controller_unittest.cc
+++ b/ash/wm/window_modality_controller_unittest.cc
@@ -161,6 +161,13 @@ TEST_F(WindowModalityControllerTest, NestedModalsOuterClosed) {
   ActivateWindow(w1.get());
   EXPECT_TRUE(IsActiveWindow(w111));
 
+  w111->Hide();
+  EXPECT_TRUE(IsActiveWindow(w11.get()));
+
+  // TODO(oshima): Re-showing doesn't set the focus back to
+  // modal window. There is no such use case right now, but it
+  // probably should.
+
   w11.reset();
   EXPECT_TRUE(IsActiveWindow(w1.get()));
 }