Fix WaylandDisplayObserver and output resources leaks

They needs to be deleted when the display associated with it is disconnected.

Change-Id: I2b65f84b8615b0394bfe405aeb99c85a9f866687
Bug: b/159560648
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2325130Reviewed-by: Jun Mukai <mukai@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793244}

components/exo/wayland/wayland_display_observer.cc

@@ -8,6 +8,7 @@
 
 #include <string>
 
+#include "components/exo/wayland/wayland_display_output.h"
 #include "components/exo/wm_helper.h"
 #include "ui/display/manager/managed_display_info.h"
 #include "ui/display/screen.h"
@@ -15,14 +16,16 @@
 namespace exo {
 namespace wayland {
 
-WaylandDisplayObserver::WaylandDisplayObserver(int64_t id,
+WaylandDisplayObserver::WaylandDisplayObserver(WaylandDisplayOutput* output,
                                                wl_resource* output_resource)
-    : id_(id), output_resource_(output_resource) {
+    : output_(output), output_resource_(output_resource) {
+  output_->RegisterOutput(output_resource_);
   display::Screen::GetScreen()->AddObserver(this);
   SendDisplayMetrics();
 }
 
 WaylandDisplayObserver::~WaylandDisplayObserver() {
+  output_->UnregisterOutput(output_resource_);
   display::Screen::GetScreen()->RemoveObserver(this);
 }
 
@@ -34,7 +37,7 @@ void WaylandDisplayObserver::AddScaleObserver(ScaleObserver* scale_observer) {
 void WaylandDisplayObserver::OnDisplayMetricsChanged(
     const display::Display& display,
     uint32_t changed_metrics) {
-  if (id_ != display.id())
+  if (output_->id() != display.id())
     return;
 
   // There is no need to check DISPLAY_METRIC_PRIMARY because when primary
@@ -54,8 +57,8 @@ void WaylandDisplayObserver::OnDisplayMetricsChanged(
 
 void WaylandDisplayObserver::SendDisplayMetrics() {
   display::Display display;
-  bool rv =
-      display::Screen::GetScreen()->GetDisplayWithDisplayId(id_, &display);
+  bool rv = display::Screen::GetScreen()->GetDisplayWithDisplayId(output_->id(),
+                                                                  &display);
   DCHECK(rv);
 
   const display::ManagedDisplayInfo& info =

components/exo/wayland/wayland_display_observer.h

@@ -16,6 +16,7 @@ struct wl_resource;
 
 namespace exo {
 namespace wayland {
+class WaylandDisplayOutput;
 
 class WaylandDisplayObserver : public display::DisplayObserver {
  public:
@@ -29,7 +30,8 @@ class WaylandDisplayObserver : public display::DisplayObserver {
     ~ScaleObserver() override {}
   };
 
-  WaylandDisplayObserver(int64_t id, wl_resource* output_resource);
+  WaylandDisplayObserver(WaylandDisplayOutput* output,
+                         wl_resource* output_resource);
   ~WaylandDisplayObserver() override;
   void AddScaleObserver(ScaleObserver* scale_observer);
   bool HasScaleObserver(ScaleObserver* scale_observer) const;
@@ -44,8 +46,8 @@ class WaylandDisplayObserver : public display::DisplayObserver {
   // compensate for the rotation of an output device.
   wl_output_transform OutputTransform(display::Display::Rotation rotation);
 
-  // The ID of the display being observed.
-  const int64_t id_;
+  // Output.
+  WaylandDisplayOutput* output_;
 
   // The output resource associated with the display.
   wl_resource* const output_resource_;

components/exo/wayland/wayland_display_output.cc

@@ -7,12 +7,22 @@
 #include <wayland-server-core.h>
 #include <wayland-server-protocol-core.h>
 
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "components/exo/surface.h"
+#include "components/exo/wayland/server_util.h"
+
 namespace exo {
 namespace wayland {
 
 WaylandDisplayOutput::WaylandDisplayOutput(int64_t id) : id_(id) {}
 
 WaylandDisplayOutput::~WaylandDisplayOutput() {
+  // Empty the output_ids_ so that Unregister will be no op.
+  auto ids = std::move(output_ids_);
+  for (auto pair : ids)
+    wl_resource_destroy(pair.second);
+
   if (global_)
     wl_global_destroy(global_);
 }
@@ -25,5 +35,24 @@ void WaylandDisplayOutput::set_global(wl_global* global) {
   global_ = global;
 }
 
+void WaylandDisplayOutput::UnregisterOutput(wl_resource* output_resource) {
+  base::EraseIf(output_ids_, [output_resource](auto& pair) {
+    return pair.second == output_resource;
+  });
+}
+
+void WaylandDisplayOutput::RegisterOutput(wl_resource* output_resource) {
+  auto* client = wl_resource_get_client(output_resource);
+  output_ids_.insert(std::make_pair(client, output_resource));
+}
+
+wl_resource* WaylandDisplayOutput::GetOutputResourceForClient(
+    wl_client* client) {
+  auto iter = output_ids_.find(client);
+  if (iter == output_ids_.end())
+    return nullptr;
+  return iter->second;
+}
+
 }  // namespace wayland
 }  // namespace exo

components/exo/wayland/wayland_display_output.h

@@ -7,9 +7,12 @@
 
 #include <stdint.h>
 
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 
+struct wl_client;
 struct wl_global;
+struct wl_resource;
 
 namespace exo {
 namespace wayland {
@@ -18,15 +21,23 @@ namespace wayland {
 // and associated with a global.
 class WaylandDisplayOutput {
  public:
-  explicit WaylandDisplayOutput(int64_t id);
+  explicit WaylandDisplayOutput(int64_t display_id);
   ~WaylandDisplayOutput();
 
   int64_t id() const;
   void set_global(wl_global* global);
 
+  // Register/Unregister output resources, which will be used to
+  // notify surface when enter/leave the output.
+  void UnregisterOutput(wl_resource* output_resource);
+  void RegisterOutput(wl_resource* output_resource);
+
+  wl_resource* GetOutputResourceForClient(wl_client* client);
+
  private:
   const int64_t id_;
   wl_global* global_ = nullptr;
+  base::flat_map<wl_client*, wl_resource*> output_ids_;
 
   DISALLOW_COPY_AND_ASSIGN(WaylandDisplayOutput);
 };

components/exo/wayland/wl_compositor.cc

@@ -12,14 +12,17 @@
 #include "components/exo/buffer.h"
 #include "components/exo/display.h"
 #include "components/exo/surface.h"
+#include "components/exo/wayland/server.h"
 #include "components/exo/wayland/server_util.h"
 #include "third_party/skia/include/core/SkRegion.h"
+#include "ui/display/types/display_constants.h"
 
 #if defined(OS_CHROMEOS)
 #include "components/exo/wayland/zwp_linux_explicit_synchronization.h"
 #endif
 
 namespace exo {
+class Server;
 namespace wayland {
 namespace {
 
@@ -202,8 +205,8 @@ const struct wl_surface_interface surface_implementation = {
 void compositor_create_surface(wl_client* client,
                                wl_resource* resource,
                                uint32_t id) {
-  std::unique_ptr<Surface> surface =
-      GetUserDataAs<Display>(resource)->CreateSurface();
+  Display* display = GetUserDataAs<Display>(resource);
+  std::unique_ptr<Surface> surface = display->CreateSurface();
 
   wl_resource* surface_resource = wl_resource_create(
       client, &wl_surface_interface, wl_resource_get_version(resource), id);

components/exo/wayland/wl_output.cc

@@ -28,15 +28,17 @@ void output_release(wl_client* client, wl_resource* resource) {
 
 const struct wl_output_interface output_implementation = {output_release};
 
-void bind_output(wl_client* client, void* data, uint32_t version, uint32_t id) {
+void bind_output(wl_client* client,
+                 void* data,
+                 uint32_t version,
+                 uint32_t output_id) {
   WaylandDisplayOutput* output = static_cast<WaylandDisplayOutput*>(data);
 
-  wl_resource* resource = wl_resource_create(
-      client, &wl_output_interface, std::min(version, kWlOutputVersion), id);
-
-  SetImplementation(
-      resource, &output_implementation,
-      std::make_unique<WaylandDisplayObserver>(output->id(), resource));
+  wl_resource* resource =
+      wl_resource_create(client, &wl_output_interface,
+                         std::min(version, kWlOutputVersion), output_id);
+  SetImplementation(resource, &output_implementation,
+                    std::make_unique<WaylandDisplayObserver>(output, resource));
 }
 
 }  // namespace wayland