From 35f7c05de1d49463c28a5e858b857a2833f70359 Mon Sep 17 00:00:00 2001 From: stransky Date: Tue, 8 Nov 2022 15:26:25 +0000 Subject: [PATCH] Bug 1796130 [Wayland] Track and release Vsync frame callbacks so we don't get frame callback after nsWindow::Destroy() r=emilio Differential Revision: https://phabricator.services.mozilla.com/D160364 --- widget/gtk/WaylandVsyncSource.cpp | 34 ++++++++++++++++++++----------- widget/gtk/WaylandVsyncSource.h | 3 ++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/widget/gtk/WaylandVsyncSource.cpp b/widget/gtk/WaylandVsyncSource.cpp index a12ff274000b..aba4ca87d2ac 100644 --- a/widget/gtk/WaylandVsyncSource.cpp +++ b/widget/gtk/WaylandVsyncSource.cpp @@ -38,19 +38,18 @@ namespace mozilla { static void WaylandVsyncSourceCallbackHandler(void* aData, struct wl_callback* aCallback, uint32_t aTime) { - WaylandVsyncSource* context = (WaylandVsyncSource*)aData; - wl_callback_destroy(aCallback); - context->FrameCallback(aTime); -} - -static void WaylandVsyncSourceCallbackHandler(void* aData, uint32_t aTime) { - WaylandVsyncSource* context = (WaylandVsyncSource*)aData; - context->FrameCallback(aTime); + RefPtr context(static_cast(aData)); + context->FrameCallback(aCallback, aTime); } static const struct wl_callback_listener WaylandVsyncSourceCallbackListener = { WaylandVsyncSourceCallbackHandler}; +static void NativeLayerRootWaylandVsyncCallback(void* aData, uint32_t aTime) { + RefPtr context(static_cast(aData)); + context->FrameCallback(nullptr, aTime); +} + static float GetFPS(TimeDuration aVsyncRate) { return 1000.0 / aVsyncRate.ToMilliseconds(); } @@ -207,7 +206,7 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) { if (mNativeLayerRoot) { LOG(" use mNativeLayerRoot"); - mNativeLayerRoot->RequestFrameCallback(&WaylandVsyncSourceCallbackHandler, + mNativeLayerRoot->RequestFrameCallback(&NativeLayerRootWaylandVsyncCallback, this); } else { MozContainerSurfaceLock lock(mContainer); @@ -222,8 +221,9 @@ void WaylandVsyncSource::SetupFrameCallback(const MutexAutoLock& aProofOfLock) { } LOG(" register frame callback"); - wl_callback* callback = wl_surface_frame(surface); - wl_callback_add_listener(callback, &WaylandVsyncSourceCallbackListener, + MozClearPointer(mCallback, wl_callback_destroy); + mCallback = wl_surface_frame(surface); + wl_callback_add_listener(mCallback, &WaylandVsyncSourceCallbackListener, this); wl_surface_commit(surface); wl_display_flush(WaylandDisplayGet()->GetDisplay()); @@ -286,7 +286,7 @@ void WaylandVsyncSource::IdleCallback() { } } -void WaylandVsyncSource::FrameCallback(uint32_t aTime) { +void WaylandVsyncSource::FrameCallback(wl_callback* aCallback, uint32_t aTime) { LOG("WaylandVsyncSource::FrameCallback"); MOZ_DIAGNOSTIC_ASSERT(NS_IsMainThread()); @@ -294,11 +294,20 @@ void WaylandVsyncSource::FrameCallback(uint32_t aTime) { // This might enable vsync. RefPtr window = mWindow; window->NotifyOcclusionState(OcclusionState::VISIBLE); + // NotifyOcclusionState can destroy us. + if (window->IsDestroyed()) { + return; + } } MutexAutoLock lock(mMutex); mCallbackRequested = false; + if (aCallback) { + MOZ_RELEASE_ASSERT(aCallback == mCallback); + MozClearPointer(mCallback, wl_callback_destroy); + } + if (!mVsyncEnabled || !mMonitorEnabled) { // We are unwanted by either our creator or our consumer, so we just stop // here without setting up a new frame callback. @@ -404,6 +413,7 @@ void WaylandVsyncSource::Shutdown() { mVsyncEnabled = false; mCallbackRequested = false; MozClearHandleID(mIdleTimerID, g_source_remove); + MozClearPointer(mCallback, wl_callback_destroy); } } // namespace mozilla diff --git a/widget/gtk/WaylandVsyncSource.h b/widget/gtk/WaylandVsyncSource.h index 71947b380497..faed35021f84 100644 --- a/widget/gtk/WaylandVsyncSource.h +++ b/widget/gtk/WaylandVsyncSource.h @@ -55,7 +55,7 @@ class WaylandVsyncSource final : public gfx::VsyncSource { void EnableMonitor(); void DisableMonitor(); - void FrameCallback(uint32_t aTime); + void FrameCallback(wl_callback* aCallback, uint32_t aTime); void IdleCallback(); TimeDuration GetVsyncRate() override; @@ -87,6 +87,7 @@ class WaylandVsyncSource final : public gfx::VsyncSource { TimeDuration mVsyncRate MOZ_GUARDED_BY(mMutex); TimeStamp mLastVsyncTimeStamp MOZ_GUARDED_BY(mMutex); guint mIdleTimerID MOZ_GUARDED_BY(mMutex) = 0; + wl_callback* mCallback MOZ_GUARDED_BY(mMutex) = nullptr; nsWindow* const mWindow; // Main thread only, except for logging. const guint mIdleTimeout;