Fix a bug where in the shell the modal window was being visually stacked below the launcher after showing and hiding the non-modal transient.

BUG=none
TEST=open window type launcher, show and then hide a window using the show/hide button, then open a window-modal window. the window modal window would appear rendered behind the type launcher, but if you clicked on where it would have been you can actually move it out from under. this bug makes sure it is always stacked on top visually also.
Review URL: https://chromiumcodereview.appspot.com/9298038

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

4 files changed, 165 insertions, 1 deletions

diff --git a/ui/aura/test/test_windows.cc b/ui/aura/test/test_windows.cc
index 1c8a540..14fd638 100644
--- a/ui/aura/test/test_windows.cc
+++ b/ui/aura/test/test_windows.cc
@@ -54,5 +54,35 @@ Window* CreateTestWindowWithDelegateAndType(WindowDelegate* delegate,
   return window;
 }
 
+Window* CreateTransientChild(int id, Window* parent) {
+  Window* window = new Window(NULL);
+  window->set_id(id);
+  window->SetType(aura::client::WINDOW_TYPE_NORMAL);
+  window->Init(ui::Layer::LAYER_TEXTURED);
+  window->SetParent(NULL);
+  parent->AddTransientChild(window);
+  return window;
+}
+
+template <typename T>
+bool ObjectIsAbove(T* upper, T* lower) {
+  DCHECK_EQ(upper->parent(), lower->parent());
+  DCHECK_NE(upper, lower);
+  const std::vector<T*>& children = upper->parent()->children();
+  const size_t upper_i =
+      std::find(children.begin(), children.end(), upper) - children.begin();
+  const size_t lower_i =
+      std::find(children.begin(), children.end(), lower) - children.begin();
+  return upper_i > lower_i;
+}
+
+bool WindowIsAbove(Window* upper, Window* lower) {
+  return ObjectIsAbove<Window>(upper, lower);
+}
+
+bool LayerIsAbove(Window* upper, Window* lower) {
+  return ObjectIsAbove<ui::Layer>(upper->layer(), lower->layer());
+}
+
 }  // namespace test
 }  // namespace aura
diff --git a/ui/aura/test/test_windows.h b/ui/aura/test/test_windows.h
index 1781104..d0b782d 100644
--- a/ui/aura/test/test_windows.h
+++ b/ui/aura/test/test_windows.h
@@ -31,6 +31,16 @@ Window* CreateTestWindowWithDelegateAndType(WindowDelegate* delegate,
                                             const gfx::Rect& bounds,
                                             Window* parent);
 
+// Creates a transient child window of |parent|.
+Window* CreateTransientChild(int id, Window* parent);
+
+// Returns true if |upper| is above |lower| in the window stacking order.
+bool WindowIsAbove(Window* upper, Window* lower);
+
+// Returns true if |upper|'s layer is above |lower|'s layer in the layer
+// stacking order.
+bool LayerIsAbove(Window* upper, Window* lower);
+
 }  // namespace test
 }  // namespace aura
 
diff --git a/ui/aura/window.cc b/ui/aura/window.cc
index 93743d6..ccea8a0 100644
--- a/ui/aura/window.cc
+++ b/ui/aura/window.cc
@@ -247,6 +247,8 @@ void Window::StackChildAbove(Window* child, Window* other) {
   children_.erase(children_.begin() + child_i);
   children_.insert(children_.begin() + dest_i, child);
 
+  // See test WindowTest.StackingMadrigal for an explanation of this and the
+  // check below in the transient loop.
   if (other->layer()->delegate())
     layer()->StackAbove(child->layer(), other->layer());
 
@@ -258,7 +260,8 @@ void Window::StackChildAbove(Window* child, Window* other) {
     Window* transient_child = *i;
     if (transient_child->parent_ == this) {
       StackChildAbove(transient_child, last_transient);
-      last_transient = transient_child;
+      if (transient_child->layer()->delegate())
+        last_transient = transient_child;
     }
   }
 
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index 5bb0bbd..6c53e6b 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -12,6 +12,7 @@
 #include "ui/aura/client/visibility_client.h"
 #include "ui/aura/event.h"
 #include "ui/aura/focus_manager.h"
+#include "ui/aura/layout_manager.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/root_window_observer.h"
 #include "ui/aura/test/aura_test_base.h"
@@ -1341,5 +1342,125 @@ TEST_F(WindowTest, MouseEventsOnWindowChange) {
   EXPECT_EQ("1 1 0", d1.GetMouseCountsAndReset());
 }
 
+class StackingMadrigalLayoutManager : public LayoutManager {
+ public:
+  StackingMadrigalLayoutManager() {
+    RootWindow::GetInstance()->SetLayoutManager(this);
+  }
+  virtual ~StackingMadrigalLayoutManager() {
+  }
+
+ private:
+  // Overridden from LayoutManager:
+  virtual void OnWindowResized() OVERRIDE {}
+  virtual void OnWindowAddedToLayout(Window* child) OVERRIDE {}
+  virtual void OnWillRemoveWindowFromLayout(Window* child) OVERRIDE {}
+  virtual void OnChildWindowVisibilityChanged(Window* child,
+                                              bool visible) OVERRIDE {
+    Window::Windows::const_iterator it =
+        RootWindow::GetInstance()->children().begin();
+    Window* last_window = NULL;
+    for (; it != RootWindow::GetInstance()->children().end(); ++it) {
+      if (*it == child && last_window) {
+        if (!visible)
+          RootWindow::GetInstance()->StackChildAbove(last_window, *it);
+        else
+          RootWindow::GetInstance()->StackChildAbove(*it, last_window);
+        break;
+      }
+      last_window = *it;
+    }
+  }
+  virtual void SetChildBounds(Window* child,
+                              const gfx::Rect& requested_bounds) OVERRIDE {
+    SetChildBoundsDirect(child, requested_bounds);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(StackingMadrigalLayoutManager);
+};
+
+class StackingMadrigalVisibilityClient : public client::VisibilityClient {
+ public:
+  StackingMadrigalVisibilityClient() : ignored_window_(NULL) {
+    client::SetVisibilityClient(this);
+  }
+  virtual ~StackingMadrigalVisibilityClient() {
+    client::SetVisibilityClient(NULL);
+  }
+
+  void set_ignored_window(Window* ignored_window) {
+    ignored_window_ = ignored_window;
+  }
+
+ private:
+  // Overridden from client::VisibilityClient:
+  virtual void UpdateLayerVisibility(Window* window, bool visible) OVERRIDE {
+    if (!visible) {
+      if (window == ignored_window_)
+        window->layer()->set_delegate(NULL);
+      else
+        window->layer()->SetVisible(visible);
+    } else {
+      window->layer()->SetVisible(visible);
+    }
+  }
+
+  Window* ignored_window_;
+
+  DISALLOW_COPY_AND_ASSIGN(StackingMadrigalVisibilityClient);
+};
+
+// This test attempts to reconstruct a circumstance that can happen when the
+// aura client attempts to manipulate the visibility and delegate of a layer
+// independent of window visibility.
+// A use case is where the client attempts to keep a window's visible onscreen
+// even after code has called Hide() on the window. The use case for this would
+// be that window hides are animated (e.g. the window fades out). To prevent
+// spurious updating the client code may also clear window's layer's delegate,
+// so that the window cannot attempt to paint or update it further. The window
+// uses the presence of a NULL layer delegate as a signal in stacking to note
+// that the window is being manipulated by such a use case and its stacking
+// should not be adjusted.
+// One issue that can arise when a window opens two transient children, and the
+// first is hidden. Subsequent attempts to activate the transient parent can
+// result in the transient parent being stacked above the second transient
+// child. A fix is made to Window::StackAbove to prevent this, and this test
+// verifies this fix.
+TEST_F(WindowTest, StackingMadrigal) {
+  new StackingMadrigalLayoutManager;
+  StackingMadrigalVisibilityClient visibility_client;
+
+  scoped_ptr<Window> window1(CreateTestWindowWithId(1, NULL));
+  scoped_ptr<Window> window11(CreateTransientChild(11, window1.get()));
+
+  visibility_client.set_ignored_window(window11.get());
+
+  window11->Show();
+  window11->Hide();
+
+  // As a transient, window11 should still be stacked above window1, even when
+  // hidden.
+  EXPECT_TRUE(WindowIsAbove(window11.get(), window1.get()));
+  EXPECT_TRUE(LayerIsAbove(window11.get(), window1.get()));
+
+  scoped_ptr<Window> window12(CreateTransientChild(12, window1.get()));
+  window12->Show();
+
+  EXPECT_TRUE(WindowIsAbove(window12.get(), window11.get()));
+  EXPECT_TRUE(LayerIsAbove(window12.get(), window11.get()));
+
+  // Prior to the NULL check in the transient restacking loop in
+  // Window::StackChildAbove() introduced with this change, attempting to stack
+  // window1 above window12 at this point would actually restack the layers
+  // resulting in window12's layer being below window1's layer (though the
+  // windows themselves would still be correctly stacked, so events would pass
+  // through.)
+  RootWindow::GetInstance()->StackChildAbove(window1.get(), window12.get());
+
+  // Both window12 and its layer should be stacked above window1.
+  EXPECT_TRUE(WindowIsAbove(window12.get(), window1.get()));
+  EXPECT_TRUE(LayerIsAbove(window12.get(), window1.get()));
+}
+
 }  // namespace test
 }  // namespace aura