Use BuildWindowListIgnoreModal for orientaiton controller

Changed BuildWindowListIgnoreModal to keep the activation order.
This also fixes the method not to add a window w/o parent.

Bug: None
Test: covered by unittests.
Change-Id: I727b3af0421b9f0f61552ec4a25e440e30e903d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935287
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720767}

ash/display/screen_orientation_controller.cc

@@ -643,7 +643,8 @@ void ScreenOrientationController::ApplyLockForActiveWindow() {
   }
 
   MruWindowTracker::WindowList mru_windows(
-      Shell::Get()->mru_window_tracker()->BuildMruWindowList(kActiveDesk));
+      Shell::Get()->mru_window_tracker()->BuildWindowListIgnoreModal(
+          kActiveDesk));
 
   for (auto* window : mru_windows) {
     if (!window->TargetVisibility())

ash/display/screen_orientation_controller_unittest.cc

@@ -176,6 +176,11 @@ TEST_F(ScreenOrientationControllerTest, LockOrientation) {
   Lock(child_window.get(), OrientationLockType::kLandscape);
   EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
   EXPECT_TRUE(RotationLocked());
+
+  auto modal = CreateTestWindow(gfx::Rect(0, 0, 400, 400));
+  modal->SetProperty(aura::client::kModalKey, ui::MODAL_TYPE_SYSTEM);
+  EXPECT_EQ(display::Display::ROTATE_0, GetCurrentInternalDisplayRotation());
+  EXPECT_TRUE(RotationLocked());
 }
 
 // Tests that a Window can unlock rotation.

ash/wm/mru_window_tracker.cc

@@ -17,6 +17,7 @@
 #include "ash/wm/window_util.h"
 #include "base/containers/adapters.h"
 #include "base/stl_util.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/aura/window.h"
 #include "ui/wm/core/window_util.h"
 #include "ui/wm/public/activation_client.h"
@@ -49,11 +50,17 @@ class ScopedWindowClosingObserver : public aura::WindowObserver {
   DISALLOW_COPY_AND_ASSIGN(ScopedWindowClosingObserver);
 };
 
-bool IsWindowConsideredActivatable(aura::Window* window) {
+bool IsNonSysModalWindowConsideredActivatable(aura::Window* window) {
   DCHECK(window);
   ScopedWindowClosingObserver observer(window);
   AshFocusRules* focus_rules = Shell::Get()->focus_rules();
 
+  // Exclude system modal because we only care about non systm modal windows.
+  if (window->GetProperty(aura::client::kModalKey) ==
+      static_cast<int>(ui::MODAL_TYPE_SYSTEM)) {
+    return false;
+  }
+
   // Only toplevel windows can be activated.
   if (!focus_rules->IsToplevelWindow(window))
     return false;
@@ -123,9 +130,9 @@ MruWindowTracker::WindowList BuildWindowListInternal(
 
         if (!can_include_window_predicate(window))
           continue;
-      }
 
-      windows.emplace_back(window);
+        windows.emplace_back(window);
+      }
     }
   }
 
@@ -201,8 +208,8 @@ MruWindowTracker::WindowList MruWindowTracker::BuildMruWindowList(
 
 MruWindowTracker::WindowList MruWindowTracker::BuildWindowListIgnoreModal(
     DesksMruType desks_mru_type) const {
-  return BuildWindowListInternal(nullptr, desks_mru_type,
-                                 IsWindowConsideredActivatable);
+  return BuildWindowListInternal(&mru_windows_, desks_mru_type,
+                                 IsNonSysModalWindowConsideredActivatable);
 }
 
 MruWindowTracker::WindowList MruWindowTracker::BuildWindowForCycleList(

ash/wm/mru_window_tracker_unittest.cc

@@ -9,7 +9,10 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/window_state.h"
 #include "ash/wm/window_util.h"
+#include "ui/aura/client/aura_constants.h"
 #include "ui/base/hit_test.h"
+#include "ui/base/ui_base_types.h"
+#include "ui/views/widget/widget_delegate.h"
 
 namespace ash {
 
@@ -47,14 +50,65 @@ TEST_F(MruWindowTrackerTest, Basic) {
   EXPECT_EQ(w3.get(), window_list[2]);
 }
 
+// Tests that windows being dragged are only in the WindowList once.
+TEST_F(MruWindowTrackerTest, DraggedWindowsInListOnlyOnce) {
+  std::unique_ptr<aura::Window> w1(CreateTestWindow());
+  wm::ActivateWindow(w1.get());
+
+  // Start dragging the window.
+  WindowState::Get(w1.get())->CreateDragDetails(gfx::Point(), HTRIGHT,
+                                                ::wm::WINDOW_MOVE_SOURCE_TOUCH);
+
+  // The dragged window should only be in the list once.
+  MruWindowTracker::WindowList window_list =
+      mru_window_tracker()->BuildWindowListIgnoreModal(kActiveDesk);
+  EXPECT_EQ(1, std::count(window_list.begin(), window_list.end(), w1.get()));
+}
+
+class MruWindowTrackerOrderTest : public MruWindowTrackerTest,
+                                  public ::testing::WithParamInterface<bool> {
+ public:
+  MruWindowTrackerOrderTest() {}
+  ~MruWindowTrackerOrderTest() override = default;
+
+  MruWindowTracker::WindowList BuildMruWindowList() const {
+    return GetParam()
+               ? Shell::Get()->mru_window_tracker()->BuildWindowListIgnoreModal(
+                     kActiveDesk)
+               : Shell::Get()->mru_window_tracker()->BuildMruWindowList(
+                     kActiveDesk);
+  }
+};
+
+namespace {
+
+class TestDelegate : public views::WidgetDelegateView {
+ public:
+  TestDelegate() = default;
+  ~TestDelegate() override = default;
+
+  // views::WidgetDelegateView:
+  ui::ModalType GetModalType() const override { return ui::MODAL_TYPE_SYSTEM; }
+};
+
+}  // namespace
+
 // Test that minimized windows are not treated specially.
-TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
+TEST_P(MruWindowTrackerOrderTest, MinimizedWindowsAreLru) {
   std::unique_ptr<aura::Window> w1(CreateTestWindow());
   std::unique_ptr<aura::Window> w2(CreateTestWindow());
   std::unique_ptr<aura::Window> w3(CreateTestWindow());
+
+  // Make w3 always on top.
+  w3->SetProperty(aura::client::kZOrderingKey,
+                  ui::ZOrderLevel::kFloatingWindow);
+  // They're in different container.
+  EXPECT_NE(w3->parent(), w1->parent());
+
   std::unique_ptr<aura::Window> w4(CreateTestWindow());
   std::unique_ptr<aura::Window> w5(CreateTestWindow());
   std::unique_ptr<aura::Window> w6(CreateTestWindow());
+
   wm::ActivateWindow(w6.get());
   wm::ActivateWindow(w5.get());
   wm::ActivateWindow(w4.get());
@@ -70,29 +124,34 @@ TEST_F(MruWindowTrackerTest, MinimizedWindowsAreLru) {
   // front of the MRU queue.
   EXPECT_TRUE(wm::IsActiveWindow(w2.get()));
 
-  MruWindowTracker::WindowList window_list =
-      mru_window_tracker()->BuildMruWindowList(kActiveDesk);
+  MruWindowTracker::WindowList window_list = BuildMruWindowList();
   EXPECT_EQ(w2.get(), window_list[0]);
   EXPECT_EQ(w1.get(), window_list[1]);
   EXPECT_EQ(w3.get(), window_list[2]);
   EXPECT_EQ(w4.get(), window_list[3]);
   EXPECT_EQ(w5.get(), window_list[4]);
   EXPECT_EQ(w6.get(), window_list[5]);
-}
 
-// Tests that windows being dragged are only in the WindowList once.
-TEST_F(MruWindowTrackerTest, DraggedWindowsInListOnlyOnce) {
-  std::unique_ptr<aura::Window> w1(CreateTestWindow());
-  wm::ActivateWindow(w1.get());
-
-  // Start dragging the window.
-  WindowState::Get(w1.get())->CreateDragDetails(gfx::Point(), HTRIGHT,
-                                                ::wm::WINDOW_MOVE_SOURCE_TOUCH);
-
-  // The dragged window should only be in the list once.
-  MruWindowTracker::WindowList window_list =
-      mru_window_tracker()->BuildWindowListIgnoreModal(kActiveDesk);
-  EXPECT_EQ(1, std::count(window_list.begin(), window_list.end(), w1.get()));
+  std::unique_ptr<views::Widget> modal =
+      CreateTestWidget(new TestDelegate(), kShellWindowId_Invalid);
+  EXPECT_EQ(modal.get()->GetNativeView()->parent()->id(),
+            kShellWindowId_SystemModalContainer);
+
+  window_list = BuildMruWindowList();
+  auto iter = window_list.begin();
+  if (GetParam()) {
+    EXPECT_EQ(w2.get(), *iter++);
+    EXPECT_EQ(w1.get(), *iter++);
+    EXPECT_EQ(w3.get(), *iter++);
+    EXPECT_EQ(w4.get(), *iter++);
+    EXPECT_EQ(w5.get(), *iter++);
+    EXPECT_EQ(w6.get(), *iter++);
+  }
+  EXPECT_EQ(iter, window_list.end());
 }
 
+INSTANTIATE_TEST_SUITE_P(MruWindowTrackerOrder,
+                         MruWindowTrackerOrderTest,
+                         /*use ignore modal=*/::testing::Bool());
+
 }  // namespace ash