commit 6e156c09fb325dde84ee14a31610380f570504d2
parent 1ec0d52bbd4ddb9ec2219fbee6f0ae31d043bf69
Author: Serban Stanca <sstanca@mozilla.com>
Date: Fri, 7 Nov 2025 17:01:34 +0200
Revert "Bug 1998382 [Wayland] Enforce mSurface is present r=emilio" as requested by @stransky due to regressions in freeze.
This reverts commit 324d853cb2ea3ed0801cacc2b53c448aa053bd2b.
This reverts commit ed60948e3ae9aa11fdd0092bee390279e7ba0560.
This reverts commit ca82b9a13e818d60088d4fb0988922a1a4b4b433.
This reverts commit add312760cc6101eba159772b6caedf4893f297b.
Diffstat:
1 file changed, 73 insertions(+), 25 deletions(-)
diff --git a/widget/gtk/WaylandSurface.cpp b/widget/gtk/WaylandSurface.cpp
@@ -87,19 +87,10 @@ WaylandSurface::WaylandSurface(RefPtr<WaylandSurface> aParent,
LOGWAYLAND("WaylandSurface::WaylandSurface(), parent [%p] size [%d x %d]",
mParent ? mParent->GetLoggingWidget() : nullptr, mSizeScaled.width,
mSizeScaled.height);
- struct wl_compositor* compositor = WaylandDisplayGet()->GetCompositor();
- mSurface = wl_compositor_create_surface(compositor);
- MOZ_RELEASE_ASSERT(mSurface, "Can't create wl_surface!");
}
WaylandSurface::~WaylandSurface() {
LOGWAYLAND("WaylandSurface::~WaylandSurface()");
-
- wl_egl_window* tmp = nullptr;
- mEGLWindow.exchange(tmp);
- MozClearPointer(tmp, wl_egl_window_destroy);
- MozClearPointer(mSurface, wl_surface_destroy);
-
MOZ_RELEASE_ASSERT(!mIsMapped, "We can't release mapped WaylandSurface!");
MOZ_RELEASE_ASSERT(!mSurfaceLock, "We can't release locked WaylandSurface!");
MOZ_RELEASE_ASSERT(mBufferTransactions.Length() == 0,
@@ -139,7 +130,7 @@ void WaylandSurface::ReadyToDrawFrameCallbackHandler(
WaylandSurfaceLock lock(this);
MozClearPointer(mReadyToDrawFrameCallback, wl_callback_destroy);
// It's possible that we're already unmapped so quit in such case.
- if (!mIsMapped) {
+ if (!mSurface) {
LOGWAYLAND(" WaylandSurface is unmapped, quit.");
if (!mReadyToDrawCallbacks.empty()) {
NS_WARNING("Unmapping WaylandSurface with active draw callback!");
@@ -182,6 +173,13 @@ void WaylandSurface::AddReadyToDrawCallbackLocked(
const std::function<void(void)>& aDrawCB) {
LOGVERBOSE("WaylandSurface::AddReadyToDrawCallbackLocked()");
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
+
+ if (mIsReadyToDraw && !mSurface) {
+ NS_WARNING(
+ "WaylandSurface::AddReadyToDrawCallbackLocked():"
+ " ready to draw without wayland surface!");
+ }
+ MOZ_DIAGNOSTIC_ASSERT(!mIsReadyToDraw || !mSurface);
mReadyToDrawCallbacks.push_back(aDrawCB);
}
@@ -189,7 +187,12 @@ void WaylandSurface::AddOrFireReadyToDrawCallback(
const std::function<void(void)>& aDrawCB) {
{
WaylandSurfaceLock lock(this);
- if (!mIsReadyToDraw) {
+ if (mIsReadyToDraw && !mSurface) {
+ NS_WARNING(
+ "WaylandSurface::AddOrFireReadyToDrawCallback(): ready to draw "
+ "without wayland surface!");
+ }
+ if (!mIsReadyToDraw || !mSurface) {
LOGVERBOSE(
"WaylandSurface::AddOrFireReadyToDrawCallback() callback stored");
mReadyToDrawCallbacks.push_back(aDrawCB);
@@ -310,6 +313,8 @@ void WaylandSurface::RequestFrameCallbackLocked(
return;
}
+ MOZ_DIAGNOSTIC_ASSERT(mSurface, "Missing mapped surface!");
+
if (!mFrameCallback) {
static const struct wl_callback_listener listener{
[](void* aData, struct wl_callback* callback, uint32_t time) {
@@ -481,7 +486,7 @@ bool WaylandSurface::MapLocked(const WaylandSurfaceLock& aProofOfLock,
MOZ_DIAGNOSTIC_ASSERT(!mIsMapped, "Already mapped?");
MOZ_DIAGNOSTIC_ASSERT(!(aParentWLSurface && aParentWaylandSurfaceLock),
"Only one parent can be used.");
- MOZ_DIAGNOSTIC_ASSERT(!mSubsurface, "Already mapped?");
+ MOZ_DIAGNOSTIC_ASSERT(!mSurface && !mSubsurface, "Already mapped?");
if (aParentWLSurface) {
LOGWAYLAND(" parent wl_surface [%p]", aParentWLSurface);
@@ -500,9 +505,17 @@ bool WaylandSurface::MapLocked(const WaylandSurfaceLock& aProofOfLock,
mBufferAttached = false;
mLatestAttachedBuffer = 0;
+ struct wl_compositor* compositor = WaylandDisplayGet()->GetCompositor();
+ mSurface = wl_compositor_create_surface(compositor);
+ if (!mSurface) {
+ LOGWAYLAND(" Failed - can't create surface!");
+ return false;
+ }
+
mSubsurface = wl_subcompositor_get_subsurface(
WaylandDisplayGet()->GetSubcompositor(), mSurface, mParentSurface);
if (!mSubsurface) {
+ MozClearPointer(mSurface, wl_surface_destroy);
LOGWAYLAND(" Failed - can't create sub-surface!");
return false;
}
@@ -579,6 +592,7 @@ void WaylandSurface::RunUnmapCallback() {
void WaylandSurface::GdkCleanUpLocked(const WaylandSurfaceLock& aProofOfLock) {
LOGWAYLAND("WaylandSurface::GdkCleanUp()");
AssertIsOnMainThread();
+ MOZ_DIAGNOSTIC_ASSERT(mSurface);
if (mGdkWindow) {
RemoveOpaqueSurfaceHandlerLocked(aProofOfLock);
mGdkWindow = nullptr;
@@ -586,6 +600,9 @@ void WaylandSurface::GdkCleanUpLocked(const WaylandSurfaceLock& aProofOfLock) {
MozClearHandleID(mEmulatedFrameCallbackTimerID, g_source_remove);
mIsPendingGdkCleanup = false;
+ if (!mIsMapped) {
+ MozClearPointer(mSurface, wl_surface_destroy);
+ }
}
void WaylandSurface::ReleaseAllWaylandTransactionsLocked(
@@ -616,6 +633,9 @@ void WaylandSurface::UnmapLocked(WaylandSurfaceLock& aSurfaceLock) {
mViewportDestinationSize = gfx::IntSize(-1, -1);
mViewportSourceRect = gfx::Rect(-1, -1, -1, -1);
+ wl_egl_window* tmp = nullptr;
+ mEGLWindow.exchange(tmp);
+ MozClearPointer(tmp, wl_egl_window_destroy);
MozClearPointer(mFractionalScaleListener, wp_fractional_scale_v1_destroy);
MozClearPointer(mSubsurface, wl_subsurface_destroy);
MozClearPointer(mColorSurface, wp_color_management_surface_v1_destroy);
@@ -628,6 +648,12 @@ void WaylandSurface::UnmapLocked(WaylandSurfaceLock& aSurfaceLock) {
MozClearPointer(mPendingOpaqueRegion, wl_region_destroy);
MozClearPointer(mOpaqueRegionFrameCallback, wl_callback_destroy);
+ // We can't release mSurface if it's used by Gdk for frame callback routing,
+ // it will be released at GdkCleanUpLocked().
+ if (!mIsPendingGdkCleanup) {
+ MozClearPointer(mSurface, wl_surface_destroy);
+ }
+
mIsReadyToDraw = false;
mBufferAttached = false;
@@ -651,7 +677,8 @@ void WaylandSurface::Commit(WaylandSurfaceLock* aProofOfLock, bool aForceCommit,
bool aForceDisplayFlush) {
MOZ_DIAGNOSTIC_ASSERT(aProofOfLock == mSurfaceLock);
- if (aForceCommit || mSurfaceNeedsCommit) {
+ // mSurface may be already deleted, see WaylandSurface::Unmap();
+ if (mSurface && (aForceCommit || mSurfaceNeedsCommit)) {
LOGVERBOSE(
"WaylandSurface::Commit() allowed [%d] needs commit %d, force commit "
"%d flush %d",
@@ -720,7 +747,7 @@ void WaylandSurface::OpaqueCallbackHandler() {
void WaylandSurface::SetOpaqueLocked(const WaylandSurfaceLock& aProofOfLock) {
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
- if (!IsOpaqueRegionEnabled()) {
+ if (!mSurface || !IsOpaqueRegionEnabled()) {
return;
}
LOGVERBOSE("WaylandSurface::SetOpaqueLocked()");
@@ -734,7 +761,7 @@ void WaylandSurface::SetOpaqueLocked(const WaylandSurfaceLock& aProofOfLock) {
void WaylandSurface::SetOpaqueRegionLocked(
const WaylandSurfaceLock& aProofOfLock, const gfx::IntRegion& aRegion) {
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
- if (!IsOpaqueRegionEnabled()) {
+ if (!mSurface || !IsOpaqueRegionEnabled()) {
return;
}
@@ -764,6 +791,9 @@ void WaylandSurface::SetOpaqueRegion(const gfx::IntRegion& aRegion) {
void WaylandSurface::ClearOpaqueRegionLocked(
const WaylandSurfaceLock& aProofOfLock) {
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
+ if (!mSurface) {
+ return;
+ }
LOGVERBOSE("WaylandSurface::ClearOpaqueLocked()");
MozClearPointer(mPendingOpaqueRegion, wl_region_destroy);
mPendingOpaqueRegion =
@@ -929,16 +959,18 @@ wl_surface* WaylandSurface::Lock(WaylandSurfaceLock* aWaylandSurfaceLock)
mMutex.Lock();
MOZ_DIAGNOSTIC_ASSERT(!mSurfaceLock);
mSurfaceLock = aWaylandSurfaceLock;
- return mSurface;
+ return mIsReadyToDraw ? mSurface : nullptr;
}
void WaylandSurface::Unlock(struct wl_surface** aSurface,
WaylandSurfaceLock* aWaylandSurfaceLock) {
- MOZ_DIAGNOSTIC_ASSERT(*aSurface);
- MOZ_DIAGNOSTIC_ASSERT(*aSurface == mSurface);
+ MOZ_DIAGNOSTIC_ASSERT(*aSurface == nullptr || mSurface == nullptr ||
+ *aSurface == mSurface);
MOZ_DIAGNOSTIC_ASSERT(mSurfaceLock == aWaylandSurfaceLock);
mMutex.AssertCurrentThreadOwns();
- *aSurface = nullptr;
+ if (*aSurface) {
+ *aSurface = nullptr;
+ }
mSurfaceLock = nullptr;
mMutex.Unlock();
}
@@ -1001,9 +1033,11 @@ bool WaylandSurface::RemoveOpaqueSurfaceHandlerLocked(
return false;
}
AssertIsOnMainThread();
- LOGWAYLAND("WaylandSurface::RemoveOpaqueSurfaceHandlerLocked()");
- sGdkWaylandWindowRemoveCallbackSurface(mGdkWindow, mSurface);
- mIsOpaqueSurfaceHandlerSet = false;
+ if (mSurface) {
+ LOGWAYLAND("WaylandSurface::RemoveOpaqueSurfaceHandlerLocked()");
+ sGdkWaylandWindowRemoveCallbackSurface(mGdkWindow, mSurface);
+ mIsOpaqueSurfaceHandlerSet = false;
+ }
if (mGdkAfterPaintId) {
GdkFrameClock* frameClock = gdk_window_get_frame_clock(mGdkWindow);
// If we're already unmapped frameClock is nullptr
@@ -1019,7 +1053,11 @@ wl_egl_window* WaylandSurface::GetEGLWindow(DesktopIntSize aSize) {
LOGWAYLAND("WaylandSurface::GetEGLWindow() eglwindow %p", (void*)mEGLWindow);
WaylandSurfaceLock lock(this);
- MOZ_DIAGNOSTIC_ASSERT(mSurface, "Missing wl_surface!");
+ if (!mSurface || !mIsReadyToDraw) {
+ LOGWAYLAND(" quit, mSurface %p mIsReadyToDraw %d", mSurface,
+ (bool)mIsReadyToDraw);
+ return nullptr;
+ }
auto scaledSize = LayoutDeviceIntSize::Round(
aSize * DesktopToLayoutDeviceScale(GetScale()));
@@ -1052,7 +1090,9 @@ bool WaylandSurface::SetEGLWindowSize(LayoutDeviceIntSize aSize) {
// In such case don't return false which would block compositor.
// We return true here and don't block flush WebRender queue.
// We'll be repainted if our window become visible again anyway.
- MOZ_DIAGNOSTIC_ASSERT(mEGLWindow, "Missing ELG window?");
+ if (!mEGLWindow) {
+ return true;
+ }
auto unscaledSize =
DesktopIntSize::Round(aSize / DesktopToLayoutDeviceScale(GetScale()));
@@ -1083,6 +1123,8 @@ void WaylandSurface::InvalidateRegionLocked(
void WaylandSurface::InvalidateLocked(const WaylandSurfaceLock& aProofOfLock) {
MOZ_DIAGNOSTIC_ASSERT(&aProofOfLock == mSurfaceLock);
+ MOZ_DIAGNOSTIC_ASSERT(mSurface);
+
wl_surface_damage_buffer(mSurface, 0, 0, INT32_MAX, INT32_MAX);
mSurfaceNeedsCommit = true;
}
@@ -1150,6 +1192,7 @@ bool WaylandSurface::IsBufferAttached(WaylandBuffer* aBuffer) {
bool WaylandSurface::AttachLocked(const WaylandSurfaceLock& aSurfaceLock,
RefPtr<WaylandBuffer> aBuffer) {
MOZ_DIAGNOSTIC_ASSERT(&aSurfaceLock == mSurfaceLock);
+ MOZ_DIAGNOSTIC_ASSERT(mSurface);
auto scale = GetScale();
LayoutDeviceIntSize bufferSize = aBuffer->GetSize();
@@ -1180,6 +1223,7 @@ bool WaylandSurface::AttachLocked(const WaylandSurfaceLock& aSurfaceLock,
void WaylandSurface::RemoveAttachedBufferLocked(
const WaylandSurfaceLock& aSurfaceLock) {
MOZ_DIAGNOSTIC_ASSERT(&aSurfaceLock == mSurfaceLock);
+ MOZ_DIAGNOSTIC_ASSERT(mSurface);
LOGWAYLAND("WaylandSurface::RemoveAttachedBufferLocked()");
@@ -1207,7 +1251,9 @@ void WaylandSurface::PlaceAboveLocked(const WaylandSurfaceLock& aProofOfLock,
// It's possible that lowerSurface becomed unmapped. In such rare case
// just skip the operation, we may be deleted anyway.
- wl_subsurface_place_above(mSubsurface, lowerSurface->mSurface);
+ if (lowerSurface->mSurface) {
+ wl_subsurface_place_above(mSubsurface, lowerSurface->mSurface);
+ }
mSurfaceNeedsCommit = true;
}
@@ -1219,6 +1265,8 @@ void WaylandSurface::SetTransformFlippedLocked(
return;
}
+ MOZ_RELEASE_ASSERT(mSurface);
+
mBufferTransformFlippedX = aFlippedX;
mBufferTransformFlippedY = aFlippedY;