Fix crash during wayland server shutdown.

Explicitly shutdown SerialTracker and Seat before destroying
display, as deleting window depends on them.

Bug: 1111390, 1124106
Test: manually tested by existing chrome while lacros is running

Change-Id: I54ec70bb6d5e35fd178ccc90ed748210486394fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2387481Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804261}

components/exo/display.cc

@@ -72,7 +72,16 @@ Display::Display(
           gfx::CreateClientNativePixmapFactoryDmabuf()) {}
 #endif  // defined(OS_CHROMEOS)
 
-Display::~Display() {}
+Display::~Display() {
+  Shutdown();
+}
+
+void Display::Shutdown() {
+  if (shutdown_)
+    return;
+  shutdown_ = true;
+  seat_.Shutdown();
+}
 
 std::unique_ptr<Surface> Display::CreateSurface() {
   TRACE_EVENT0("exo", "Display::CreateSurface");
@@ -272,7 +281,7 @@ std::unique_ptr<SubSurface> Display::CreateSubSurface(Surface* surface,
 
 std::unique_ptr<DataDevice> Display::CreateDataDevice(
     DataDeviceDelegate* delegate) {
-  return std::make_unique<DataDevice>(delegate, &seat_, file_helper_.get());
+  return std::make_unique<DataDevice>(delegate, seat(), file_helper_.get());
 }
 
 }  // namespace exo

components/exo/display.h

@@ -66,6 +66,8 @@ class Display {
 
   ~Display();
 
+  void Shutdown();
+
   // Creates a new surface.
   std::unique_ptr<Surface> CreateSurface();
 
@@ -142,6 +144,8 @@ class Display {
   std::unique_ptr<FileHelper> file_helper_;
   Seat seat_;
 
+  bool shutdown_ = false;
+
 #if defined(USE_OZONE)
   std::unique_ptr<gfx::ClientNativePixmapFactory> client_native_pixmap_factory_;
 #endif  // defined(USE_OZONE)

components/exo/seat.cc

@@ -65,6 +65,13 @@ Seat::Seat() : changing_clipboard_data_to_selection_source_(false) {
 }
 
 Seat::~Seat() {
+  Shutdown();
+}
+
+void Seat::Shutdown() {
+  if (shutdown_)
+    return;
+  shutdown_ = true;
   DCHECK(!selection_source_) << "DataSource must be released before Seat";
   WMHelper::GetInstance()->RemoveFocusObserver(this);
   WMHelper::GetInstance()->RemovePreTargetHandler(this);

components/exo/seat.h

@@ -49,6 +49,8 @@ class Seat : public aura::client::FocusChangeObserver,
   Seat();
   ~Seat() override;
 
+  void Shutdown();
+
   void AddObserver(SeatObserver* observer);
   void RemoveObserver(SeatObserver* observer);
 
@@ -163,6 +165,8 @@ class Seat : public aura::client::FocusChangeObserver,
 
   gfx::Point last_location_;
 
+  bool shutdown_ = false;
+
 #if defined(OS_CHROMEOS)
   std::unique_ptr<UILockController> ui_lock_controller_;
 #endif  // defined(OS_CHROMEOS)

components/exo/wayland/serial_tracker.cc

@@ -21,7 +21,14 @@ SerialTracker::SerialTracker(struct wl_display* display)
 
 SerialTracker::~SerialTracker() {}
 
+void SerialTracker::Shutdown() {
+  display_ = nullptr;
+}
+
 uint32_t SerialTracker::GetNextSerial(EventType type) {
+  if (!display_)
+    return 0;
+
   uint32_t serial = wl_display_next_serial(display_);
   events_[serial % kMaxEventsTracked] = type;
   max_event_ = serial + 1;

components/exo/wayland/serial_tracker.h

@@ -34,6 +34,9 @@ class SerialTracker {
   explicit SerialTracker(struct wl_display* display);
   ~SerialTracker();
 
+  // After shutdown, |GetNextSerial| returns 0.
+  void Shutdown();
+
   uint32_t GetNextSerial(EventType type);
 
   // Get the serial number of the last {pointer,touch} pressed event, or nullopt
@@ -52,7 +55,7 @@ class SerialTracker {
  private:
   FRIEND_TEST_ALL_PREFIXES(SerialTrackerTest, WrapAroundWholeRange);
 
-  struct wl_display* const display_;
+  struct wl_display* display_;
 
   // EventTypes are stored in a circular buffer, because serial numbers are
   // issued sequentially and we only want to store the most recent events.

components/exo/wayland/server.cc

@@ -126,9 +126,8 @@ bool IsDrmAtomicAvailable() {
 // Server, public:
 
 Server::Server(Display* display)
-    : display_(display),
-      wl_display_(wl_display_create()),
-      serial_tracker_(std::make_unique<SerialTracker>(wl_display_.get())) {
+    : display_(display), wl_display_(wl_display_create()) {
+  serial_tracker_ = std::make_unique<SerialTracker>(wl_display_.get());
   wl_global_create(wl_display_.get(), &wl_compositor_interface,
                    kWlCompositorVersion, this, bind_compositor);
   wl_global_create(wl_display_.get(), &wl_shm_interface, 1, display_, bind_shm);
@@ -232,6 +231,10 @@ Server::Server(Display* display)
 
 Server::~Server() {
   display::Screen::GetScreen()->RemoveObserver(this);
+  // TODO(https://crbug.com/1124106): Investigate if we can eliminate Shutdown
+  // methods.
+  serial_tracker_->Shutdown();
+  display_->Shutdown();
 }
 
 // static

components/exo/wayland/server.h

@@ -67,8 +67,9 @@ class Server : public display::DisplayObserver {
 
  private:
   Display* const display_;
-  std::unique_ptr<wl_display, WlDisplayDeleter> wl_display_;
+  // Deleting wl_display depends on SerialTracker.
   std::unique_ptr<SerialTracker> serial_tracker_;
+  std::unique_ptr<wl_display, WlDisplayDeleter> wl_display_;
   base::flat_map<int64_t, std::unique_ptr<WaylandDisplayOutput>> outputs_;
   std::unique_ptr<WaylandDataDeviceManager> data_device_manager_data_;
   std::unique_ptr<WaylandSeat> seat_data_;