Wait destroying wl_output objects

1) until clients release their wl_output outputs.
2) but up to 9 seconds, then destroy the global output object.

ug: b/168545216
Change-Id: I1035f417c5461ba9337a61506acba83d6ba08080
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2553527
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833901}

components/exo/wayland/server.cc

@@ -340,7 +340,10 @@ void Server::OnDisplayAdded(const display::Display& new_display) {
 
 void Server::OnDisplayRemoved(const display::Display& old_display) {
   DCHECK_EQ(outputs_.count(old_display.id()), 1u);
+  std::unique_ptr<WaylandDisplayOutput> output =
+      std::move(outputs_[old_display.id()]);
   outputs_.erase(old_display.id());
+  output.release()->OnDisplayRemoved();
 }
 
 wl_resource* Server::GetOutputResource(wl_client* client, int64_t display_id) {

components/exo/wayland/wayland_display_observer.cc

@@ -35,9 +35,14 @@ void WaylandDisplayHandler::AddObserver(WaylandDisplayObserver* observer) {
   observers_.AddObserver(observer);
 
   display::Display display;
-  bool rv = display::Screen::GetScreen()->GetDisplayWithDisplayId(output_->id(),
-                                                                  &display);
-  DCHECK(rv);
+  bool exists = display::Screen::GetScreen()->GetDisplayWithDisplayId(
+      output_->id(), &display);
+  if (!exists) {
+    // WaylandDisplayHandler is created asynchronously, and the
+    // display can be deleted before created. This usually won't happen
+    // in real environment, but can happen in test environment.
+    return;
+  }
 
   // Send the first round of changes to the observer.
   constexpr uint32_t all_changes = 0xFFFFFFFF;
@@ -53,6 +58,8 @@ void WaylandDisplayHandler::AddObserver(WaylandDisplayObserver* observer) {
 void WaylandDisplayHandler::OnDisplayMetricsChanged(
     const display::Display& display,
     uint32_t changed_metrics) {
+  DCHECK(output_resource_);
+
   if (output_->id() != display.id())
     return;
 
@@ -68,9 +75,11 @@ void WaylandDisplayHandler::OnDisplayMetricsChanged(
     wl_client_flush(wl_resource_get_client(output_resource_));
   }
 }
-
 bool WaylandDisplayHandler::SendDisplayMetrics(const display::Display& display,
                                                uint32_t changed_metrics) {
+  if (!output_resource_)
+    return false;
+
   // There is no need to check DISPLAY_METRIC_PRIMARY because when primary
   // changes, bounds always changes. (new primary should have had non
   // 0,0 origin).

components/exo/wayland/wayland_display_output.cc

@@ -11,11 +11,27 @@
 
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "base/task/thread_pool.h"
 #include "components/exo/surface.h"
 #include "components/exo/wayland/server_util.h"
 
 namespace exo {
 namespace wayland {
+namespace {
+base::TimeDelta kDeleteTaskDelay = base::TimeDelta::FromSeconds(3);
+
+void DoDelete(WaylandDisplayOutput* output, int retry_count) {
+  if (retry_count > 0 && output->output_counts() > 0) {
+    // Try a few times to give a client chance to release it.
+    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, base::BindOnce(&DoDelete, output, retry_count - 1),
+        kDeleteTaskDelay);
+  } else {
+    delete output;
+  }
+}
+
+}  // namespace
 
 WaylandDisplayOutput::WaylandDisplayOutput(int64_t id) : id_(id) {}
 
@@ -29,6 +45,15 @@ WaylandDisplayOutput::~WaylandDisplayOutput() {
     wl_global_destroy(global_);
 }
 
+void WaylandDisplayOutput::OnDisplayRemoved() {
+  if (global_)
+    wl_global_remove(global_);
+
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, base::BindOnce(&DoDelete, this, /*retry_count=*/3),
+      kDeleteTaskDelay);
+}
+
 int64_t WaylandDisplayOutput::id() const {
   return id_;
 }

components/exo/wayland/wayland_display_output.h

@@ -17,8 +17,11 @@ struct wl_resource;
 namespace exo {
 namespace wayland {
 
-// Class that represent a wayland output. Tied to a specific display ID
-// and associated with a global.
+// Class that represent a wayland output. Tied to a specific display ID and
+// associated with a global, and wl_outputs created by clients.  This object
+// will self destruct upon the display removal aftrer delays up to 9 seconds (3
+// seconds x 3 times) to give time for clients to release the output they
+// created, excepf for the shutdown scenario where they're removed immediately.
 class WaylandDisplayOutput {
  public:
   explicit WaylandDisplayOutput(int64_t display_id);
@@ -34,6 +37,11 @@ class WaylandDisplayOutput {
 
   wl_resource* GetOutputResourceForClient(wl_client* client);
 
+  // Self destruct in 5 seconeds.
+  void OnDisplayRemoved();
+
+  int output_counts() const { return output_ids_.size(); }
+
  private:
   const int64_t id_;
   wl_global* global_ = nullptr;