free-the-X11-pixmap-in-the-NativePixmapEGLX11Bind.patch 2.6 KB

12345678910111213141516171819202122232425262728293031323334353637383940414243444546474849505152535455565758596061626364656667
  1. From 42d57d016f5fb6d2a1a354743b9be911c1be87e8 Mon Sep 17 00:00:00 2001
  2. From: Jianhui Dai <jianhui.j.dai@intel.com>
  3. Date: Fri, 22 Sep 2023 21:30:04 +0000
  4. Subject: [PATCH] [GL] Free the X11 pixmap in the NativePixmapEGLX11Binding
  5. destructor
  6. This CL frees the X11 pixmap in the NativePixmapEGLX11Binding destructor
  7. to prevent a memory leak in the X server.
  8. Bug: 1467689
  9. Change-Id: Id4cba30825417db52176f9165db34d7234a05a05
  10. Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4886249
  11. Reviewed-by: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
  12. Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
  13. Reviewed-by: Maksim Sisov <msisov@igalia.com>
  14. Cr-Commit-Position: refs/heads/main@{#1200486}
  15. ---
  16. .../platform/x11/native_pixmap_egl_x11_binding.cc | 12 +++++++++---
  17. .../platform/x11/native_pixmap_egl_x11_binding.h | 2 ++
  18. 2 files changed, 11 insertions(+), 3 deletions(-)
  19. diff --git a/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.cc b/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.cc
  20. index 46a2d3274b9..b46eb67b9de 100644
  21. --- a/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.cc
  22. +++ b/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.cc
  23. @@ -147,9 +147,17 @@ NativePixmapEGLX11Binding::~NativePixmapEGLX11Binding() {
  24. if (surface_) {
  25. eglDestroySurface(display_, surface_);
  26. }
  27. +
  28. + if (pixmap_ != x11::Pixmap::None) {
  29. + auto* connection = x11::Connection::Get();
  30. + connection->FreePixmap({pixmap_});
  31. + }
  32. }
  33. bool NativePixmapEGLX11Binding::Initialize(x11::Pixmap pixmap) {
  34. + CHECK_NE(pixmap, x11::Pixmap::None);
  35. + pixmap_ = pixmap;
  36. +
  37. if (eglInitialize(display_, nullptr, nullptr) != EGL_TRUE) {
  38. return false;
  39. }
  40. @@ -223,9 +231,7 @@ std::unique_ptr<NativePixmapGLBinding> NativePixmapEGLX11Binding::Create(
  41. return nullptr;
  42. }
  43. - // TODO(https://crbug.com/1411749): if we early out below, should we call
  44. - // FreePixmap()?
  45. -
  46. + // Transfer the ownership of `pixmap` to `NativePixmapEGLX11Binding`.
  47. if (!binding->Initialize(std::move(pixmap))) {
  48. VLOG(1) << "Unable to initialize binding from pixmap";
  49. return nullptr;
  50. diff --git a/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.h b/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.h
  51. index 013df3f776a..99b262b82ce 100644
  52. --- a/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.h
  53. +++ b/ui/ozone/platform/x11/native_pixmap_egl_x11_binding.h
  54. @@ -47,6 +47,8 @@ class NativePixmapEGLX11Binding : public NativePixmapGLBinding {
  55. EGLSurface surface_ = nullptr;
  56. EGLDisplay display_;
  57. gfx::BufferFormat format_;
  58. +
  59. + x11::Pixmap pixmap_ = x11::Pixmap::None;
  60. };
  61. } // namespace ui