Restore pinned window only when being destroyed

Bug: 1050816
Test: covered by unittest

Change-Id: Ie5d4ce89870cf6690793e9367ed75ee96bb2021b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2126089Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754540}

ash/wm/screen_pinning_controller.cc

@@ -157,6 +157,9 @@ ScreenPinningController::ScreenPinningController()
 
 ScreenPinningController::~ScreenPinningController() {
   Shell::Get()->window_tree_host_manager()->RemoveObserver(this);
+  if (pinned_window_)
+    pinned_window_->RemoveObserver(this);
+  pinned_window_ = nullptr;
 }
 
 bool ScreenPinningController::IsPinned() const {
@@ -177,6 +180,8 @@ void ScreenPinningController::SetPinnedWindow(aura::Window* pinned_window) {
 
     // Set up the container which has the pinned window.
     pinned_window_ = pinned_window;
+    // To monitor destruction.
+    pinned_window_->AddObserver(this);
     AlwaysOnTopController::SetDisallowReparent(pinned_window);
     container->StackChildAtTop(pinned_window);
     container->StackChildBelow(CreateWindowDimmer(container), pinned_window);
@@ -219,6 +224,7 @@ void ScreenPinningController::SetPinnedWindow(aura::Window* pinned_window) {
     container->RemoveObserver(pinned_container_window_observer_.get());
 
     window_dimmers_->clear();
+    pinned_window_->RemoveObserver(this);
     pinned_window_ = nullptr;
   }
 
@@ -234,8 +240,6 @@ void ScreenPinningController::OnWindowAddedToPinnedContainer(
 void ScreenPinningController::OnWillRemoveWindowFromPinnedContainer(
     aura::Window* window) {
   window->RemoveObserver(pinned_container_child_window_observer_.get());
-  if (window == pinned_window_)
-    WindowState::Get(pinned_window_)->Restore();
 }
 
 void ScreenPinningController::OnPinnedContainerWindowStackingChanged(
@@ -311,6 +315,13 @@ void ScreenPinningController::OnDisplayConfigurationChanged() {
   }
 }
 
+void ScreenPinningController::OnWindowDestroying(aura::Window* window) {
+  DCHECK_EQ(pinned_window_, window);
+  WindowState::Get(window)->Restore();
+  window->RemoveObserver(this);
+  pinned_window_ = nullptr;
+}
+
 void ScreenPinningController::KeepPinnedWindowOnTop() {
   if (in_restacking_)
     return;

ash/wm/screen_pinning_controller.h

@@ -31,7 +31,8 @@ class WindowUserData;
 // https://developer.android.com/about/versions/android-5.0.html#ScreenPinning
 // See also ArcKioskAppLauncher::CheckAndPinWindow().
 class ASH_EXPORT ScreenPinningController
-    : public WindowTreeHostManager::Observer {
+    : public WindowTreeHostManager::Observer,
+      aura::WindowObserver {
  public:
   ScreenPinningController();
   ~ScreenPinningController() override;
@@ -86,6 +87,9 @@ class ASH_EXPORT ScreenPinningController
   // WindowTreeHostManager::Observer:
   void OnDisplayConfigurationChanged() override;
 
+  // aura::WindowObserver:
+  void OnWindowDestroying(aura::Window* window) override;
+
   // Pinned window should be on top in the parent window.
   aura::Window* pinned_window_ = nullptr;
 

ash/wm/screen_pinning_controller_unittest.cc

@@ -165,4 +165,24 @@ TEST_F(ScreenPinningControllerTest, TrustedPinnedWithAccelerator) {
   EXPECT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
 }
 
+TEST_F(ScreenPinningControllerTest, ExitUnifiedDisplay) {
+  display_manager()->SetUnifiedDesktopEnabled(true);
+
+  UpdateDisplay("400x300, 400x400");
+
+  aura::Window* w1 = CreateTestWindowInShellWithId(0);
+  wm::ActivateWindow(w1);
+  auto* window_state = WindowState::Get(w1);
+
+  window_util::PinWindow(w1, /*trusted=*/true);
+
+  EXPECT_TRUE(window_state->IsPinned());
+  EXPECT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
+
+  UpdateDisplay("200x200");
+
+  EXPECT_TRUE(window_state->IsPinned());
+  EXPECT_TRUE(Shell::Get()->screen_pinning_controller()->IsPinned());
+}
+
 }  // namespace ash