wm: Modify PersistentWindowController to get MRU windows during lock.

This will allow us to skip restoring external display bounds when
heading to ACTIVE session, and instead restore on Display added during
a lock state.

Test: added test, manual
Bug: 1033263
Change-Id: I4bb91fc7db6f7423ce8280465e783cfc4f468dd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134570
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759876}

ash/display/persistent_window_controller.cc

@@ -5,9 +5,9 @@
 #include "ash/display/persistent_window_controller.h"
 
 #include "ash/display/persistent_window_info.h"
-#include "ash/session/session_controller_impl.h"
 #include "ash/shell.h"
 #include "ash/wm/mru_window_tracker.h"
+#include "ash/wm/tablet_mode/scoped_skip_user_session_blocked_check.h"
 #include "ash/wm/window_state.h"
 #include "base/bind.h"
 #include "base/command_line.h"
@@ -25,20 +25,19 @@ display::DisplayManager* GetDisplayManager() {
 }
 
 MruWindowTracker::WindowList GetWindowList() {
+  // MRU tracker normally skips windows if called during a non active session.
+  // |scoped_skip_user_session_blocked_check| allows us to get the list of MRU
+  // windows even when a display is added during for example lock session.
+  ScopedSkipUserSessionBlockedCheck scoped_skip_user_session_blocked_check;
   return Shell::Get()->mru_window_tracker()->BuildWindowForCycleList(kAllDesks);
 }
 
 // Returns true when window cycle list can be processed to perform save/restore
 // operations on observing display changes.
 bool ShouldProcessWindowList() {
-  // Window cycle list exists in active user session only.
-  if (!Shell::Get()->session_controller()->IsActiveUserSessionStarted())
+  if (!Shell::Get()->desks_controller())
     return false;
-
-  if (GetDisplayManager()->IsInMirrorMode())
-    return false;
-
-  return true;
+  return !GetDisplayManager()->IsInMirrorMode();
 }
 
 }  // namespace
@@ -47,13 +46,11 @@ constexpr char PersistentWindowController::kNumOfWindowsRestoredHistogramName[];
 
 PersistentWindowController::PersistentWindowController() {
   display::Screen::GetScreen()->AddObserver(this);
-  Shell::Get()->session_controller()->AddObserver(this);
   Shell::Get()->window_tree_host_manager()->AddObserver(this);
 }
 
 PersistentWindowController::~PersistentWindowController() {
   Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
-  Shell::Get()->session_controller()->RemoveObserver(this);
   display::Screen::GetScreen()->RemoveObserver(this);
 }
 
@@ -78,11 +75,6 @@ void PersistentWindowController::OnDisplayAdded(
       base::Unretained(this));
 }
 
-void PersistentWindowController::OnSessionStateChanged(
-    session_manager::SessionState state) {
-  MaybeRestorePersistentWindowBounds();
-}
-
 void PersistentWindowController::OnDisplayConfigurationChanged() {
   if (restore_callback_)
     std::move(restore_callback_).Run();

ash/display/persistent_window_controller.h

@@ -7,9 +7,7 @@
 
 #include "ash/ash_export.h"
 #include "ash/display/window_tree_host_manager.h"
-#include "ash/session/session_observer.h"
 #include "base/callback.h"
-#include "base/macros.h"
 #include "ui/display/display_observer.h"
 
 namespace ash {
@@ -18,7 +16,6 @@ namespace ash {
 // multi-displays scenario.
 class ASH_EXPORT PersistentWindowController
     : public display::DisplayObserver,
-      public SessionObserver,
       public WindowTreeHostManager::Observer {
  public:
   // Public so it can be used by unit tests.
@@ -26,6 +23,9 @@ class ASH_EXPORT PersistentWindowController
       "Ash.PersistentWindow.NumOfWindowsRestored";
 
   PersistentWindowController();
+  PersistentWindowController(const PersistentWindowController&) = delete;
+  PersistentWindowController& operator=(const PersistentWindowController&) =
+      delete;
   ~PersistentWindowController() override;
 
  private:
@@ -33,9 +33,6 @@ class ASH_EXPORT PersistentWindowController
   void OnWillProcessDisplayChanges() override;
   void OnDisplayAdded(const display::Display& new_display) override;
 
-  // SessionObserver:
-  void OnSessionStateChanged(session_manager::SessionState state) override;
-
   // WindowTreeHostManager::Observer:
   void OnDisplayConfigurationChanged() override;
 
@@ -44,8 +41,6 @@ class ASH_EXPORT PersistentWindowController
 
   // Callback binded on display added and run on display configuration changed.
   base::OnceCallback<void()> restore_callback_;
-
-  DISALLOW_COPY_AND_ASSIGN(PersistentWindowController);
 };
 
 }  // namespace ash

ash/display/persistent_window_controller_unittest.cc

@@ -341,7 +341,7 @@ TEST_F(PersistentWindowControllerTest, ReconnectOnLockScreen) {
   display_info_list.push_back(secondary_info);
   display_manager()->OnNativeDisplaysChanged(display_info_list);
   EXPECT_EQ(gfx::Rect(200, 0, 100, 200), w1->GetBoundsInScreen());
-  EXPECT_EQ(gfx::Rect(1, 0, 200, 100), w2->GetBoundsInScreen());
+  EXPECT_EQ(gfx::Rect(501, 0, 200, 100), w2->GetBoundsInScreen());
 
   // Unlocks and checks that |w2| is restored.
   GetSessionControllerClient()->SetSessionState(SessionState::ACTIVE);

ash/wm/window_util.cc

@@ -52,19 +52,6 @@ namespace ash {
 namespace window_util {
 namespace {
 
-// Moves |window| to the given |root| window's corresponding container, if it is
-// not already in the same root window. Returns true if |window| was moved.
-bool MoveWindowToRoot(aura::Window* window, aura::Window* root) {
-  if (!root || root == window->GetRootWindow())
-    return false;
-  aura::Window* container = RootWindowController::ForWindow(root)->GetContainer(
-      window->parent()->id());
-  if (!container)
-    return false;
-  container->AddChild(window);
-  return true;
-}
-
 // This window targeter reserves space for the portion of the resize handles
 // that extend within a top level window.
 class InteriorResizeHandleTargeter : public aura::WindowTargeter {
@@ -154,6 +141,13 @@ void SetAutoHideShelf(aura::Window* window, bool autohide) {
 
 bool MoveWindowToDisplay(aura::Window* window, int64_t display_id) {
   DCHECK(window);
+
+  aura::Window* root = Shell::GetRootWindowForDisplayId(display_id);
+  if (!root || root == window->GetRootWindow()) {
+    NOTREACHED();
+    return false;
+  }
+
   WindowState* window_state = WindowState::Get(window);
   if (window_state->allow_set_bounds_direct()) {
     display::Display display;
@@ -168,14 +162,22 @@ bool MoveWindowToDisplay(aura::Window* window, int64_t display_id) {
     window_state->OnWMEvent(&event);
     return true;
   }
-  aura::Window* root = Shell::GetRootWindowForDisplayId(display_id);
+
+  // Moves |window| to the given |root| window's corresponding container.
+  aura::Window* container = RootWindowController::ForWindow(root)->GetContainer(
+      window->parent()->id());
+  if (!container)
+    return false;
+
   // Update restore bounds to target root window.
   if (window_state->HasRestoreBounds()) {
     gfx::Rect restore_bounds = window_state->GetRestoreBoundsInParent();
     ::wm::ConvertRectToScreen(root, &restore_bounds);
     window_state->SetRestoreBoundsInScreen(restore_bounds);
   }
-  return root && MoveWindowToRoot(window, root);
+
+  container->AddChild(window);
+  return true;
 }
 
 int GetNonClientComponent(aura::Window* window, const gfx::Point& location) {

ash/wm/window_util_unittest.cc

@@ -156,13 +156,6 @@ TEST_F(WindowUtilTest, MoveWindowToDisplay) {
   const int original_container_id = window->parent()->id();
   const aura::Window* original_root = window->GetRootWindow();
 
-  EXPECT_FALSE(MoveWindowToDisplay(window.get(), display::kInvalidDisplayId));
-  EXPECT_EQ(original_display_id,
-            screen->GetDisplayNearestWindow(window.get()).id());
-  EXPECT_FALSE(MoveWindowToDisplay(window.get(), original_display_id));
-  EXPECT_EQ(original_display_id,
-            screen->GetDisplayNearestWindow(window.get()).id());
-
   ASSERT_EQ(2, screen->GetNumDisplays());
   const int64_t secondary_display_id = screen->GetAllDisplays()[1].id();
   EXPECT_NE(original_display_id, secondary_display_id);
@@ -179,6 +172,38 @@ TEST_F(WindowUtilTest, MoveWindowToDisplay) {
   EXPECT_EQ(original_root, window->GetRootWindow());
 }
 
+// Tests that locking and unlocking the screen does not alter the display of a
+// window moved by MoveWindowToDisplay.
+TEST_F(WindowUtilTest, MoveWindowToDisplayAndLockScreen) {
+  UpdateDisplay("500x400, 600x400");
+  auto window = CreateTestWindow(gfx::Rect(12, 20, 100, 100));
+  display::Screen* screen = display::Screen::GetScreen();
+  ASSERT_EQ(2, screen->GetNumDisplays());
+  const int64_t primary_display_id = screen->GetAllDisplays()[0].id();
+  const int64_t secondary_display_id = screen->GetAllDisplays()[1].id();
+  ASSERT_EQ(primary_display_id,
+            screen->GetDisplayNearestWindow(window.get()).id());
+
+  EXPECT_TRUE(MoveWindowToDisplay(window.get(), secondary_display_id));
+  EXPECT_EQ(secondary_display_id,
+            screen->GetDisplayNearestWindow(window.get()).id());
+
+  // Tests that after locking and unlocking the screen the window remains on the
+  // secondary display.
+  GetSessionControllerClient()->LockScreen();
+  GetSessionControllerClient()->UnlockScreen();
+  EXPECT_EQ(secondary_display_id,
+            screen->GetDisplayNearestWindow(window.get()).id());
+
+  // Move the window to the primary display. Tests that after locking and
+  // unlocking the screen the window remains on the secondary display.
+  EXPECT_TRUE(MoveWindowToDisplay(window.get(), primary_display_id));
+  GetSessionControllerClient()->LockScreen();
+  GetSessionControllerClient()->UnlockScreen();
+  EXPECT_EQ(primary_display_id,
+            screen->GetDisplayNearestWindow(window.get()).id());
+}
+
 TEST_F(WindowUtilTest, RemoveTransientDescendants) {
   // Create two windows which have no transient children or parents. Test that
   // neither of them get removed when running RemoveTransientDescendants.