Bug 1780389 [Wayland] Implement RAII class MozContainerSurfaceLock to get&lock wl_surface of moz_container 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. - implement RAII class MozContainerSurfaceLock and use it to lock moz_contatier and get wl_surface of it. Differential Revision: https://phabricator.services.mozilla.com/D152276
2022-08-15 19:35:40 +00:00 · 2022-08-15 19:35:40 +00:00 · 616c7c35ee
--- a/gfx/layers/NativeLayerWayland.cpp
+++ b/gfx/layers/NativeLayerWayland.cpp
@ -148,11 +148,12 @@ bool NativeLayerRootWayland::CommitToScreen() {
 bool NativeLayerRootWayland::CommitToScreen(const MutexAutoLock& aProofOfLock) {
  mFrameInProcess = false;

-  wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
+  MozContainerSurfaceLock lock(mContainer);
+  struct wl_surface* containerSurface = lock.GetSurface();
  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 +241,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;
 }
@ -256,11 +256,11 @@ void NativeLayerRootWayland::RequestFrameCallback(CallbackFunc aCallbackFunc,
    layer->RequestFrameCallback(mCallbackMultiplexHelper);
  }

-  wl_surface* wlSurface = moz_container_wayland_surface_lock(mContainer);
+  MozContainerSurfaceLock lockContainer(mContainer);
+  struct wl_surface* wlSurface = lockContainer.GetSurface();
  if (wlSurface) {
    wl_surface_commit(wlSurface);
    wl_display_flush(widget::WaylandDisplayGet()->GetDisplay());
-    moz_container_wayland_surface_unlock(mContainer, &wlSurface);
  }
 }

@ -271,14 +271,13 @@ static void sAfterFrameClockAfterPaint(

 void NativeLayerRootWayland::AfterFrameClockAfterPaint() {
  MutexAutoLock lock(mMutex);
-  wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
-
+  MozContainerSurfaceLock lockContainer(mContainer);
+  struct wl_surface* containerSurface = lockContainer.GetSurface();
  for (const RefPtr<NativeLayerWayland>& layer : mSublayersOnMainThread) {
    wl_surface_commit(layer->mWlSurface);
  }
  if (containerSurface) {
    wl_surface_commit(containerSurface);
-    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
  }
 }

@ -293,7 +292,8 @@ void NativeLayerRootWayland::UpdateLayersOnMainThread() {
      (void (*)(GdkWindow*, struct wl_surface*))dlsym(
          RTLD_DEFAULT, "gdk_wayland_window_remove_frame_callback_surface");

-  wl_surface* containerSurface = moz_container_wayland_surface_lock(mContainer);
+  MozContainerSurfaceLock lockContainer(mContainer);
+  struct wl_surface* containerSurface = lockContainer.GetSurface();
  GdkWindow* gdkWindow = gtk_widget_get_window(GTK_WIDGET(mContainer));

  mSublayersOnMainThread.RemoveElementsBy([&](const auto& layer) {
@ -339,7 +339,6 @@ void NativeLayerRootWayland::UpdateLayersOnMainThread() {

  if (containerSurface) {
    wl_surface_commit(containerSurface);
-    moz_container_wayland_surface_unlock(mContainer, &containerSurface);
  }

  if (!mGdkAfterPaintId && gdkWindow) {
--- a/widget/gtk/MozContainerWayland.cpp
+++ b/widget/gtk/MozContainerWayland.cpp
@ -96,6 +96,25 @@ static bool moz_container_wayland_surface_create_locked(
 static void moz_container_wayland_set_opaque_region_locked(
    const MutexAutoLock& aProofOfLock, MozContainer* 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.
+static struct wl_surface* moz_container_wayland_surface_lock(
+    MozContainer* container);
+static void moz_container_wayland_surface_unlock(MozContainer* container,
+                                                 struct wl_surface** surface);
+
+MozContainerSurfaceLock::MozContainerSurfaceLock(MozContainer* aContainer) {
+  mContainer = aContainer;
+  mSurface = moz_container_wayland_surface_lock(aContainer);
+}
+MozContainerSurfaceLock::~MozContainerSurfaceLock() {
+  moz_container_wayland_surface_unlock(mContainer, &mSurface);
+}
+struct wl_surface* MozContainerSurfaceLock::GetSurface() {
+  return mSurface;
+}
+
 // Imlemented in MozContainer.cpp
 void moz_container_realize(GtkWidget* widget);

@ -199,14 +218,28 @@ 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;
  {
    MutexAutoLock lock(*container->wl_container.container_lock);
    if (wl_container->ready_to_draw && !wl_container->surface) {
      NS_WARNING(
-          "moz_container_wayland_add_initial_draw_callback: ready to draw "
+          "moz_container_wayland_add_or_fire_initial_draw_callback: ready to "
+          "draw "
          "without wayland surface!");
    }
    if (!wl_container->ready_to_draw || !wl_container->surface) {
@ -267,7 +300,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();
@ -279,10 +312,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);
+  MozContainerSurfaceLock lock(container);
+  struct wl_surface* surface = lock.GetSurface();
  if (surface) {
    wl_surface_commit(surface);
-    moz_container_wayland_surface_unlock(container, &surface);
  }
 }

@ -368,19 +401,21 @@ static gboolean moz_container_wayland_map_event(GtkWidget* widget,
  gtk_widget_set_mapped(widget, TRUE);

  // Make sure we're on main thread as we can't lock mozContainer here
-  // due to moz_container_wayland_add_initial_draw_callback() call below.
+  // due to moz_container_wayland_add_or_fire_initial_draw_callback() call
+  // below.
  MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread());

  // Set waiting_to_show flag. It means the mozcontainer is cofigured/mapped
  // and it's supposed to be visible. *But* it's really visible when we get
-  // moz_container_wayland_add_initial_draw_callback() which means
+  // moz_container_wayland_add_or_fire_initial_draw_callback() which means
  // wayland compositor makes it live.
  wl_container->waiting_to_show = true;
  MozContainer* container = MOZ_CONTAINER(widget);
-  moz_container_wayland_add_initial_draw_callback(
+  moz_container_wayland_add_or_fire_initial_draw_callback(
      container, [container]() -> void {
        LOGCONTAINER(
-            "[%p] moz_container_wayland_add_initial_draw_callback set visible",
+            "[%p] moz_container_wayland_add_or_fire_initial_draw_callback set "
+            "visible",
            moz_container_get_nsWindow(container));
        moz_container_wayland_clear_waiting_to_show_flag(container);
      });
@ -652,11 +687,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;
 }

@ -667,32 +702,8 @@ 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;
  }
-}
-
-struct wl_surface* moz_container_wayland_get_surface_locked(
-    const MutexAutoLock& aProofOfLock, MozContainer* container) {
-  LOGWAYLAND("%s [%p] surface %p ready_to_draw %d\n", __FUNCTION__,
-             (void*)moz_container_get_nsWindow(container),
-             (void*)container->wl_container.surface,
-             container->wl_container.ready_to_draw);
-  if (!container->wl_container.surface ||
-      !container->wl_container.ready_to_draw) {
-    return nullptr;
-  }
-  moz_container_wayland_set_scale_factor_locked(container);
-  return container->wl_container.surface;
-}
-
-void moz_container_wayland_lock(MozContainer* container)
-    MOZ_NO_THREAD_SAFETY_ANALYSIS {
-  container->wl_container.container_lock->Lock();
-}
-
-void moz_container_wayland_unlock(MozContainer* container)
-    MOZ_NO_THREAD_SAFETY_ANALYSIS {
  container->wl_container.container_lock->Unlock();
 }

--- a/widget/gtk/MozContainerWayland.h
+++ b/widget/gtk/MozContainerWayland.h
@ -59,18 +59,19 @@ struct _MozContainerClass;
 typedef struct _MozContainer MozContainer;
 typedef struct _MozContainerClass MozContainerClass;

+class MozContainerSurfaceLock {
+  MozContainer* mContainer;
+  struct wl_surface* mSurface;
+
+ public:
+  MozContainerSurfaceLock(MozContainer* aContainer);
+  ~MozContainerSurfaceLock();
+  struct wl_surface* GetSurface();
+};
+
 void moz_container_wayland_class_init(MozContainerClass* klass);
 void moz_container_wayland_init(MozContainerWayland* container);

-struct wl_surface* moz_container_wayland_surface_lock(MozContainer* container);
-void moz_container_wayland_surface_unlock(MozContainer* container,
-                                          struct wl_surface** surface);
-
-struct wl_surface* moz_container_wayland_get_surface_locked(
-    const mozilla::MutexAutoLock& aProofOfLock, MozContainer* container);
-void moz_container_wayland_lock(MozContainer* container);
-void moz_container_wayland_unlock(MozContainer* container);
-
 struct wl_egl_window* moz_container_wayland_get_egl_window(
    MozContainer* container, double scale);

@ -79,9 +80,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);
--- a/widget/gtk/WakeLockListener.cpp
+++ b/widget/gtk/WakeLockListener.cpp
@ -279,12 +279,11 @@ bool WakeLockTopic::InhibitWaylandIdle() {

  UninhibitWaylandIdle();

-  MozContainer* container = focusedWindow->GetMozContainer();
-  wl_surface* waylandSurface = moz_container_wayland_surface_lock(container);
+  MozContainerSurfaceLock lock(focusedWindow->GetMozContainer());
+  struct wl_surface* waylandSurface = lock.GetSurface();
  if (waylandSurface) {
    mWaylandInhibitor = zwp_idle_inhibit_manager_v1_create_inhibitor(
        waylandDisplay->GetIdleInhibitManager(), waylandSurface);
-    moz_container_wayland_surface_unlock(container, &waylandSurface);
  }
  return true;
 }
--- a/widget/gtk/WaylandVsyncSource.cpp
+++ b/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>

@ -147,21 +148,21 @@ void WaylandVsyncSource::Refresh(const MutexAutoLock& aProofOfLock) {
  }

  if (mContainer) {
-    struct wl_surface* surface = moz_container_wayland_surface_lock(mContainer);
+    MozContainerSurfaceLock lock(mContainer);
+    struct wl_surface* surface = lock.GetSurface();
    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
@ -206,7 +207,8 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) {
    mNativeLayerRoot->RequestFrameCallback(&WaylandVsyncSourceCallbackHandler,
                                           this);
  } else {
-    struct wl_surface* surface = moz_container_wayland_surface_lock(mContainer);
+    MozContainerSurfaceLock lock(mContainer);
+    struct wl_surface* surface = lock.GetSurface();
    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 +224,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;
--- a/widget/gtk/WindowSurfaceWaylandMultiBuffer.cpp
+++ b/widget/gtk/WindowSurfaceWaylandMultiBuffer.cpp
@ -266,7 +266,8 @@ void WindowSurfaceWaylandMB::Commit(
  mFrameInProcess = false;

  MozContainer* container = mWindow->GetMozContainer();
-  wl_surface* waylandSurface = moz_container_wayland_surface_lock(container);
+  MozContainerSurfaceLock lock(container);
+  struct wl_surface* waylandSurface = lock.GetSurface();
  if (!waylandSurface) {
    LOGWAYLAND(
        "WindowSurfaceWaylandMB::Commit [%p] frame queued: can't lock "
@ -274,7 +275,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) {
@ -301,7 +302,6 @@ void WindowSurfaceWaylandMB::Commit(

  moz_container_wayland_set_scale_factor_locked(container);
  mInProgressBuffer->AttachAndCommit(waylandSurface);
-  moz_container_wayland_surface_unlock(container, &waylandSurface);

  mInProgressBuffer->ResetBufferAge();
  mFrontBuffer = mInProgressBuffer;
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@ -5452,7 +5452,7 @@ void nsWindow::ConfigureCompositor() {

  LOG("nsWindow::ConfigureCompositor()");
  auto startCompositing = [self = RefPtr{this}, this]() -> void {
-    LOG("  moz_container_wayland_add_initial_draw_callback "
+    LOG("  moz_container_wayland_add_or_fire_initial_draw_callback "
        "ConfigureCompositor");

    // too late
@ -5471,8 +5471,8 @@ void nsWindow::ConfigureCompositor() {

  if (GdkIsWaylandDisplay()) {
 #ifdef MOZ_WAYLAND
-    moz_container_wayland_add_initial_draw_callback(mContainer,
-                                                    startCompositing);
+    moz_container_wayland_add_or_fire_initial_draw_callback(mContainer,
+                                                            startCompositing);
 #endif
  } else {
    startCompositing();