Remove stale copy of WaylandOutput

WaylandSurface retains a copy of the already removed WaylandOutput in
entered_output_ when the monitor is unplugged or switched off in a
multi-monitor setup.

Bug: 1238354
Change-Id: I8427a7b8defc65ead18e9793df2217f23267ca23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3084287
Reviewed-by: Maksim Sisov <msisov@igalia.com>
Commit-Queue: Abhijeet Kandalkar <abhijeet@igalia.com>
Cr-Commit-Position: refs/heads/master@{#911808}

ui/ozone/platform/wayland/host/wayland_output_manager.cc

@@ -53,6 +53,14 @@ void WaylandOutputManager::RemoveWaylandOutput(uint32_t output_id) {
   if (output_it == output_list_.end())
     return;
 
+  // Remove WaylandOutput in following order :
+  // 1. from `WaylandSurface::entered_outputs_`
+  // 2. from `WaylandScreen::display_list_`
+  // 3. from `WaylandOutputManager::output_list_`
+  auto* wayland_window_manager = connection_->wayland_window_manager();
+  for (auto* window : wayland_window_manager->GetAllWindows())
+    window->RemoveEnteredOutput(output_id);
+
   if (wayland_screen_)
     wayland_screen_->OnOutputRemoved(output_id);
   output_list_.erase(output_it);

ui/ozone/platform/wayland/host/wayland_screen_unittest.cc

@@ -141,6 +141,77 @@ TEST_P(WaylandScreenTest, OutputBaseTest) {
             gfx::Rect(0, 0, kOutputWidth, kOutputHeight));
 }
 
+// In multi-monitor setup, the `entered_outputs_` list should be updated when
+// the display is unplugged or switched off.
+TEST_P(WaylandScreenTest, EnteredOutputListAfterDisplayRemoval) {
+  wl::TestOutput* output1 = server_.output();
+  gfx::Rect output1_rect = server_.output()->GetRect();
+
+  // Add a second display.
+  wl::TestOutput* output2 = server_.CreateAndInitializeOutput();
+  Sync();
+  // The second display is located to the right of first display
+  gfx::Rect output2_rect(output1_rect.right(), 0, 800, 600);
+  output2->SetRect(output2_rect);
+  output2->Flush();
+  Sync();
+
+  // Add a third display.
+  wl::TestOutput* output3 = server_.CreateAndInitializeOutput();
+  Sync();
+  // The third display is located to the right of second display
+  gfx::Rect output3_rect(output2_rect.right(), 0, 800, 600);
+  output3->SetRect(output3_rect);
+  output3->Flush();
+  Sync();
+
+  EXPECT_EQ(3u, platform_screen_->GetAllDisplays().size());
+
+  wl::MockSurface* surface = server_.GetObject<wl::MockSurface>(
+      window_->root_surface()->GetSurfaceId());
+  ASSERT_TRUE(surface);
+
+  wl_surface_send_enter(surface->resource(), output1->resource());
+  wl_surface_send_enter(surface->resource(), output2->resource());
+  Sync();
+  // The window entered two outputs
+  auto entered_outputs = window_->root_surface()->entered_outputs();
+  EXPECT_EQ(2u, entered_outputs.size());
+
+  wl_surface_send_enter(surface->resource(), output3->resource());
+  Sync();
+  // The window entered three outputs
+  entered_outputs = window_->root_surface()->entered_outputs();
+  EXPECT_EQ(3u, entered_outputs.size());
+
+  // Destroy third display
+  output3->DestroyGlobal();
+  Sync();
+  entered_outputs = window_->root_surface()->entered_outputs();
+  EXPECT_EQ(2u, entered_outputs.size());
+
+  // Destroy second display
+  output2->DestroyGlobal();
+  Sync();
+  entered_outputs = window_->root_surface()->entered_outputs();
+  EXPECT_EQ(1u, entered_outputs.size());
+
+  // Add a second display.
+  output2 = server_.CreateAndInitializeOutput();
+  Sync();
+  // The second display is located to the right of first display
+  output2->SetRect(output2_rect);
+  output2->Flush();
+  Sync();
+
+  wl_surface_send_enter(surface->resource(), output2->resource());
+  Sync();
+
+  // The window entered two outputs
+  entered_outputs = window_->root_surface()->entered_outputs();
+  EXPECT_EQ(2u, entered_outputs.size());
+}
+
 TEST_P(WaylandScreenTest, MultipleOutputsAddedAndRemoved) {
   TestDisplayObserver observer;
   platform_screen_->AddObserver(&observer);

ui/ozone/platform/wayland/host/wayland_surface.cc

@@ -352,8 +352,9 @@ void WaylandSurface::Enter(void* data,
   auto* const surface = static_cast<WaylandSurface*>(data);
   DCHECK(surface);
 
-  surface->entered_outputs_.emplace_back(
-      static_cast<WaylandOutput*>(wl_output_get_user_data(output)));
+  auto* wayland_output =
+      static_cast<WaylandOutput*>(wl_output_get_user_data(output));
+  surface->entered_outputs_.emplace_back(wayland_output);
 
   if (surface->root_window_)
     surface->root_window_->OnEnteredOutputIdAdded();
@@ -366,20 +367,33 @@ void WaylandSurface::Leave(void* data,
   auto* const surface = static_cast<WaylandSurface*>(data);
   DCHECK(surface);
 
-  auto entered_outputs_it_ = std::find(
-      surface->entered_outputs_.begin(), surface->entered_outputs_.end(),
-      static_cast<WaylandOutput*>(wl_output_get_user_data(output)));
-  // Workaround: when a user switches physical output between two displays,
-  // a surface does not necessarily receive enter events immediately or until
-  // a user resizes/moves it.  This means that switching output between
+  auto* wayland_output =
+      static_cast<WaylandOutput*>(wl_output_get_user_data(output));
+  surface->RemoveEnteredOutput(wayland_output->output_id());
+}
+
+void WaylandSurface::RemoveEnteredOutput(uint32_t output_id) {
+  if (entered_outputs().empty())
+    return;
+
+  auto entered_outputs_it_ = std::find_if(
+      entered_outputs_.begin(), entered_outputs_.end(),
+      [&output_id](auto* it) { return it->output_id() == output_id; });
+
+  // The `entered_outputs_` list should be updated,
+  // 1. for wl_surface::leave, when a user switches physical output between two
+  // displays, a surface does not necessarily receive enter events immediately
+  // or until a user resizes/moves it.  This means that switching output between
   // displays in a single output mode results in leave events, but the surface
   // might not have received enter event before.  Thus, remove the id of the
   // output that the surface leaves only if it was stored before.
-  if (entered_outputs_it_ != surface->entered_outputs_.end())
-    surface->entered_outputs_.erase(entered_outputs_it_);
+  // 2. for wl_registry::global_remove, when wl_output is removed by a server
+  // after the display is unplugged or switched off.
+  if (entered_outputs_it_ != entered_outputs_.end())
+    entered_outputs_.erase(entered_outputs_it_);
 
-  if (surface->root_window_)
-    surface->root_window_->OnEnteredOutputIdRemoved();
+  if (root_window_)
+    root_window_->OnEnteredOutputIdRemoved();
 }
 
 // static

ui/ozone/platform/wayland/host/wayland_surface.h

@@ -122,6 +122,10 @@ class WaylandSurface {
   // |parent|. Callers take ownership of the wl_subsurface.
   wl::Object<wl_subsurface> CreateSubsurface(WaylandSurface* parent);
 
+  // When display is removed, the WaylandOutput from `entered_outputs_` should
+  // be removed.
+  void RemoveEnteredOutput(uint32_t id);
+
  private:
   // Holds information about each explicit synchronization buffer release.
   struct ExplicitReleaseInfo {

ui/ozone/platform/wayland/host/wayland_window.cc

@@ -181,6 +181,10 @@ void WaylandWindow::SetPointerFocus(bool focus) {
     UpdateCursorShape(cursor_);
 }
 
+void WaylandWindow::RemoveEnteredOutput(uint32_t output_id) {
+  root_surface_->RemoveEnteredOutput(output_id);
+}
+
 bool WaylandWindow::StartDrag(const ui::OSExchangeData& data,
                               int operation,
                               gfx::NativeCursor cursor,

ui/ozone/platform/wayland/host/wayland_window.h

@@ -127,6 +127,9 @@ class WaylandWindow : public PlatformWindow,
     update_visual_size_immediately_ = update_immediately;
   }
 
+  // Remove WaylandOutput associated with WaylandSurface of this window.
+  void RemoveEnteredOutput(uint32_t output_id);
+
   // WmDragHandler
   bool StartDrag(const ui::OSExchangeData& data,
                  int operation,