From f289de4ecbaa2f3924eb7c7351c7b1bfb606b58b Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 31 Jul 2020 12:56:18 +0000 Subject: [PATCH] Bug 1652743 - Replace Window origin caching with another implementation. r=aosmond The new implementation is based on traversing the parent window hierarchy and accumulating relative positions. The implementation unfortunately has to be disabled while another issue is investigated. Differential Revision: https://phabricator.services.mozilla.com/D85278 --- widget/gtk/nsWindow.cpp | 86 ++++++++++++++++++----------------------- widget/gtk/nsWindow.h | 5 --- 2 files changed, 38 insertions(+), 53 deletions(-) diff --git a/widget/gtk/nsWindow.cpp b/widget/gtk/nsWindow.cpp index d70f990b8acf..db4ed70d2769 100644 --- a/widget/gtk/nsWindow.cpp +++ b/widget/gtk/nsWindow.cpp @@ -375,6 +375,37 @@ static void UpdateLastInputEventTime(void* aGdkEvent) { sLastUserInputTime = timestamp; } +void GetWindowOrigin(GdkWindow* aWindow, int* aX, int* aY) { + gdk_window_get_origin(aWindow, aX, aY); + + // TODO(bug 1655924): gdk_window_get_origin is can block waiting for the x + // server for a long time, we would like to use the implementation below + // instead. However, removing the synchronous x server queries causes a race + // condition to surface, causing issues such as bug 1652743 and bug 1653711. +#if 0 + *aX = 0; + *aY = 0; + if (!aWindow) { + return; + } + + GdkWindow* current = aWindow; + while (GdkWindow* parent = gdk_window_get_parent(current)) { + if (parent == current) { + break; + } + + int x = 0; + int y = 0; + gdk_window_get_position(current, &x, &y); + *aX += x; + *aY += y; + + current = parent; + } +#endif +} + nsWindow::nsWindow() { mIsTopLevel = false; mIsDestroyed = false; @@ -465,7 +496,6 @@ nsWindow::nsWindow() { mWindowScaleFactor = 1; mIsAccelerated = false; - mWindowOrigin = Nothing(); } nsWindow::~nsWindow() { @@ -528,45 +558,6 @@ void nsWindow::MaybeDispatchResized() { } } -nsIntPoint nsWindow::GetWindowOrigin() { - if (!mGdkWindow) { - return nsIntPoint(0, 0); - } - - if (mWindowOrigin.isNothing()) { - int x = 0; - int y = 0; - gdk_window_get_origin(mGdkWindow, &x, &y); - - mWindowOrigin = Some(nsIntPoint(x, y)); - } - - return mWindowOrigin.value(); -} - -void nsWindow::InvalidateWindowOrigin() { - if (!mGdkWindow) { - return; - } - - mWindowOrigin = Nothing(); - - // The window origin is in a coordinate space relative to the root window. - // In addition GDK_CONFIGURE events are not issued for windows which type - // is GDK_WINDOW_CHILD. - // We have to invalidate the cached window origin of child windows when - // the parent moves. - GList* children = gdk_window_get_children(mGdkWindow); - for (GList* list = children; list; list = list->next) { - RefPtr childWindow = - get_window_for_gdk_window(GDK_WINDOW(list->data)); - if (childWindow) { - childWindow->InvalidateWindowOrigin(); - } - } - g_list_free(children); -} - nsIWidgetListener* nsWindow::GetListener() { return mAttachedWidgetListener ? mAttachedWidgetListener : mWidgetListener; } @@ -1455,8 +1446,8 @@ void nsWindow::NativeMoveResizeWaylandPopupCB(const GdkRectangle* aFinalSize, // in mBounds we have position relative to toplevel window. We need to check // and update mBounds in the toplevel coordinates. int x_parent, y_parent; - gdk_window_get_origin(gtk_widget_get_window(GTK_WIDGET(parentGtkWindow)), - &x_parent, &y_parent); + GetWindowOrigin(gtk_widget_get_window(GTK_WIDGET(parentGtkWindow)), &x_parent, + &y_parent); LayoutDeviceIntRect newBounds(aFinalSize->x, aFinalSize->y, aFinalSize->width, aFinalSize->height); @@ -1500,7 +1491,7 @@ void nsWindow::NativeMoveResizeWaylandPopupCB(const GdkRectangle* aFinalSize, // The NotifyWindowMoved requires the coordinates relative to the toplevel. // We use the gdk_window_get_origin to get correct coordinates. gint x = 0, y = 0; - gdk_window_get_origin(gtk_widget_get_window(GTK_WIDGET(mShell)), &x, &y); + GetWindowOrigin(gtk_widget_get_window(GTK_WIDGET(mShell)), &x, &y); NotifyWindowMoved(GdkCoordToDevicePixels(x), GdkCoordToDevicePixels(y)); } } @@ -1599,8 +1590,8 @@ void nsWindow::NativeMoveResizeWaylandPopup(GdkPoint* aPosition, int x_parent = 0, y_parent = 0; GtkWindow* parentGtkWindow = gtk_window_get_transient_for(GTK_WINDOW(mShell)); if (parentGtkWindow) { - gdk_window_get_origin(gtk_widget_get_window(GTK_WIDGET(parentGtkWindow)), - &x_parent, &y_parent); + GetWindowOrigin(gtk_widget_get_window(GTK_WIDGET(parentGtkWindow)), + &x_parent, &y_parent); } LOG((" x_parent %d y_parent %d\n", x_parent, y_parent)); anchorRect.MoveBy(-x_parent, -y_parent); @@ -2497,7 +2488,8 @@ void nsWindow::SetIcon(const nsAString& aIconSpec) { } LayoutDeviceIntPoint nsWindow::WidgetToScreenOffset() { - nsIntPoint origin = GetWindowOrigin(); + nsIntPoint origin(0, 0); + GetWindowOrigin(mGdkWindow, &origin.x, &origin.y); return GdkPointToDevicePixels({origin.x, origin.y}); } @@ -3001,8 +2993,6 @@ gboolean nsWindow::OnConfigureEvent(GtkWidget* aWidget, // Override-redirect windows are children of the root window so parent // coordinates are root coordinates. - InvalidateWindowOrigin(); - LOG(("configure event [%p] %d %d %d %d\n", (void*)this, aEvent->x, aEvent->y, aEvent->width, aEvent->height)); diff --git a/widget/gtk/nsWindow.h b/widget/gtk/nsWindow.h index 2086924afe0f..ed81f7e52b58 100644 --- a/widget/gtk/nsWindow.h +++ b/widget/gtk/nsWindow.h @@ -431,9 +431,6 @@ class nsWindow final : public nsBaseWidget { void DispatchResized(); void MaybeDispatchResized(); - nsIntPoint GetWindowOrigin(); - void InvalidateWindowOrigin(); - virtual void RegisterTouchWindow() override; virtual bool CompositorInitiallyPaused() override { #ifdef MOZ_WAYLAND @@ -522,8 +519,6 @@ class nsWindow final : public nsBaseWidget { float mAspectRatio; float mAspectRatioSaved; nsIntPoint mClientOffset; - // Cached result of gdk_window_get_origin which can be expensive. - mozilla::Maybe mWindowOrigin; // This field omits duplicate scroll events caused by GNOME bug 726878. guint32 mLastScrollEventTime;