Prevent an argument over window ownership

When a surface is both always on top and pinned there is a bad
interaction between ScreenPinningController and AlwaysOnTopController.
AlwaysOnTopController will attempt to reparent a window that
ScreenPinningController is positioning, causing a crash. Prevent this
using AlwaysOnTopController::SetDisallowReparent.

Bug: 980366
Change-Id: I99f86d43fa6092b4d6c1729de1c983178c76eb6a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1701313
Commit-Queue: Fergus Dall <sidereal@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678544}

ash/wm/screen_pinning_controller.cc

@@ -11,6 +11,7 @@
 #include "ash/public/cpp/shell_window_ids.h"
 #include "ash/shell.h"
 #include "ash/window_user_data.h"
+#include "ash/wm/always_on_top_controller.h"
 #include "ash/wm/container_finder.h"
 #include "ash/wm/window_dimmer.h"
 #include "ash/wm/window_state.h"
@@ -176,6 +177,7 @@ void ScreenPinningController::SetPinnedWindow(aura::Window* pinned_window) {
 
     // Set up the container which has the pinned window.
     pinned_window_ = pinned_window;
+    AlwaysOnTopController::SetDisallowReparent(pinned_window);
     container->StackChildAtTop(pinned_window);
     container->StackChildBelow(CreateWindowDimmer(container), pinned_window);
 
@@ -262,6 +264,7 @@ aura::Window* ScreenPinningController::CreateWindowDimmer(
   std::unique_ptr<WindowDimmer> window_dimmer =
       std::make_unique<WindowDimmer>(container);
   window_dimmer->SetDimOpacity(1);  // Fully opaque.
+  AlwaysOnTopController::SetDisallowReparent(window_dimmer->window());
   ::wm::SetWindowFullscreen(window_dimmer->window(), true);
   window_dimmer->window()->Show();
   aura::Window* window = window_dimmer->window();

ash/wm/workspace/workspace_layout_manager.cc

@@ -518,6 +518,9 @@ void WorkspaceLayoutManager::UpdateAlwaysOnTop(
   // appropriate windows will be included in the iteration.
   WindowSet windows(windows_);
   for (aura::Window* window : windows) {
+    if (window == active_desk_fullscreen_window)
+      continue;
+
     wm::WindowState* window_state = wm::GetWindowState(window);
     if (active_desk_fullscreen_window)
       window_state->DisableZOrdering(active_desk_fullscreen_window);

components/exo/display_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "components/exo/display.h"
 #include "ash/public/cpp/shell_window_ids.h"
+#include "ash/public/cpp/window_pin_type.h"
 #include "ash/wm/desks/desks_util.h"
 #include "components/exo/buffer.h"
 #include "components/exo/client_controlled_shell_surface.h"
@@ -245,5 +246,23 @@ TEST_F(DisplayTest, CreateDataDevice) {
   EXPECT_TRUE(device.get());
 }
 
+TEST_F(DisplayTest, PinnedAlwaysOnTopWindow) {
+  Display display;
+
+  std::unique_ptr<Surface> surface = display.CreateSurface();
+  ASSERT_TRUE(surface);
+
+  std::unique_ptr<ClientControlledShellSurface> shell_surface =
+      display.CreateClientControlledShellSurface(
+          surface.get(), ash::desks_util::GetActiveDeskContainerId(),
+          2.0 /* default_scale_factor */);
+  ASSERT_TRUE(shell_surface);
+  EXPECT_EQ(shell_surface->scale(), 2.0);
+
+  // This should not crash
+  shell_surface->SetAlwaysOnTop(true);
+  shell_surface->SetPinned(ash::WindowPinType::kPinned);
+}
+
 }  // namespace
 }  // namespace exo

ui/aura/window.cc

@@ -960,6 +960,10 @@ void Window::StackChildRelativeTo(Window* child,
   const size_t target_i =
       std::find(children_.begin(), children_.end(), target) - children_.begin();
 
+  DCHECK_LT(child_i, children_.size()) << "Child was not in list of children!";
+  DCHECK_LT(target_i, children_.size())
+      << "Target was not in list of children!";
+
   // Don't move the child if it is already in the right place.
   if ((direction == STACK_ABOVE && child_i == target_i + 1) ||
       (direction == STACK_BELOW && child_i + 1 == target_i))