From 606fcb422c56e6678dc1ad44818d34911414fbd0 Mon Sep 17 00:00:00 2001
From: Qiang Xu <warx@chromium.org>
Date: Sat, 6 Jan 2018 10:37:34 +0800
Subject: [PATCH] cros: move first-non-transient transient-parent if transient

changes:
Transient child window could get activated. In this case, shortcut to
move active window between displays will move its first-non-transient
transient-parent window.

move browser window that has permission prompt shown and activated.

Bug: 778438
Test: added test coverage. Also tested on device operating shortcuts to
Change-Id: I5d24c2c8ecfc17c30b97fe16db025307c5b23810
Reviewed-on: https://chromium-review.googlesource.com/848696
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527498}
---
 ash/display/display_move_window_util.cc       | 21 +++++++---
 .../display_move_window_util_unittest.cc      | 40 +++++++++++++++++++
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/ash/display/display_move_window_util.cc b/ash/display/display_move_window_util.cc
index 5440da6409e21c..ac6bf0cbbba4b0 100644
--- a/ash/display/display_move_window_util.cc
+++ b/ash/display/display_move_window_util.cc
@@ -15,6 +15,7 @@
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/display/types/display_constants.h"
+#include "ui/wm/core/window_util.h"
 
 namespace ash {
 
@@ -142,12 +143,20 @@ void HandleMoveActiveWindowToDisplay(DisplayMoveWindowDirection direction) {
   if (!window)
     return;
 
-  // When |window_list| is not empty, |window| can only be the first one of the
-  // fresh built list if it is in the list.
-  MruWindowTracker::WindowList window_list =
-      Shell::Get()->mru_window_tracker()->BuildWindowForCycleList();
-  if (window_list.empty() || window_list.front() != window)
-    return;
+  // If |window| is transient window, move its first non-transient
+  // transient-parent window instead. Otherwise, it should be the first one in
+  // window cycle list.
+  if (::wm::GetTransientParent(window)) {
+    while (::wm::GetTransientParent(window))
+      window = ::wm::GetTransientParent(window);
+    if (window == window->GetRootWindow())
+      return;
+  } else {
+    MruWindowTracker::WindowList window_list =
+        Shell::Get()->mru_window_tracker()->BuildWindowForCycleList();
+    if (window_list.empty() || window_list.front() != window)
+      return;
+  }
 
   display::Display origin_display =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window);
diff --git a/ash/display/display_move_window_util_unittest.cc b/ash/display/display_move_window_util_unittest.cc
index 09e1773b9a3dcb..880ce8c699f97d 100644
--- a/ash/display/display_move_window_util_unittest.cc
+++ b/ash/display/display_move_window_util_unittest.cc
@@ -22,6 +22,7 @@
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/screen.h"
 #include "ui/display/test/display_manager_test_api.h"
+#include "ui/views/widget/widget.h"
 #include "ui/wm/core/window_util.h"
 
 namespace ash {
@@ -440,4 +441,43 @@ TEST_F(DisplayMoveWindowUtilTest, WindowWithTransientChild) {
   EXPECT_EQ(gfx::Rect(430, 50, 40, 50), child->GetBoundsInScreen());
 }
 
+// Test that when operating move window between displays on activated transient
+// child window, its first non-transient transient-parent window should be the
+// target instead.
+TEST_F(DisplayMoveWindowUtilTest, ActiveTransientChildWindow) {
+  UpdateDisplay("400x300,400x300");
+  std::unique_ptr<views::Widget> window = CreateTestWidget();
+  window->SetBounds(gfx::Rect(10, 20, 200, 100));
+
+  // Create a |child| transient widget of |window|. When |child| is shown, it is
+  // activated.
+  std::unique_ptr<views::Widget> child(new views::Widget);
+  views::Widget::InitParams params(views::Widget::InitParams::TYPE_WINDOW);
+  params.delegate = nullptr;
+  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.bounds = gfx::Rect(20, 30, 40, 50);
+  params.parent = window->GetNativeWindow();
+  child->Init(params);
+  child->Show();
+  display::Screen* screen = display::Screen::GetScreen();
+  EXPECT_EQ(display_manager()->GetDisplayAt(0).id(),
+            screen->GetDisplayNearestWindow(window->GetNativeWindow()).id());
+  EXPECT_EQ(display_manager()->GetDisplayAt(0).id(),
+            screen->GetDisplayNearestWindow(child->GetNativeWindow()).id());
+  // Ensure |child| window is activated.
+  EXPECT_FALSE(wm::IsActiveWindow(window->GetNativeWindow()));
+  EXPECT_TRUE(wm::IsActiveWindow(child->GetNativeWindow()));
+
+  // Operate moving window to right display. Check display and bounds.
+  HandleMoveActiveWindowToDisplay(DisplayMoveWindowDirection::kRight);
+  EXPECT_EQ(display_manager()->GetDisplayAt(1).id(),
+            screen->GetDisplayNearestWindow(window->GetNativeWindow()).id());
+  EXPECT_EQ(display_manager()->GetDisplayAt(1).id(),
+            screen->GetDisplayNearestWindow(child->GetNativeWindow()).id());
+  EXPECT_EQ(gfx::Rect(410, 20, 200, 100),
+            window->GetNativeWindow()->GetBoundsInScreen());
+  EXPECT_EQ(gfx::Rect(420, 30, 40, 50),
+            child->GetNativeWindow()->GetBoundsInScreen());
+}
+
 }  // namespace ash