Bug 1662425 [Wayland] Create nsWaylandDisplay as ref counted, r=jhorak

- Create nsWaylandDisplay as a ref-counted struct to avoid potential delete when a display is used.
- Don't use Mutex to access all display array operations. Use only write mutex when the array is modified / released.
- Store all wayland displays in nsTArray instead of fixed plain C array.
- Release all displays before we close Gtk display to make sure we quit before Gtk connections are closed.

Differential Revision: https://phabricator.services.mozilla.com/D89188

toolkit/xre/nsAppRunner.cpp

@@ -298,6 +298,7 @@ nsString gAbsoluteArgv0Path;
 #  include <gtk/gtk.h>
 #  ifdef MOZ_WAYLAND
 #    include <gdk/gdkwayland.h>
+#    include "mozilla/widget/nsWaylandDisplay.h"
 #  endif
 #  ifdef MOZ_X11
 #    include <gdk/gdkx.h>
@@ -334,6 +335,7 @@ bool RunningGTest() { return RunGTest; }
 }  // namespace mozilla
 
 using namespace mozilla;
+using namespace mozilla::widget;
 using namespace mozilla::startup;
 using mozilla::Unused;
 using mozilla::dom::ContentChild;
@@ -4975,6 +4977,9 @@ int XREMain::XRE_main(int argc, char* argv[], const BootstrapConfig& aConfig) {
   // gdk_display_close also calls gdk_display_manager_set_default_display
   // appropriately when necessary.
   if (!gfxPlatform::IsHeadless()) {
+#  ifdef MOZ_WAYLAND
+    WaylandDisplayRelease();
+#  endif
     MOZ_gdk_display_close(mGdkDisplay);
   }
 #endif

widget/gtk/MozContainerWayland.cpp

@@ -431,7 +431,7 @@ static struct wl_surface* moz_container_wayland_get_surface_locked(
 
 struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container) {
   GdkDisplay* display = gtk_widget_get_display(GTK_WIDGET(container));
-  nsWaylandDisplay* waylandDisplay = WaylandDisplayGet(display);
+  RefPtr<nsWaylandDisplay> waylandDisplay = WaylandDisplayGet(display);
 
   LOGWAYLAND(("%s [%p] surface %p\n", __FUNCTION__, (void*)container,
               (void*)container->wl_container.surface));
@@ -458,7 +458,7 @@ void moz_container_wayland_surface_unlock(MozContainer* container,
 struct wl_egl_window* moz_container_wayland_get_egl_window(
     MozContainer* container, int scale) {
   GdkDisplay* display = gtk_widget_get_display(GTK_WIDGET(container));
-  nsWaylandDisplay* waylandDisplay = WaylandDisplayGet(display);
+  RefPtr<nsWaylandDisplay> waylandDisplay = WaylandDisplayGet(display);
   MozContainerWayland* wl_container = &container->wl_container;
 
   LOGWAYLAND(("%s [%p] eglwindow %p\n", __FUNCTION__, (void*)container,

widget/gtk/WakeLockListener.cpp

@@ -236,12 +236,12 @@ bool WakeLockTopic::InhibitXScreenSaver(bool inhibit) {
 
 /* static */
 bool WakeLockTopic::CheckWaylandIdleInhibitSupport() {
-  nsWaylandDisplay* waylandDisplay = WaylandDisplayGet();
+  RefPtr<nsWaylandDisplay> waylandDisplay = WaylandDisplayGet();
   return waylandDisplay && waylandDisplay->GetIdleInhibitManager() != nullptr;
 }
 
 bool WakeLockTopic::InhibitWaylandIdle() {
-  nsWaylandDisplay* waylandDisplay = WaylandDisplayGet();
+  RefPtr<nsWaylandDisplay> waylandDisplay = WaylandDisplayGet();
   if (!waylandDisplay) {
     return false;
   }

widget/gtk/WaylandVsyncSource.cpp

@@ -13,6 +13,8 @@
 
 #  include <gdk/gdkwayland.h>
 
+using namespace mozilla::widget;
+
 namespace mozilla {
 
 static void WaylandVsyncSourceCallbackHandler(void* data,
@@ -37,7 +39,7 @@ WaylandVsyncSource::WaylandDisplay::WaylandDisplay(MozContainer* container)
 
   // We store the display here so all the frame callbacks won't have to look it
   // up all the time.
-  mDisplay = widget::WaylandDisplayGet()->GetDisplay();
+  mDisplay = WaylandDisplayGetWLDisplay();
 }
 
 void WaylandVsyncSource::WaylandDisplay::ClearFrameCallback() {

widget/gtk/WindowSurfaceWayland.cpp

@@ -163,7 +163,7 @@ handle to wayland compositor by WindowBackBuffer/WindowSurfaceWayland
 #define BUFFER_BPP 4
 gfx::SurfaceFormat WindowBackBuffer::mFormat = gfx::SurfaceFormat::B8G8R8A8;
 
-nsWaylandDisplay* WindowBackBuffer::GetWaylandDisplay() {
+RefPtr<nsWaylandDisplay> WindowBackBuffer::GetWaylandDisplay() {
   return mWindowSurfaceWayland->GetWaylandDisplay();
 }
 
@@ -200,7 +200,8 @@ static int WaylandAllocateShmMemory(int aSize) {
   return fd;
 }
 
-WaylandShmPool::WaylandShmPool(nsWaylandDisplay* aWaylandDisplay, int aSize)
+WaylandShmPool::WaylandShmPool(RefPtr<nsWaylandDisplay> aWaylandDisplay,
+                               int aSize)
     : mAllocatedSize(aSize) {
   mShmPoolFd = WaylandAllocateShmMemory(mAllocatedSize);
   mImageData = mmap(nullptr, mAllocatedSize, PROT_READ | PROT_WRITE, MAP_SHARED,

widget/gtk/WindowSurfaceWayland.h

@@ -25,7 +25,7 @@ class WindowSurfaceWayland;
 // Allocates and owns shared memory for Wayland drawing surface
 class WaylandShmPool {
  public:
-  WaylandShmPool(nsWaylandDisplay* aDisplay, int aSize);
+  WaylandShmPool(RefPtr<nsWaylandDisplay> aDisplay, int aSize);
   ~WaylandShmPool();
 
   bool Resize(int aSize);
@@ -72,7 +72,7 @@ class WindowBackBuffer {
 
   static gfx::SurfaceFormat GetSurfaceFormat() { return mFormat; }
 
-  nsWaylandDisplay* GetWaylandDisplay();
+  RefPtr<nsWaylandDisplay> GetWaylandDisplay();
 
   WindowBackBuffer(WindowSurfaceWayland* aWindowSurfaceWayland)
       : mWindowSurfaceWayland(aWindowSurfaceWayland){};
@@ -183,7 +183,7 @@ class WindowSurfaceWayland : public WindowSurface {
   // (see WindowBackBufferShm::Detach() for instance).
   void CommitWaylandBuffer();
 
-  nsWaylandDisplay* GetWaylandDisplay() { return mWaylandDisplay; };
+  RefPtr<nsWaylandDisplay> GetWaylandDisplay() { return mWaylandDisplay; };
 
   // Image cache mode can be set by widget.wayland_cache_mode
   typedef enum {
@@ -229,7 +229,7 @@ class WindowSurfaceWayland : public WindowSurface {
   // during resize when mBounds are updated immediately but actual
   // GtkWidget size is updated asynchronously (see Bug 1489463).
   LayoutDeviceIntRect mWLBufferRect;
-  nsWaylandDisplay* mWaylandDisplay;
+  RefPtr<nsWaylandDisplay> mWaylandDisplay;
 
   // Actual buffer (backed by wl_buffer) where all drawings go into.
   // Drawn areas are stored at mWaylandBufferDamage and if there's

widget/gtk/nsClipboardWayland.cpp

@@ -29,6 +29,9 @@
 #include <fcntl.h>
 #include <errno.h>
 
+using namespace mozilla;
+using namespace mozilla::widget;
+
 const char* nsRetrievalContextWayland::sTextMimeTypes[TEXT_MIME_TYPES_NUM] = {
     "text/plain;charset=utf-8", "UTF8_STRING", "COMPOUND_TEXT"};
 
@@ -658,7 +661,7 @@ bool nsRetrievalContextWayland::HasSelectionSupport(void) {
 
 nsRetrievalContextWayland::nsRetrievalContextWayland(void)
     : mInitialized(false),
-      mDisplay(mozilla::widget::WaylandDisplayGet()),
+      mDisplay(WaylandDisplayGet()),
       mActiveOffers(g_hash_table_new(NULL, NULL)),
       mClipboardOffer(nullptr),
       mPrimaryOffer(nullptr),

widget/gtk/nsClipboardWayland.h

@@ -139,7 +139,7 @@ class nsRetrievalContextWayland : public nsRetrievalContext {
 
  private:
   bool mInitialized;
-  mozilla::widget::nsWaylandDisplay* mDisplay;
+  RefPtr<mozilla::widget::nsWaylandDisplay> mDisplay;
 
   // Data offers provided by Wayland data device
   GHashTable* mActiveOffers;

widget/gtk/nsWaylandDisplay.cpp

@@ -23,16 +23,13 @@ wl_display* WaylandDisplayGetWLDisplay(GdkDisplay* aGdkDisplay) {
   return gdk_wayland_display_get_wl_display(aGdkDisplay);
 }
 
-// nsWaylandDisplay needs to be created for each calling thread(main thread,
-// compositor thread and render thread)
-#define MAX_DISPLAY_CONNECTIONS 5
-
-static nsWaylandDisplay* gWaylandDisplays[MAX_DISPLAY_CONNECTIONS];
-static StaticMutex gWaylandDisplayArrayMutex;
-static StaticMutex gWaylandThreadLoopMutex;
+// An array of active wayland displays. We need a display for every thread
+// where is wayland interface used as we need to dispatch waylands events
+// there.
+static nsTArray<RefPtr<nsWaylandDisplay>> gWaylandDisplays;
+static StaticMutex gWaylandDisplayArrayWriteMutex;
 
 void WaylandDisplayShutdown() {
-  StaticMutexAutoLock lock(gWaylandDisplayArrayMutex);
   for (auto& display : gWaylandDisplays) {
     if (display) {
       display->ShutdownThreadLoop();
@@ -40,16 +37,17 @@ void WaylandDisplayShutdown() {
   }
 }
 
-static void ReleaseDisplaysAtExit() {
-  StaticMutexAutoLock lock(gWaylandDisplayArrayMutex);
-  for (int i = 0; i < MAX_DISPLAY_CONNECTIONS; i++) {
-    delete gWaylandDisplays[i];
-    gWaylandDisplays[i] = nullptr;
+static void DispatchDisplay(RefPtr<nsWaylandDisplay> aDisplay) {
+  // We can't use aDisplay directly here as it can be already released.
+  // Instead look for aDisplay in gWaylandDisplays and dispatch it only when
+  // it's still active.
+  for (auto& display : gWaylandDisplays) {
+    if (display == aDisplay) {
+      aDisplay->DispatchEventQueue();
+      return;
+    }
   }
-}
-
-static void DispatchDisplay(nsWaylandDisplay* aDisplay) {
-  aDisplay->DispatchEventQueue();
+  NS_WARNING("DispatchDisplay was called for released display!");
 }
 
 // Each thread which is using wayland connection (wl_display) has to operate
@@ -60,10 +58,8 @@ static void DispatchDisplay(nsWaylandDisplay* aDisplay) {
 // global objects as we need (wl_display, wl_shm) and operates wl_event_queue on
 // compositor (not the main) thread.
 void WaylandDispatchDisplays() {
-  StaticMutexAutoLock arrayLock(gWaylandDisplayArrayMutex);
   for (auto& display : gWaylandDisplays) {
     if (display) {
-      StaticMutexAutoLock loopLock(gWaylandThreadLoopMutex);
       MessageLoop* loop = display->GetThreadLoop();
       if (loop) {
         loop->PostTask(NewRunnableFunction("WaylandDisplayDispatch",
@@ -73,10 +69,17 @@ void WaylandDispatchDisplays() {
   }
 }
 
+void WaylandDisplayRelease() {
+  StaticMutexAutoLock lock(gWaylandDisplayArrayWriteMutex);
+  gWaylandDisplays.Clear();
+}
+
 // Get WaylandDisplay for given wl_display and actual calling thread.
-static nsWaylandDisplay* WaylandDisplayGetLocked(GdkDisplay* aGdkDisplay,
-                                                 const StaticMutexAutoLock&) {
+RefPtr<nsWaylandDisplay> WaylandDisplayGet(GdkDisplay* aGdkDisplay) {
   wl_display* waylandDisplay = WaylandDisplayGetWLDisplay(aGdkDisplay);
+  if (!waylandDisplay) {
+    return nullptr;
+  }
 
   // Search existing display connections for wl_display:thread combination.
   for (auto& display : gWaylandDisplays) {
@@ -85,28 +88,9 @@ static nsWaylandDisplay* WaylandDisplayGetLocked(GdkDisplay* aGdkDisplay,
     }
   }
 
-  for (auto& display : gWaylandDisplays) {
-    if (display == nullptr) {
-      display = new nsWaylandDisplay(waylandDisplay);
-      atexit(ReleaseDisplaysAtExit);
-      return display;
-    }
-  }
-
-  MOZ_CRASH("There's too many wayland display conections!");
-  return nullptr;
-}
-
-nsWaylandDisplay* WaylandDisplayGet(GdkDisplay* aGdkDisplay) {
-  if (!aGdkDisplay) {
-    aGdkDisplay = gdk_display_get_default();
-    if (!aGdkDisplay || GDK_IS_X11_DISPLAY(aGdkDisplay)) {
-      return nullptr;
-    }
-  }
-
-  StaticMutexAutoLock lock(gWaylandDisplayArrayMutex);
-  return WaylandDisplayGetLocked(aGdkDisplay, lock);
+  StaticMutexAutoLock arrayLock(gWaylandDisplayArrayWriteMutex);
+  gWaylandDisplays.AppendElement(new nsWaylandDisplay(waylandDisplay));
+  return gWaylandDisplays[gWaylandDisplays.Length() - 1];
 }
 
 void nsWaylandDisplay::SetShm(wl_shm* aShm) { mShm = aShm; }
@@ -358,10 +342,7 @@ nsWaylandDisplay::nsWaylandDisplay(wl_display* aDisplay, bool aLighWrapper)
   }
 }
 
-void nsWaylandDisplay::ShutdownThreadLoop() {
-  StaticMutexAutoLock lock(gWaylandThreadLoopMutex);
-  mThreadLoop = nullptr;
-}
+void nsWaylandDisplay::ShutdownThreadLoop() { mThreadLoop = nullptr; }
 
 nsWaylandDisplay::~nsWaylandDisplay() {
   wl_registry_destroy(mRegistry);

widget/gtk/nsWaylandDisplay.h

@@ -24,15 +24,15 @@ namespace widget {
 
 // Our general connection to Wayland display server,
 // holds our display connection and runs event loop.
-// We have a global nsWaylandDisplay object for each thread,
-// recently we have three for main, compositor and render one.
+// We have a global nsWaylandDisplay object for each thread.
 class nsWaylandDisplay {
  public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(nsWaylandDisplay)
+
   // Create nsWaylandDisplay object on top of native Wayland wl_display
   // connection. When aLighWrapper is set we don't get wayland registry
   // objects and only event loop is provided.
   explicit nsWaylandDisplay(wl_display* aDisplay, bool aLighWrapper = false);
-  virtual ~nsWaylandDisplay();
 
   bool DispatchEventQueue();
 
@@ -77,6 +77,8 @@ class nsWaylandDisplay {
   bool IsExplicitSyncEnabled() { return mExplicitSync; }
 
  private:
+  ~nsWaylandDisplay();
+
   MessageLoop* mThreadLoop;
   PRThread* mThreadId;
   wl_display* mDisplay;
@@ -96,7 +98,9 @@ class nsWaylandDisplay {
 
 void WaylandDispatchDisplays();
 void WaylandDisplayShutdown();
-nsWaylandDisplay* WaylandDisplayGet(GdkDisplay* aGdkDisplay = nullptr);
+void WaylandDisplayRelease();
+
+RefPtr<nsWaylandDisplay> WaylandDisplayGet(GdkDisplay* aGdkDisplay = nullptr);
 wl_display* WaylandDisplayGetWLDisplay(GdkDisplay* aGdkDisplay = nullptr);
 
 }  // namespace widget