Fixes crashes and other bad behavior with modal dialogs. Maintains that transient children of docked windows or attached panels are parented in the same container as their transient parents. Ignores transient children for the purposes of panel or docked layout.

Fixes the stacking order of groups with transient children to move all descendants of common siblings when stacking above or below other windows.

BUG=297974
BUG=297977
TEST=follow the steps in bug repro.
TEST=ash_unittests --gtest_filter=*DragWindowWithTransientChild*
TEST=ash_unittests --gtest_filter=*PanelWithTransientChild*

Review URL: https://codereview.chromium.org/24450003

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

13 files changed, 427 insertions, 61 deletions

diff --git a/ash/wm/dock/docked_window_layout_manager.cc b/ash/wm/dock/docked_window_layout_manager.cc
index ad2a913..1475031 100644
--- a/ash/wm/dock/docked_window_layout_manager.cc
+++ b/ash/wm/dock/docked_window_layout_manager.cc
@@ -89,11 +89,17 @@ DockedWindowLayoutManager* GetDockLayoutManager(aura::Window* window,
       dock->layout_manager());
 }
 
+// Returns true if a window is a popup or a transient child.
+bool IsPopupOrTransient(aura::Window* window) {
+  return (window->type() == aura::client::WINDOW_TYPE_POPUP ||
+          window->transient_parent());
+}
+
 // Certain windows (minimized, hidden or popups) do not matter to docking.
 bool IsUsedByLayout(aura::Window* window) {
   return (window->IsVisible() &&
           !wm::GetWindowState(window)->IsMinimized() &&
-          window->type() != aura::client::WINDOW_TYPE_POPUP);
+          !IsPopupOrTransient(window));
 }
 
 // A functor used to sort the windows in order of their center Y position.
@@ -211,6 +217,7 @@ void DockedWindowLayoutManager::RemoveObserver(
 void DockedWindowLayoutManager::StartDragging(aura::Window* window) {
   DCHECK(!dragged_window_);
   dragged_window_ = window;
+  DCHECK(!IsPopupOrTransient(window));
   // Start observing a window unless it is docked container's child in which
   // case it is already observed.
   if (dragged_window_->parent() != dock_container_) {
@@ -222,11 +229,13 @@ void DockedWindowLayoutManager::StartDragging(aura::Window* window) {
 }
 
 void DockedWindowLayoutManager::DockDraggedWindow(aura::Window* window) {
+  DCHECK(!IsPopupOrTransient(window));
   OnWindowDocked(window);
   Relayout();
 }
 
 void DockedWindowLayoutManager::UndockDraggedWindow() {
+  DCHECK(!IsPopupOrTransient(dragged_window_));
   OnWindowUndocked();
   Relayout();
   UpdateDockBounds();
@@ -235,6 +244,7 @@ void DockedWindowLayoutManager::UndockDraggedWindow() {
 
 void DockedWindowLayoutManager::FinishDragging() {
   DCHECK(dragged_window_);
+  DCHECK(!IsPopupOrTransient(dragged_window_));
   if (is_dragged_window_docked_)
     OnWindowUndocked();
   DCHECK (!is_dragged_window_docked_);
@@ -299,10 +309,8 @@ DockedAlignment DockedWindowLayoutManager::CalculateAlignment() const {
   // the docked state).
   for (size_t i = 0; i < dock_container_->children().size(); ++i) {
     aura::Window* window(dock_container_->children()[i]);
-    if (window != dragged_window_ &&
-        window->type() != aura::client::WINDOW_TYPE_POPUP) {
+    if (window != dragged_window_ && !IsPopupOrTransient(window))
       return alignment_;
-    }
   }
   // No docked windows remain other than possibly the window being dragged.
   // Return |NONE| to indicate that windows may get docked on either side.
@@ -315,6 +323,10 @@ bool DockedWindowLayoutManager::CanDockWindow(aura::Window* window,
       switches::kAshEnableDockedWindows)) {
     return false;
   }
+  // Don't allow interactive docking of windows with transient parents such as
+  // modal browser dialogs.
+  if (IsPopupOrTransient(window))
+    return false;
   // If a window is wide and cannot be resized down to maximum width allowed
   // then it cannot be docked.
   if (window->bounds().width() > kMaxDockWidth &&
@@ -351,7 +363,7 @@ void DockedWindowLayoutManager::OnWindowResized() {
 }
 
 void DockedWindowLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
-  if (child->type() == aura::client::WINDOW_TYPE_POPUP)
+  if (IsPopupOrTransient(child))
     return;
   // Dragged windows are already observed by StartDragging and do not change
   // docked alignment during the drag.
@@ -370,7 +382,7 @@ void DockedWindowLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
 }
 
 void DockedWindowLayoutManager::OnWindowRemovedFromLayout(aura::Window* child) {
-  if (child->type() == aura::client::WINDOW_TYPE_POPUP)
+  if (IsPopupOrTransient(child))
     return;
   // Dragged windows are stopped being observed by FinishDragging and do not
   // change alignment during the drag. They also cannot be set to be the
@@ -392,7 +404,7 @@ void DockedWindowLayoutManager::OnWindowRemovedFromLayout(aura::Window* child) {
 void DockedWindowLayoutManager::OnChildWindowVisibilityChanged(
     aura::Window* child,
     bool visible) {
-  if (child->type() == aura::client::WINDOW_TYPE_POPUP)
+  if (IsPopupOrTransient(child))
     return;
   if (visible)
     wm::GetWindowState(child)->Restore();
@@ -441,6 +453,8 @@ void DockedWindowLayoutManager::OnShelfAlignmentChanged(
 void DockedWindowLayoutManager::OnWindowShowTypeChanged(
     wm::WindowState* window_state,
     wm::WindowShowType old_type) {
+  if (IsPopupOrTransient(window_state->window()))
+    return;
   // The window property will still be set, but no actual change will occur
   // until WillChangeVisibilityState is called when the shelf is visible again
   if (shelf_hidden_)
@@ -465,6 +479,8 @@ void DockedWindowLayoutManager::OnWindowBoundsChanged(
 
 void DockedWindowLayoutManager::OnWindowVisibilityChanging(
     aura::Window* window, bool visible) {
+  if (IsPopupOrTransient(window))
+    return;
   int animation_type = WINDOW_VISIBILITY_ANIMATION_TYPE_MINIMIZE;
   if (visible) {
     animation_type = views::corewm::WINDOW_VISIBILITY_ANIMATION_TYPE_DEFAULT;
@@ -490,6 +506,8 @@ void DockedWindowLayoutManager::OnWindowDestroying(aura::Window* window) {
 
 void DockedWindowLayoutManager::OnWindowActivated(aura::Window* gained_active,
                                                   aura::Window* lost_active) {
+  if (gained_active && IsPopupOrTransient(gained_active))
+    return;
   // Ignore if the window that is not managed by this was activated.
   aura::Window* ancestor = NULL;
   for (aura::Window* parent = gained_active;
@@ -523,7 +541,7 @@ void DockedWindowLayoutManager::WillChangeVisibilityState(
     base::AutoReset<bool> auto_reset_in_layout(&in_layout_, true);
     for (size_t i = 0; i < dock_container_->children().size(); ++i) {
       aura::Window* window = dock_container_->children()[i];
-      if (window->type() == aura::client::WINDOW_TYPE_POPUP)
+      if (IsPopupOrTransient(window))
         continue;
       wm::WindowState* window_state = wm::GetWindowState(window);
       if (shelf_hidden_) {
@@ -559,7 +577,7 @@ void DockedWindowLayoutManager::MaybeMinimizeChildrenExcept(
 
 void DockedWindowLayoutManager::MinimizeDockedWindow(
     wm::WindowState* window_state) {
-  DCHECK_NE(window_state->window()->type(), aura::client::WINDOW_TYPE_POPUP);
+  DCHECK(!IsPopupOrTransient(window_state->window()));
   window_state->window()->Hide();
   if (window_state->IsActive())
     window_state->Deactivate();
@@ -568,7 +586,7 @@ void DockedWindowLayoutManager::MinimizeDockedWindow(
 void DockedWindowLayoutManager::RestoreDockedWindow(
     wm::WindowState* window_state) {
   aura::Window* window = window_state->window();
-  DCHECK_NE(window->type(), aura::client::WINDOW_TYPE_POPUP);
+  DCHECK(!IsPopupOrTransient(window));
   // Always place restored window at the top shuffling the other windows down.
   // TODO(varkha): add a separate container for docked windows to keep track
   // of ordering.
diff --git a/ash/wm/dock/docked_window_resizer.cc b/ash/wm/dock/docked_window_resizer.cc
index c0ca976..8bc27b0 100644
--- a/ash/wm/dock/docked_window_resizer.cc
+++ b/ash/wm/dock/docked_window_resizer.cc
@@ -16,6 +16,7 @@
 #include "ash/wm/coordinate_conversion.h"
 #include "ash/wm/dock/docked_window_layout_manager.h"
 #include "ash/wm/window_state.h"
+#include "ash/wm/window_util.h"
 #include "ash/wm/workspace/magnetism_matcher.h"
 #include "ash/wm/workspace/workspace_window_resizer.h"
 #include "base/command_line.h"
@@ -228,7 +229,7 @@ void DockedWindowResizer::StartedDragging() {
     aura::Window* docked_container = Shell::GetContainer(
         GetTarget()->GetRootWindow(),
         kShellWindowId_DockedContainer);
-    docked_container->AddChild(GetTarget());
+    wm::ReparentChildWithTransientChildren(docked_container, GetTarget());
   }
   if (is_docked_)
     dock_layout_->DockDraggedWindow(GetTarget());
@@ -265,7 +266,7 @@ void DockedWindowResizer::FinishedDragging() {
   if ((is_resized || !attached_panel) &&
       is_docked_ != (window->parent() == dock_container)) {
     if (is_docked_) {
-      dock_container->AddChild(window);
+      wm::ReparentChildWithTransientChildren(dock_container, window);
     } else if (window->parent()->id() == kShellWindowId_DockedContainer) {
       // Reparent the window back to workspace.
       // We need to be careful to give SetDefaultParentByRootWindow location in
@@ -275,8 +276,11 @@ void DockedWindowResizer::FinishedDragging() {
       // mouse is).
       gfx::Rect near_last_location(last_location_, gfx::Size());
       // Reparenting will cause Relayout and possible dock shrinking.
+      aura::Window* previous_parent = window->parent();
       window->SetDefaultParentByRootWindow(window->GetRootWindow(),
                                            near_last_location);
+      if (window->parent() != previous_parent)
+        wm::ReparentTransientChildrenOfChild(window->parent(), window);
     }
   }
   dock_layout_->FinishDragging();
diff --git a/ash/wm/dock/docked_window_resizer_unittest.cc b/ash/wm/dock/docked_window_resizer_unittest.cc
index 1808be1..4fad779 100644
--- a/ash/wm/dock/docked_window_resizer_unittest.cc
+++ b/ash/wm/dock/docked_window_resizer_unittest.cc
@@ -430,8 +430,7 @@ TEST_P(DockedWindowResizerTest, AttachMinimizeRestore) {
 }
 
 // Dock two windows, undock one, check that the other one is still docked.
-TEST_P(DockedWindowResizerTest, AttachTwoWindows)
-{
+TEST_P(DockedWindowResizerTest, AttachTwoWindows) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -470,8 +469,7 @@ TEST_P(DockedWindowResizerTest, AttachTwoWindows)
 
 // Dock one window, try to dock another window on the opposite side (should not
 // dock).
-TEST_P(DockedWindowResizerTest, AttachOnTwoSides)
-{
+TEST_P(DockedWindowResizerTest, AttachOnTwoSides) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -584,8 +582,7 @@ TEST_P(DockedWindowResizerTest, DragAcrossDisplays) {
 
 // Dock two windows, undock one.
 // Test the docked windows area size and default container resizing.
-TEST_P(DockedWindowResizerTest, AttachTwoWindowsDetachOne)
-{
+TEST_P(DockedWindowResizerTest, AttachTwoWindowsDetachOne) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -657,8 +654,7 @@ TEST_P(DockedWindowResizerTest, AttachTwoWindowsDetachOne)
 }
 
 // Dock one of the windows. Maximize other testing desktop resizing.
-TEST_P(DockedWindowResizerTest, AttachWindowMaximizeOther)
-{
+TEST_P(DockedWindowResizerTest, AttachWindowMaximizeOther) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -757,8 +753,7 @@ TEST_P(DockedWindowResizerTest, AttachWindowMaximizeOther)
 }
 
 // Dock one window. Test the sticky behavior near screen or desktop edge.
-TEST_P(DockedWindowResizerTest, AttachOneTestSticky)
-{
+TEST_P(DockedWindowResizerTest, AttachOneTestSticky) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -855,8 +850,7 @@ TEST_P(DockedWindowResizerTest, AttachOneTestSticky)
 
 // Dock two windows, resize one or both.
 // Test the docked windows area size and remaining desktop resizing.
-TEST_P(DockedWindowResizerTest, ResizeTwoWindows)
-{
+TEST_P(DockedWindowResizerTest, ResizeTwoWindows) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -996,8 +990,9 @@ TEST_P(DockedWindowResizerTest, ResizeTwoWindows)
             ScreenAsh::GetDisplayWorkAreaBoundsInParent(w2.get()).width());
 }
 
-TEST_P(DockedWindowResizerTest, DragToShelf)
-{
+// Tests that dragging a window down to shelf attaches a panel but does not
+// attach a regular window.
+TEST_P(DockedWindowResizerTest, DragToShelf) {
   if (!SupportsHostWindowResize())
     return;
 
@@ -1042,6 +1037,47 @@ TEST_P(DockedWindowResizerTest, DragToShelf)
   }
 }
 
+// Tests that docking and undocking a |window| with a transient child properly
+// maintains the parent of that transient child to be the same as the |window|.
+TEST_P(DockedWindowResizerTest, DragWindowWithTransientChild) {
+  if (!SupportsHostWindowResize())
+    return;
+
+  // Create a window with a transient child.
+  scoped_ptr<aura::Window> window(CreateTestWindow(gfx::Rect(0, 0, 201, 201)));
+  scoped_ptr<aura::Window> child(CreateTestWindowInShellWithDelegateAndType(
+      NULL, aura::client::WINDOW_TYPE_NORMAL, 0, gfx::Rect(20, 20, 150, 20)));
+  window->AddTransientChild(child.get());
+  if (window->parent() != child->parent())
+    window->parent()->AddChild(child.get());
+  EXPECT_EQ(window.get(), child->transient_parent());
+
+  DragToVerticalPositionAndToEdge(DOCKED_EDGE_RIGHT, window.get(), 20);
+
+  // A window should be attached and snapped to the right edge.
+  EXPECT_EQ(internal::kShellWindowId_DockedContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DockedContainer, child->parent()->id());
+
+  // Drag the child - it should move freely and stay where it is dragged.
+  ASSERT_NO_FATAL_FAILURE(DragStart(child.get()));
+  DragMove(500, 20);
+  DragEnd();
+  EXPECT_EQ(gfx::Point(20 + 500, 20 + 20).ToString(),
+            child->GetBoundsInScreen().origin().ToString());
+
+  // Undock the window by dragging left.
+  ASSERT_NO_FATAL_FAILURE(DragStart(window.get()));
+  DragMove(-32, -10);
+  DragEnd();
+
+  // The window should be undocked and the transient child should be reparented.
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, child->parent()->id());
+  // The child should not have moved.
+  EXPECT_EQ(gfx::Point(20 + 500, 20 + 20).ToString(),
+            child->GetBoundsInScreen().origin().ToString());
+}
+
 // Tests run twice - on both panels and normal windows
 INSTANTIATE_TEST_CASE_P(NormalOrPanel,
                         DockedWindowResizerTest,
diff --git a/ash/wm/panels/panel_layout_manager.cc b/ash/wm/panels/panel_layout_manager.cc
index a41b100..fc654fe 100644
--- a/ash/wm/panels/panel_layout_manager.cc
+++ b/ash/wm/panels/panel_layout_manager.cc
@@ -358,6 +358,7 @@ void PanelLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
     child->SetDefaultParentByRootWindow(
         child->GetRootWindow(),
         child->GetRootWindow()->GetBoundsInScreen());
+    wm::ReparentTransientChildrenOfChild(child->parent(), child);
     DCHECK(child->parent()->id() != kShellWindowId_PanelContainer);
     return;
   }
diff --git a/ash/wm/panels/panel_window_resizer.cc b/ash/wm/panels/panel_window_resizer.cc
index dcd4559..03a69f0 100644
--- a/ash/wm/panels/panel_window_resizer.cc
+++ b/ash/wm/panels/panel_window_resizer.cc
@@ -14,6 +14,7 @@
 #include "ash/wm/coordinate_conversion.h"
 #include "ash/wm/panels/panel_layout_manager.h"
 #include "ash/wm/window_state.h"
+#include "ash/wm/window_util.h"
 #include "base/memory/weak_ptr.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
@@ -197,6 +198,7 @@ void PanelWindowResizer::StartedDragging() {
     GetTarget()->SetDefaultParentByRootWindow(
         GetTarget()->GetRootWindow(),
         GetTarget()->GetRootWindow()->GetBoundsInScreen());
+    wm::ReparentTransientChildrenOfChild(GetTarget()->parent(), GetTarget());
   }
 }
 
@@ -210,6 +212,7 @@ void PanelWindowResizer::FinishDragging() {
     GetTarget()->SetDefaultParentByRootWindow(
         GetTarget()->GetRootWindow(),
         gfx::Rect(last_location_, gfx::Size()));
+    wm::ReparentTransientChildrenOfChild(GetTarget()->parent(), GetTarget());
   }
 
   // If we started the drag in one root window and moved into another root
diff --git a/ash/wm/panels/panel_window_resizer_unittest.cc b/ash/wm/panels/panel_window_resizer_unittest.cc
index 2c8cb47..4a332a7 100644
--- a/ash/wm/panels/panel_window_resizer_unittest.cc
+++ b/ash/wm/panels/panel_window_resizer_unittest.cc
@@ -219,6 +219,22 @@ class PanelWindowResizerTextDirectionTest
   DISALLOW_COPY_AND_ASSIGN(PanelWindowResizerTextDirectionTest);
 };
 
+// PanelLayoutManager and PanelWindowResizer should work if panels have
+// transient children of supported types.
+class PanelWindowResizerTransientTest
+    : public PanelWindowResizerTest,
+      public testing::WithParamInterface<aura::client::WindowType> {
+ public:
+  PanelWindowResizerTransientTest() : transient_window_type_(GetParam()) {}
+  virtual ~PanelWindowResizerTransientTest() {}
+
+ protected:
+  aura::client::WindowType transient_window_type_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PanelWindowResizerTransientTest);
+};
+
 // Verifies a window can be dragged from the panel and detached and then
 // reattached.
 TEST_F(PanelWindowResizerTest, PanelDetachReattachBottom) {
@@ -293,8 +309,7 @@ TEST_F(PanelWindowResizerTest, DetachThenDragAcrossDisplays) {
   EXPECT_EQ(initial_bounds.x(), window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
   EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 
   DragStart(window.get());
   DragMove(500, 0);
@@ -303,8 +318,7 @@ TEST_F(PanelWindowResizerTest, DetachThenDragAcrossDisplays) {
   EXPECT_EQ(initial_bounds.x() + 500, window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
   EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 }
 
 TEST_F(PanelWindowResizerTest, DetachAcrossDisplays) {
@@ -324,8 +338,7 @@ TEST_F(PanelWindowResizerTest, DetachAcrossDisplays) {
   EXPECT_EQ(initial_bounds.x() + 500, window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
   EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 }
 
 TEST_F(PanelWindowResizerTest, DetachThenAttachToSecondDisplay) {
@@ -403,8 +416,7 @@ TEST_F(PanelWindowResizerTest, RevertDragRestoresAttachment) {
   DragMove(0, -100);
   DragEnd();
   EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 
   // Drag back to launcher.
   DragStart(window.get());
@@ -413,8 +425,7 @@ TEST_F(PanelWindowResizerTest, RevertDragRestoresAttachment) {
   // When the drag is reverted it should remain detached.
   DragRevert();
   EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 }
 
 TEST_F(PanelWindowResizerTest, DragMovesToPanelLayer) {
@@ -422,8 +433,7 @@ TEST_F(PanelWindowResizerTest, DragMovesToPanelLayer) {
   DragStart(window.get());
   DragMove(0, -100);
   DragEnd();
-  EXPECT_EQ(internal::kShellWindowId_DefaultContainer,
-            window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
 
   // While moving the panel window should be moved to the panel container.
   DragStart(window.get());
@@ -452,8 +462,71 @@ TEST_F(PanelWindowResizerTest, DragReordersPanelsVertical) {
   DragAlongShelfReorder(0, -1);
 }
 
+// Tests that panels can have transient children of different types.
+// The transient children should be reparented in sync with the panel.
+TEST_P(PanelWindowResizerTransientTest, PanelWithTransientChild) {
+  scoped_ptr<aura::Window> window(CreatePanelWindow(gfx::Point(0, 0)));
+  scoped_ptr<aura::Window> child(CreateTestWindowInShellWithDelegateAndType(
+      NULL, transient_window_type_, 0, gfx::Rect(20, 20, 150, 40)));
+  window->AddTransientChild(child.get());
+  if (window->parent() != child->parent())
+    window->parent()->AddChild(child.get());
+  EXPECT_EQ(window.get(), child->transient_parent());
+
+  // Drag the child to the shelf. Its new position should not be overridden.
+  const gfx::Rect attached_bounds(window->GetBoundsInScreen());
+  const int dy = window->GetBoundsInScreen().bottom() -
+      child->GetBoundsInScreen().bottom();
+  DragStart(child.get());
+  DragMove(50, dy);
+  // While moving the transient child window should be in the panel container.
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, child->parent()->id());
+  DragEnd();
+  // Child should move, |window| should not.
+  EXPECT_EQ(gfx::Point(20 + 50, 20 + dy).ToString(),
+            child->GetBoundsInScreen().origin().ToString());
+  EXPECT_EQ(attached_bounds.ToString(), window->GetBoundsInScreen().ToString());
+
+  // Drag the child along the the shelf past the |window|.
+  // Its new position should not be overridden.
+  DragStart(child.get());
+  DragMove(350, 0);
+  // While moving the transient child window should be in the panel container.
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, child->parent()->id());
+  DragEnd();
+  // |child| should move, |window| should not.
+  EXPECT_EQ(gfx::Point(20 + 50 + 350, 20 + dy).ToString(),
+            child->GetBoundsInScreen().origin().ToString());
+  EXPECT_EQ(attached_bounds.ToString(), window->GetBoundsInScreen().ToString());
+
+  DragStart(window.get());
+  DragMove(0, -100);
+  // While moving the windows should be in the panel container.
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, child->parent()->id());
+  DragEnd();
+  // When dropped they should return to the default container.
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, child->parent()->id());
+
+  // While moving the window and child should be moved to the panel container.
+  DragStart(window.get());
+  DragMove(20, 0);
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_PanelContainer, child->parent()->id());
+  DragEnd();
+
+  // When dropped they should return to the default container.
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, window->parent()->id());
+  EXPECT_EQ(internal::kShellWindowId_DefaultContainer, child->parent()->id());
+}
+
 INSTANTIATE_TEST_CASE_P(LtrRtl, PanelWindowResizerTextDirectionTest,
                         testing::Bool());
+INSTANTIATE_TEST_CASE_P(NormalPanelPopup, PanelWindowResizerTransientTest,
+                        testing::Values(aura::client::WINDOW_TYPE_NORMAL,
+                                        aura::client::WINDOW_TYPE_PANEL,
+                                        aura::client::WINDOW_TYPE_POPUP));
 
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/wm/window_state.cc b/ash/wm/window_state.cc
index e72b46c..090792b 100644
--- a/ash/wm/window_state.cc
+++ b/ash/wm/window_state.cc
@@ -98,9 +98,9 @@ bool WindowState::CanActivate() const {
 }
 
 bool WindowState::CanSnap() const {
-  if (!CanResize())
-    return false;
-  if (window_->type() == aura::client::WINDOW_TYPE_PANEL)
+  if (!CanResize() ||
+      window_->type() == aura::client::WINDOW_TYPE_PANEL ||
+      window_->transient_parent())
     return false;
   // If a window has a maximum size defined, snapping may make it too big.
   return window_->delegate() ? window_->delegate()->GetMaximumSize().IsEmpty() :
diff --git a/ash/wm/window_util.cc b/ash/wm/window_util.cc
index 6d85c83..48947b8 100644
--- a/ash/wm/window_util.cc
+++ b/ash/wm/window_util.cc
@@ -109,5 +109,17 @@ bool MoveWindowToEventRoot(aura::Window* window, const ui::Event& event) {
   return true;
 }
 
+void ReparentChildWithTransientChildren(aura::Window* window,
+                                        aura::Window* child) {
+  window->AddChild(child);
+  ReparentTransientChildrenOfChild(window, child);
+}
+
+void ReparentTransientChildrenOfChild(aura::Window* window,
+                                      aura::Window* child) {
+  for (size_t i = 0; i < child->transient_children().size(); ++i)
+    ReparentChildWithTransientChildren(window, child->transient_children()[i]);
+}
+
 }  // namespace wm
 }  // namespace ash
diff --git a/ash/wm/window_util.h b/ash/wm/window_util.h
index 7c8ffe9..b33df78 100644
--- a/ash/wm/window_util.h
+++ b/ash/wm/window_util.h
@@ -69,6 +69,14 @@ ASH_EXPORT void AdjustBoundsToEnsureWindowVisibility(
 ASH_EXPORT bool MoveWindowToEventRoot(aura::Window* window,
                                       const ui::Event& event);
 
+// Adds |child| and all its transient children to |window|.
+void ReparentChildWithTransientChildren(aura::Window* window,
+                                        aura::Window* child);
+
+// Changes the parent of all transient children of a |child| to |window|.
+void ReparentTransientChildrenOfChild(aura::Window* window,
+                                       aura::Window* child);
+
 }  // namespace wm
 }  // namespace ash
 
diff --git a/ash/wm/workspace/workspace_window_resizer.cc b/ash/wm/workspace/workspace_window_resizer.cc
index ca7ca3f..f591c97 100644
--- a/ash/wm/workspace/workspace_window_resizer.cc
+++ b/ash/wm/workspace/workspace_window_resizer.cc
@@ -95,6 +95,7 @@ scoped_ptr<WindowResizer> CreateWindowResizer(
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kAshEnableDockedWindows) &&
       window_resizer && window->parent() &&
+      !window->transient_parent() &&
       (window->parent()->id() == internal::kShellWindowId_DefaultContainer ||
        window->parent()->id() == internal::kShellWindowId_DockedContainer ||
        window->parent()->id() == internal::kShellWindowId_PanelContainer)) {
@@ -901,9 +902,17 @@ void WorkspaceWindowResizer::UpdateSnapPhantomWindow(const gfx::Point& location,
       return;
     }
   }
+  const bool can_dock = dock_layout_->CanDockWindow(window(), snap_type_);
+  const bool can_snap = window_state()->CanSnap();
+  if (!can_snap && !can_dock) {
+    snap_type_ = SNAP_NONE;
+    snap_phantom_window_controller_.reset();
+    snap_sizer_.reset();
+    SetDraggedWindowDocked(false);
+    return;
+  }
   SnapSizer::Edge edge = (snap_type_ == SNAP_LEFT) ?
       SnapSizer::LEFT_EDGE : SnapSizer::RIGHT_EDGE;
-
   if (!snap_sizer_) {
     snap_sizer_.reset(new SnapSizer(window_state(),
                                     location,
@@ -913,11 +922,6 @@ void WorkspaceWindowResizer::UpdateSnapPhantomWindow(const gfx::Point& location,
     snap_sizer_->Update(location);
   }
 
-  const bool can_dock = dock_layout_->CanDockWindow(window(), snap_type_);
-  const bool can_snap = window_state()->CanSnap();
-  if (!can_snap && !can_dock)
-    return;
-
   // Update phantom window with snapped or docked guide bounds.
   // Windows that cannot be snapped or are less wide than kMaxDockWidth can get
   // docked without going through a snapping sequence.
diff --git a/ui/aura/window.cc b/ui/aura/window.cc
index 20ce9b5..c0c5744 100644
--- a/ui/aura/window.cc
+++ b/ui/aura/window.cc
@@ -817,6 +817,46 @@ void Window::OnParentChanged() {
       WindowObserver, observers_, OnWindowParentChanged(this, parent_));
 }
 
+bool Window::GetAllTransientAncestors(Window* window,
+                                      Windows* ancestors) const {
+  for (; window; window = window->transient_parent()) {
+    if (window->parent() == this)
+      ancestors->push_back(window);
+  }
+  return (!ancestors->empty());
+}
+
+void Window::FindCommonSiblings(Window** window1, Window** window2) const {
+  DCHECK(window1);
+  DCHECK(window2);
+  DCHECK(*window1);
+  DCHECK(*window2);
+  // Assemble chains of ancestors of both windows.
+  Windows ancestors1;
+  Windows ancestors2;
+  if (!GetAllTransientAncestors(*window1, &ancestors1) ||
+      !GetAllTransientAncestors(*window2, &ancestors2)) {
+    return;
+  }
+  // Walk the two chains backwards and look for the first difference.
+  Windows::const_reverse_iterator it1 = ancestors1.rbegin();
+  Windows::const_reverse_iterator it2 = ancestors2.rbegin();
+  for (; it1  != ancestors1.rend() && it2  != ancestors2.rend(); ++it1, ++it2) {
+    if (*it1 != *it2) {
+      *window1 = *it1;
+      *window2 = *it2;
+      break;
+    }
+  }
+}
+
+bool Window::HasTransientAncestor(const Window* ancestor) const {
+  if (transient_parent_ == ancestor)
+    return true;
+  return transient_parent_ ?
+      transient_parent_->HasTransientAncestor(ancestor) : false;
+}
+
 void Window::StackChildRelativeTo(Window* child,
                                   Window* target,
                                   StackDirection direction) {
@@ -826,14 +866,28 @@ void Window::StackChildRelativeTo(Window* child,
   DCHECK_EQ(this, child->parent());
   DCHECK_EQ(this, target->parent());
 
+  // Consider all transient children of both child's and target's ancestors
+  // up to the common ancestor if such exists and stack them as a unit.
+  // This prevents one transient group from being inserted in the middle of
+  // another.
+  FindCommonSiblings(&child, &target);
+
   const size_t target_i =
       std::find(children_.begin(), children_.end(), target) - children_.begin();
 
+  // When stacking above skip to the topmost transient descendant of the target.
+  size_t final_target_i = target_i;
+  if (direction == STACK_ABOVE && !child->HasTransientAncestor(target)) {
+    while (final_target_i + 1 < children_.size() &&
+           children_[final_target_i + 1]->HasTransientAncestor(target)) {
+      ++final_target_i;
+    }
+  }
+
   // By convention we don't stack on top of windows with layers with NULL
   // delegates.  Walk backward to find a valid target window.
   // See tests WindowTest.StackingMadrigal and StackOverClosingTransient
   // for an explanation of this.
-  size_t final_target_i = target_i;
   while (final_target_i > 0 &&
          children_[final_target_i]->layer()->delegate() == NULL) {
     --final_target_i;
@@ -853,8 +907,22 @@ void Window::StackChildRelativeTo(Window* child,
   if (child == final_target)
     return;
 
-  // Move the child and all its transients.
+  // Move the child.
   StackChildRelativeToImpl(child, final_target, direction);
+
+  // Stack any transient children that share the same parent to be in front of
+  // 'child'. Preserve the existing stacking order by iterating in the order
+  // those children appear in children_ array.
+  Window* last_transient = child;
+  Windows children(children_);
+  for (Windows::iterator it = children.begin(); it != children.end(); ++it) {
+    Window* transient_child = *it;
+    if (transient_child != last_transient &&
+        transient_child->HasTransientAncestor(child)) {
+      StackChildRelativeToImpl(transient_child, last_transient, STACK_ABOVE);
+      last_transient = transient_child;
+    }
+  }
 }
 
 void Window::StackChildRelativeToImpl(Window* child,
@@ -888,18 +956,6 @@ void Window::StackChildRelativeToImpl(Window* child,
   else
     layer()->StackBelow(child->layer(), target->layer());
 
-  // Stack any transient children that share the same parent to be in front of
-  // 'child'.
-  Window* last_transient = child;
-  for (Windows::iterator it = child->transient_children_.begin();
-       it != child->transient_children_.end(); ++it) {
-    Window* transient_child = *it;
-    if (transient_child->parent_ == this) {
-      StackChildRelativeToImpl(transient_child, last_transient, STACK_ABOVE);
-      last_transient = transient_child;
-    }
-  }
-
   child->OnStackingChanged();
 }
 
diff --git a/ui/aura/window.h b/ui/aura/window.h
index 63f9ddd..2d182b9 100644
--- a/ui/aura/window.h
+++ b/ui/aura/window.h
@@ -397,6 +397,18 @@ class AURA_EXPORT Window : public ui::LayerDelegate,
   // Called when this window's parent has changed.
   void OnParentChanged();
 
+  // Populates |ancestors| with all transient ancestors of |window| that are
+  // children of |this|. Returns true if any ancestors were found, false if not.
+  bool GetAllTransientAncestors(Window* window, Windows* ancestors) const;
+
+  // Replaces two windows |window1| and |window2| with their possible transient
+  // ancestors that are still siblings (have a common transient parent).
+  // |window1| and |window2| are not modified if such ancestors cannot be found.
+  void FindCommonSiblings(Window** window1, Window** window2) const;
+
+  // Returns true when |ancestor| is a transient ancestor of |this|.
+  bool HasTransientAncestor(const Window* ancestor) const;
+
   // Determines the real location for stacking |child| and invokes
   // StackChildRelativeToImpl().
   void StackChildRelativeTo(Window* child,
diff --git a/ui/aura/window_unittest.cc b/ui/aura/window_unittest.cc
index 3c93b04..2c44d3c 100644
--- a/ui/aura/window_unittest.cc
+++ b/ui/aura/window_unittest.cc
@@ -1645,6 +1645,143 @@ TEST_F(WindowTest, TransientChildren) {
   EXPECT_FALSE(w2->IsVisible());
 }
 
+// Tests that transient children are stacked as a unit when using stack above.
+TEST_F(WindowTest, TransientChildrenGroupAbove) {
+  scoped_ptr<Window> parent(CreateTestWindowWithId(0, root_window()));
+  scoped_ptr<Window> w1(CreateTestWindowWithId(1, parent.get()));
+  Window* w11 = CreateTestWindowWithId(11, parent.get());
+  scoped_ptr<Window> w2(CreateTestWindowWithId(2, parent.get()));
+  Window* w21 = CreateTestWindowWithId(21, parent.get());
+  Window* w211 = CreateTestWindowWithId(211, parent.get());
+  Window* w212 = CreateTestWindowWithId(212, parent.get());
+  Window* w213 = CreateTestWindowWithId(213, parent.get());
+  Window* w22 = CreateTestWindowWithId(22, parent.get());
+  ASSERT_EQ(8u, parent->children().size());
+
+  w1->AddTransientChild(w11);  // w11 is now owned by w1.
+  w2->AddTransientChild(w21);  // w21 is now owned by w2.
+  w2->AddTransientChild(w22);  // w22 is now owned by w2.
+  w21->AddTransientChild(w211);  // w211 is now owned by w21.
+  w21->AddTransientChild(w212);  // w212 is now owned by w21.
+  w21->AddTransientChild(w213);  // w213 is now owned by w21.
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  // Stack w1 at the top (end), this should force w11 to be last (on top of w1).
+  parent->StackChildAtTop(w1.get());
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 21 211 212 213 22 1 11", ChildWindowIDsAsString(parent.get()));
+
+  // This tests that the order in children_ array rather than in
+  // transient_children_ array is used when reinserting transient children.
+  // If transient_children_ array was used '22' would be following '21'.
+  parent->StackChildAtTop(w2.get());
+  EXPECT_EQ(w22, parent->children().back());
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w11, w2.get());
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 21 211 212 213 22 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w21, w1.get());
+  EXPECT_EQ(w22, parent->children().back());
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w21, w22);
+  EXPECT_EQ(w213, parent->children().back());
+  EXPECT_EQ("1 11 2 22 21 211 212 213", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w11, w21);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 211 212 213 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w213, w21);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+
+  // No change when stacking a transient parent above its transient child.
+  parent->StackChildAbove(w21, w211);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+
+  // This tests that the order in children_ array rather than in
+  // transient_children_ array is used when reinserting transient children.
+  // If transient_children_ array was used '22' would be following '21'.
+  parent->StackChildAbove(w2.get(), w1.get());
+  EXPECT_EQ(w212, parent->children().back());
+  EXPECT_EQ("1 11 2 22 21 213 211 212", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAbove(w11, w213);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+}
+
+// Tests that transient children are stacked as a unit when using stack below.
+TEST_F(WindowTest, TransientChildrenGroupBelow) {
+  scoped_ptr<Window> parent(CreateTestWindowWithId(0, root_window()));
+  scoped_ptr<Window> w1(CreateTestWindowWithId(1, parent.get()));
+  Window* w11 = CreateTestWindowWithId(11, parent.get());
+  scoped_ptr<Window> w2(CreateTestWindowWithId(2, parent.get()));
+  Window* w21 = CreateTestWindowWithId(21, parent.get());
+  Window* w211 = CreateTestWindowWithId(211, parent.get());
+  Window* w212 = CreateTestWindowWithId(212, parent.get());
+  Window* w213 = CreateTestWindowWithId(213, parent.get());
+  Window* w22 = CreateTestWindowWithId(22, parent.get());
+  ASSERT_EQ(8u, parent->children().size());
+
+  w1->AddTransientChild(w11);  // w11 is now owned by w1.
+  w2->AddTransientChild(w21);  // w21 is now owned by w2.
+  w2->AddTransientChild(w22);  // w22 is now owned by w2.
+  w21->AddTransientChild(w211);  // w211 is now owned by w21.
+  w21->AddTransientChild(w212);  // w212 is now owned by w21.
+  w21->AddTransientChild(w213);  // w213 is now owned by w21.
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  // Stack w2 at the bottom, this should force w11 to be last (on top of w1).
+  // This also tests that the order in children_ array rather than in
+  // transient_children_ array is used when reinserting transient children.
+  // If transient_children_ array was used '22' would be following '21'.
+  parent->StackChildAtBottom(w2.get());
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 21 211 212 213 22 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildAtBottom(w1.get());
+  EXPECT_EQ(w22, parent->children().back());
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w21, w1.get());
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 21 211 212 213 22 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w11, w2.get());
+  EXPECT_EQ(w22, parent->children().back());
+  EXPECT_EQ("1 11 2 21 211 212 213 22", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w22, w21);
+  EXPECT_EQ(w213, parent->children().back());
+  EXPECT_EQ("1 11 2 22 21 211 212 213", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w21, w11);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 211 212 213 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w213, w211);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+
+  // No change when stacking a transient parent below its transient child.
+  parent->StackChildBelow(w21, w211);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w1.get(), w2.get());
+  EXPECT_EQ(w212, parent->children().back());
+  EXPECT_EQ("1 11 2 22 21 213 211 212", ChildWindowIDsAsString(parent.get()));
+
+  parent->StackChildBelow(w213, w11);
+  EXPECT_EQ(w11, parent->children().back());
+  EXPECT_EQ("2 22 21 213 211 212 1 11", ChildWindowIDsAsString(parent.get()));
+}
+
 // Tests that when a focused window is closed, its parent inherits the focus.
 TEST_F(WindowTest, FocusedWindowTest) {
   scoped_ptr<Window> parent(CreateTestWindowWithId(0, root_window()));
@@ -2446,9 +2583,11 @@ TEST_F(WindowTest, StackOverClosingTransient) {
   EXPECT_EQ(root->layer()->children()[1], transient1->layer());
   EXPECT_EQ(root->layer()->children()[2], window2->layer());
   EXPECT_EQ(root->layer()->children()[3], transient2->layer());
+  EXPECT_EQ("1 11 2 21", ChildWindowIDsAsString(root_window()));
 
   // This brings window1 and its transient to the front.
   root->StackChildAtTop(window1.get());
+  EXPECT_EQ("2 21 1 11", ChildWindowIDsAsString(root_window()));
 
   EXPECT_EQ(root->children()[0], window2.get());
   EXPECT_EQ(root->children()[1], transient2.get());