Bug 1780389 [Wayland] Call moz_container_wayland_add_initial_draw_callback() on locked container, always lock MozContainer for access r=rmader

With this patch moz_container_wayland_surface_lock() always locks MozContainer and needs to be paired with moz_container_wayland_surface_unlock() even if it fails and returns nullptr.
Split moz_container_wayland_add_initial_draw_callback() to two new functions:

- moz_container_wayland_add_initial_draw_callback_locked() is called on locked container and only adds draw callback.
  It asserts when MozContainer is already to draw as we don't expect it.

- moz_container_wayland_add_or_fire_initial_draw_callback() is called on unlocked container as it has it's own lock.
  It behaves as original moz_container_wayland_add_initial_draw_callback(), i.e. stores draw callback when MosContainer is not visible
  and fires draw callback when we're ready to draw.

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

gfx/layers/NativeLayerWayland.cpp

@@ -17,6 +17,7 @@
 #include "mozilla/layers/SurfacePoolWayland.h"
 #include "mozilla/StaticPrefs_widget.h"
 #include "mozilla/webrender/RenderThread.h"
+#include "mozilla/ScopeExit.h"
 
 namespace mozilla::layers {
 
@@ -149,10 +150,13 @@ bool NativeLayerRootWayland::CommitToScreen(const MutexAutoLock& aProofOfLock) {
   mFrameInProcess = false;
 
   wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
+  auto mozContainerUnlock = MakeScopeExit([&] {
+    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
+  });
   if (!containerSurface) {
     if (!mCallbackRequested) {
       RefPtr<NativeLayerRootWayland> self(this);
-      moz_container_wayland_add_initial_draw_callback(
+      moz_container_wayland_add_initial_draw_callback_locked(
           mContainer, [self]() -> void {
             MutexAutoLock lock(self->mMutex);
             if (!self->mFrameInProcess) {
@@ -240,7 +244,6 @@ bool NativeLayerRootWayland::CommitToScreen(const MutexAutoLock& aProofOfLock) {
     wl_surface_commit(containerSurface);
   }
 
-  moz_container_wayland_surface_unlock(mContainer, &containerSurface);
   wl_display_flush(widget::WaylandDisplayGet()->GetDisplay());
   return true;
 }
@@ -257,10 +260,11 @@ void NativeLayerRootWayland::RequestFrameCallback(CallbackFunc aCallbackFunc,
   }
 
   wl_surface* wlSurface = moz_container_wayland_surface_lock(mContainer);
+  auto mozContainerUnlock = MakeScopeExit(
+      [&] { moz_container_wayland_surface_unlock(mContainer, &wlSurface); });
   if (wlSurface) {
     wl_surface_commit(wlSurface);
     wl_display_flush(widget::WaylandDisplayGet()->GetDisplay());
-    moz_container_wayland_surface_unlock(mContainer, &wlSurface);
   }
 }
 
@@ -272,13 +276,15 @@ static void sAfterFrameClockAfterPaint(
 void NativeLayerRootWayland::AfterFrameClockAfterPaint() {
   MutexAutoLock lock(mMutex);
   wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
+  auto mozContainerUnlock = MakeScopeExit([&] {
+    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
+  });
 
   for (const RefPtr<NativeLayerWayland>& layer : mSublayersOnMainThread) {
     wl_surface_commit(layer->mWlSurface);
   }
   if (containerSurface) {
     wl_surface_commit(containerSurface);
-    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
   }
 }
 
@@ -294,6 +300,9 @@ void NativeLayerRootWayland::UpdateLayersOnMainThread() {
           RTLD_DEFAULT, "gdk_wayland_window_remove_frame_callback_surface");
 
   wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
+  auto mozContainerUnlock = MakeScopeExit([&] {
+    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
+  });
   GdkWindow* gdkWindow = gtk_widget_get_window(GTK_WIDGET(mContainer));
 
   mSublayersOnMainThread.RemoveElementsBy([&](const auto& layer) {
@@ -339,7 +348,6 @@ void NativeLayerRootWayland::UpdateLayersOnMainThread() {
 
   if (containerSurface) {
     wl_surface_commit(containerSurface);
-    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
   }
 
   if (!mGdkAfterPaintId && gdkWindow) {

widget/gtk/MozContainerWayland.cpp

@@ -194,7 +194,20 @@ static void moz_container_wayland_destroy(GtkWidget* widget) {
   container->container_lock = nullptr;
 }
 
-void moz_container_wayland_add_initial_draw_callback(
+void moz_container_wayland_add_initial_draw_callback_locked(
+    MozContainer* container, const std::function<void(void)>& initial_draw_cb) {
+  MozContainerWayland* wl_container = &MOZ_CONTAINER(container)->wl_container;
+
+  if (wl_container->ready_to_draw && !wl_container->surface) {
+    NS_WARNING(
+        "moz_container_wayland_add_or_fire_initial_draw_callback:"
+        " ready to draw without wayland surface!");
+  }
+  MOZ_DIAGNOSTIC_ASSERT(!wl_container->ready_to_draw || !wl_container->surface);
+  wl_container->initial_draw_cbs.push_back(initial_draw_cb);
+}
+
+void moz_container_wayland_add_or_fire_initial_draw_callback(
     MozContainer* container, const std::function<void(void)>& initial_draw_cb) {
   MozContainerWayland* wl_container = &MOZ_CONTAINER(container)->wl_container;
   {
@@ -262,7 +275,7 @@ static void moz_container_wayland_frame_callback_handler(
   }
 
   // Call the callbacks registered by
-  // moz_container_wayland_add_initial_draw_callback().
+  // moz_container_wayland_add_or_fire_initial_draw_callback().
   // and we can't do that under mozcontainer lock.
   for (auto const& cb : cbs) {
     cb();
@@ -275,9 +288,10 @@ static const struct wl_callback_listener moz_container_frame_listener = {
 static void after_frame_clock_after_paint(GdkFrameClock* clock,
                                           MozContainer* container) {
   struct wl_surface* surface = moz_container_wayland_surface_lock(container);
+  auto mozContainerUnlock = MakeScopeExit(
+      [&] { moz_container_wayland_surface_unlock(container, &surface); });
   if (surface) {
     wl_surface_commit(surface);
-    moz_container_wayland_surface_unlock(container, &surface);
   }
 }
 
@@ -628,11 +642,11 @@ struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container)
   // LOGWAYLAND("%s [%p] surface %p ready_to_draw %d\n", __FUNCTION__,
   //           (void*)container, (void*)container->wl_container.surface,
   //           container->wl_container.ready_to_draw);
+  container->wl_container.container_lock->Lock();
   if (!container->wl_container.surface ||
       !container->wl_container.ready_to_draw) {
     return nullptr;
   }
-  container->wl_container.container_lock->Lock();
   return container->wl_container.surface;
 }
 
@@ -643,9 +657,9 @@ void moz_container_wayland_surface_unlock(MozContainer* container,
   // LOGWAYLAND("%s [%p] surface %p\n", __FUNCTION__, (void*)container,
   //            (void*)container->wl_container.surface);
   if (*surface) {
-    container->wl_container.container_lock->Unlock();
     *surface = nullptr;
   }
+  container->wl_container.container_lock->Unlock();
 }
 
 struct wl_surface* moz_container_wayland_get_surface_locked(

widget/gtk/MozContainerWayland.h

@@ -61,6 +61,9 @@ typedef struct _MozContainerClass MozContainerClass;
 void moz_container_wayland_class_init(MozContainerClass* klass);
 void moz_container_wayland_init(MozContainerWayland* container);
 
+// Lock mozcontainer and get wayland surface of it. You need to pair with
+// moz_container_wayland_surface_unlock() even
+// if moz_container_wayland_surface_lock() fails and returns nullptr.
 struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container);
 void moz_container_wayland_surface_unlock(MozContainer* container,
                                           struct wl_surface** surface);
@@ -78,9 +81,13 @@ void moz_container_wayland_egl_window_set_size(MozContainer* container,
                                                int width, int height);
 void moz_container_wayland_set_scale_factor(MozContainer* container);
 void moz_container_wayland_set_scale_factor_locked(MozContainer* container);
-void moz_container_wayland_add_initial_draw_callback(
+
+void moz_container_wayland_add_initial_draw_callback_locked(
+    MozContainer* container, const std::function<void(void)>& initial_draw_cb);
+void moz_container_wayland_add_or_fire_initial_draw_callback(
     MozContainer* container, const std::function<void(void)>& initial_draw_cb);
 void moz_container_wayland_clear_initial_draw_callback(MozContainer* container);
+
 wl_surface* moz_gtk_widget_get_wl_surface(GtkWidget* aWidget);
 void moz_container_wayland_update_opaque_region(MozContainer* container,
                                                 int corner_radius);

widget/gtk/WakeLockListener.cpp

@@ -281,10 +281,12 @@ bool WakeLockTopic::InhibitWaylandIdle() {
 
   MozContainer* container = focusedWindow->GetMozContainer();
   wl_surface* waylandSurface = moz_container_wayland_surface_lock(container);
+  auto mozContainerUnlock = MakeScopeExit([&] {
+    moz_container_wayland_surface_unlock(container, &waylandSurface);
+  });
   if (waylandSurface) {
     mWaylandInhibitor = zwp_idle_inhibit_manager_v1_create_inhibitor(
         waylandDisplay->GetIdleInhibitManager(), waylandSurface);
-    moz_container_wayland_surface_unlock(container, &waylandSurface);
   }
   return true;
 }

widget/gtk/WaylandVsyncSource.cpp

@@ -11,6 +11,7 @@
 #  include "nsThreadUtils.h"
 #  include "nsISupportsImpl.h"
 #  include "MainThreadUtils.h"
+#  include "mozilla/ScopeExit.h"
 
 #  include <gdk/gdkwayland.h>
 
@@ -148,20 +149,21 @@ void WaylandVsyncSource::Refresh(const MutexAutoLock& aProofOfLock) {
 
   if (mContainer) {
     struct wl_surface* surface = moz_container_wayland_surface_lock(mContainer);
+    auto mozContainerUnlock = MakeScopeExit(
+        [&] { moz_container_wayland_surface_unlock(mContainer, &surface); });
     LOG("  refresh from mContainer, wl_surface %p", surface);
     if (!surface) {
       LOG("  we're missing wl_surface, register Refresh() callback");
       // The surface hasn't been created yet. Try again when the surface is
       // ready.
       RefPtr<WaylandVsyncSource> self(this);
-      moz_container_wayland_add_initial_draw_callback(
+      moz_container_wayland_add_initial_draw_callback_locked(
           mContainer, [self]() -> void {
             MutexAutoLock lock(self->mMutex);
             self->Refresh(lock);
           });
       return;
     }
-    moz_container_wayland_surface_unlock(mContainer, &surface);
   }
 
   // Vsync is enabled, but we don't have a callback configured. Set one up so
@@ -207,6 +209,8 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) {
                                            this);
   } else {
     struct wl_surface* surface = moz_container_wayland_surface_lock(mContainer);
+    auto mozContainerUnlock = MakeScopeExit(
+        [&] { moz_container_wayland_surface_unlock(mContainer, &surface); });
     LOG("  use mContainer, wl_surface %p", surface);
     if (!surface) {
       // We don't have a surface, either due to being called before it was made
@@ -222,7 +226,6 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) {
                              this);
     wl_surface_commit(surface);
     wl_display_flush(WaylandDisplayGet()->GetDisplay());
-    moz_container_wayland_surface_unlock(mContainer, &surface);
   }
 
   mCallbackRequested = true;

widget/gtk/WindowSurfaceWaylandMultiBuffer.cpp

@@ -267,6 +267,9 @@ void WindowSurfaceWaylandMB::Commit(
 
   MozContainer* container = mWindow->GetMozContainer();
   wl_surface* waylandSurface = moz_container_wayland_surface_lock(container);
+  auto mozContainerUnlock = MakeScopeExit([&] {
+    moz_container_wayland_surface_unlock(container, &waylandSurface);
+  });
   if (!waylandSurface) {
     LOGWAYLAND(
         "WindowSurfaceWaylandMB::Commit [%p] frame queued: can't lock "
@@ -274,7 +277,7 @@ void WindowSurfaceWaylandMB::Commit(
         (void*)mWindow.get());
     if (!mCallbackRequested) {
       RefPtr<WindowSurfaceWaylandMB> self(this);
-      moz_container_wayland_add_initial_draw_callback(
+      moz_container_wayland_add_initial_draw_callback_locked(
           container, [self, aInvalidRegion]() -> void {
             MutexAutoLock lock(self->mSurfaceLock);
             if (!self->mFrameInProcess) {

widget/gtk/nsWindow.cpp

@@ -5313,7 +5313,7 @@ void nsWindow::EnableRenderingToWindow() {
 
   if (GdkIsWaylandDisplay()) {
 #ifdef MOZ_WAYLAND
-    moz_container_wayland_add_initial_draw_callback(
+    moz_container_wayland_add_or_fire_initial_draw_callback(
         mContainer, [self = RefPtr{this}, this]() -> void {
           LOG("moz_container_wayland initial create "
               "ResumeCompositorHiddenWindow()");