From 06c83032c0ec2dbedef3ea41e0207c7fe8b8d870 Mon Sep 17 00:00:00 2001 From: stransky Date: Thu, 11 Feb 2021 15:30:17 +0000 Subject: [PATCH] Bug 1683578 [wayland] Use wl_surface ID instead of wl_surface address when check for stored frame callback, r=jhorak Depends on D104711 Differential Revision: https://phabricator.services.mozilla.com/D104712 --- widget/gtk/WindowSurfaceWayland.cpp | 33 ++++++++++++++--------------- widget/gtk/WindowSurfaceWayland.h | 2 +- widget/gtk/nsWindow.cpp | 6 ++++++ 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/widget/gtk/WindowSurfaceWayland.cpp b/widget/gtk/WindowSurfaceWayland.cpp index 3c420b6862d3..3538d506d0be 100644 --- a/widget/gtk/WindowSurfaceWayland.cpp +++ b/widget/gtk/WindowSurfaceWayland.cpp @@ -474,7 +474,7 @@ WindowSurfaceWayland::WindowSurfaceWayland(nsWindow* aWindow) mWaylandBuffer(nullptr), mWaylandFullscreenDamage(false), mFrameCallback(nullptr), - mLastCommittedSurface(nullptr), + mLastCommittedSurfaceID(-1), mLastCommitTime(0), mDrawToWaylandBufferDirectly(true), mCanSwitchWaylandBuffer(true), @@ -812,6 +812,7 @@ already_AddRefed WindowSurfaceWayland::Lock( mWaylandBufferDamage.SetEmpty(); mCanSwitchWaylandBuffer = true; mWLBufferIsDirty = false; + mBufferNeedsClear = true; } mMozContainerRect = mozContainerSize; } @@ -978,7 +979,7 @@ bool WindowSurfaceWayland::FlushPendingCommitsLocked() { mDrawToWaylandBufferDirectly)); LOGWAYLAND((" mCanSwitchWaylandBuffer = %d\n", mCanSwitchWaylandBuffer)); LOGWAYLAND((" mFrameCallback = %p\n", mFrameCallback)); - LOGWAYLAND((" mLastCommittedSurface = %p\n", mLastCommittedSurface)); + LOGWAYLAND((" mLastCommittedSurfaceID = %d\n", mLastCommittedSurfaceID)); LOGWAYLAND((" mWLBufferIsDirty = %d\n", mWLBufferIsDirty)); LOGWAYLAND((" mBufferCommitAllowed = %d\n", mBufferCommitAllowed)); @@ -1007,12 +1008,6 @@ bool WindowSurfaceWayland::FlushPendingCommitsLocked() { LOGWAYLAND( (" moz_container_wayland_surface_lock() failed, delay commit.\n")); - // Target window is not created yet - delay the commit. This can happen only - // when the window is newly created and there's no active - // frame callback pending. - MOZ_ASSERT(!mFrameCallback || waylandSurface != mLastCommittedSurface, - "Missing wayland surface at frame callback!"); - if (!mSurfaceReadyTimerID) { mSurfaceReadyTimerID = g_timeout_add( EVENT_LOOP_DELAY, &WaylandBufferFlushPendingCommits, this); @@ -1037,22 +1032,26 @@ bool WindowSurfaceWayland::FlushPendingCommitsLocked() { // We have an active frame callback request so handle it. if (mFrameCallback) { - if (waylandSurface == mLastCommittedSurface) { - LOGWAYLAND((" [%p] wait for frame callback.\n", (void*)this)); + int waylandSurfaceID = wl_proxy_get_id((struct wl_proxy*)waylandSurface); + if (waylandSurfaceID == mLastCommittedSurfaceID) { + LOGWAYLAND((" [%p] wait for frame callback ID %d.\n", (void*)this, + waylandSurfaceID)); // We have an active frame callback pending from our recent surface. // It means we should defer the commit to FrameCallbackHandler(). return true; } - LOGWAYLAND((" Removing wrong frame callback [%p].\n", mFrameCallback)); + LOGWAYLAND((" Removing wrong frame callback [%p] ID %d.\n", + mFrameCallback, + wl_proxy_get_id((struct wl_proxy*)mFrameCallback))); // If our stored wl_surface does not match the actual one it means the frame // callback is no longer active and we should release it. wl_callback_destroy(mFrameCallback); mFrameCallback = nullptr; - mLastCommittedSurface = nullptr; + mLastCommittedSurfaceID = -1; } if (mWaylandFullscreenDamage) { - LOGWAYLAND((" wl_surface_damage full screen\n")); + LOGWAYLAND((" wl_surface_damage full screen\n")); wl_surface_damage(waylandSurface, 0, 0, INT_MAX, INT_MAX); } else { for (auto iter = mWaylandBufferDamage.RectIter(); !iter.Done(); @@ -1077,7 +1076,7 @@ bool WindowSurfaceWayland::FlushPendingCommitsLocked() { wl_callback_add_listener(mFrameCallback, &frame_listener, this); mWaylandBuffer->Attach(waylandSurface); - mLastCommittedSurface = waylandSurface; + mLastCommittedSurfaceID = wl_proxy_get_id((struct wl_proxy*)waylandSurface); mLastCommitTime = g_get_monotonic_time() / 1000; // There's no pending commit, all changes are sent to compositor. @@ -1091,8 +1090,8 @@ void WindowSurfaceWayland::Commit(const LayoutDeviceIntRegion& aInvalidRegion) { { gfx::IntRect lockSize = aInvalidRegion.GetBounds().ToUnknownRect(); LOGWAYLAND( - ("WindowSurfaceWayland::Commit [%p] damage size [%d, %d] -> [%d x %d]" - "screenSize [%d x %d]\n", + ("WindowSurfaceWayland::Commit [%p] damage size [%d, %d] -> [%d x %d] " + "MozContainer [%d x %d]\n", (void*)this, lockSize.x, lockSize.y, lockSize.width, lockSize.height, mMozContainerRect.width, mMozContainerRect.height)); LOGWAYLAND((" mDrawToWaylandBufferDirectly = %d\n", @@ -1117,7 +1116,7 @@ void WindowSurfaceWayland::Commit(const LayoutDeviceIntRegion& aInvalidRegion) { void WindowSurfaceWayland::FrameCallbackHandler() { MOZ_ASSERT(mFrameCallback != nullptr, "FrameCallbackHandler() called without valid frame callback!"); - MOZ_ASSERT(mLastCommittedSurface != nullptr, + MOZ_ASSERT(mLastCommittedSurfaceID != -1, "FrameCallbackHandler() called without valid wl_surface!"); LOGWAYLAND( ("WindowSurfaceWayland::FrameCallbackHandler [%p]\n", (void*)this)); diff --git a/widget/gtk/WindowSurfaceWayland.h b/widget/gtk/WindowSurfaceWayland.h index bedd1ba0b434..c7e848d9913d 100644 --- a/widget/gtk/WindowSurfaceWayland.h +++ b/widget/gtk/WindowSurfaceWayland.h @@ -211,7 +211,7 @@ class WindowSurfaceWayland : public WindowSurface { // Any next commit to wayland compositor will happen when frame callback // comes from wayland compositor back as it's the best time to do the commit. wl_callback* mFrameCallback; - wl_surface* mLastCommittedSurface; + int mLastCommittedSurfaceID; // Cached drawings. If we can't get WaylandBuffer (wl_buffer) at // WindowSurfaceWayland::Lock() we direct gecko rendering to diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp index fba8154a11f8..37aceee7b221 100644 --- a/widget/gtk/nsWindow.cpp +++ b/widget/gtk/nsWindow.cpp @@ -1786,6 +1786,9 @@ void nsWindow::NativeMoveResizeWaylandPopup(GdkPoint* aPosition, bool isWidgetVisible = (sGtkWidgetIsVisible != nullptr) && sGtkWidgetIsVisible(mShell); if (isWidgetVisible) { + LOG( + (" temporary hide popup due to " + "https://gitlab.gnome.org/GNOME/gtk/issues/1986\n")); PauseRemoteRenderer(); gtk_widget_hide(mShell); } @@ -1840,6 +1843,9 @@ void nsWindow::NativeMoveResizeWaylandPopup(GdkPoint* aPosition, if (isWidgetVisible) { // We show the popup with the same configuration so no need to call // ConfigureWaylandPopupWindows() before gtk_widget_show(). + LOG( + (" show popup due to " + "https://gitlab.gnome.org/GNOME/gtk/issues/1986\n")); gtk_widget_show(mShell); } }