Bug 1550051 - [Wayland] Don't track invisible popup windows, r=jhorak a=jcristau
authorMartin Stransky <stransky@redhat.com>
Tue, 28 May 2019 11:12:03 +0000
changeset 536687 1debce08ebc9f8f2b9ad5dacafe6bfd4d77df1e5
parent 536686 d6fc2a5a92f24c730c2d033de7a3c692c2d01da0
child 536688 aa42e8f17f875e32ba0bd3adf22d823c7ccf8014
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjhorak, jcristau
bugs1550051
milestone68.0
Bug 1550051 - [Wayland] Don't track invisible popup windows, r=jhorak a=jcristau Recently we track and position all popups on Wayland which leads to wrong placement when parent popup window is hidden. In this patch we use plain gtk_window_move() for hidden popups and we don't track them so they can't be used as a parent window. Also implement nsWindow::HideWaylandPopupAndAllChildren() to clearly hide a popup window with and its children windows. Differential Revision: https://phabricator.services.mozilla.com/D31056
widget/gtk/nsWindow.cpp
widget/gtk/nsWindow.h
--- a/widget/gtk/nsWindow.cpp
+++ b/widget/gtk/nsWindow.cpp
@@ -321,17 +321,17 @@ class CurrentX11TimeGetter {
 
 static NS_DEFINE_IID(kCDragServiceCID, NS_DRAGSERVICE_CID);
 
 // The window from which the focus manager asks us to dispatch key events.
 static nsWindow* gFocusWindow = nullptr;
 static bool gBlockActivateEvent = false;
 static bool gGlobalsInitialized = false;
 static bool gRaiseWindows = true;
-static GList* gCurrentPopupWindows = nullptr;
+static GList* gVisibleWaylandPopupWindows = nullptr;
 
 #if GTK_CHECK_VERSION(3, 4, 0)
 static uint32_t gLastTouchID = 0;
 #endif
 
 #define NS_WINDOW_TITLE_MAX_LENGTH 4095
 
 // If after selecting profile window, the startup fail, please refer to
@@ -1129,87 +1129,118 @@ void nsWindow::Move(double aX, double aY
 
   NotifyRollupGeometryChange();
 }
 
 bool nsWindow::IsWaylandPopup() {
   return !mIsX11Display && mIsTopLevel && mWindowType == eWindowType_popup;
 }
 
-void nsWindow::CloseWaylandTooltips() {
-  GList* popup = gCurrentPopupWindows;
-  GList* next;
-  while (popup) {
-    // CloseWaylandWindow() manipulates the gCurrentPopupWindows list.
-    next = popup->next;
-    nsWindow* window = static_cast<nsWindow*>(popup->data);
-    if (window->mPopupType == ePopupTypeTooltip) {
-      window->CloseWaylandWindow();
-    }
-    popup = next;
+void nsWindow::HideWaylandTooltips() {
+  while (gVisibleWaylandPopupWindows) {
+    nsWindow* window =
+        static_cast<nsWindow*>(gVisibleWaylandPopupWindows->data);
+    if (window->mPopupType != ePopupTypeTooltip) break;
+    window->HideWaylandWindow();
+    gVisibleWaylandPopupWindows = g_list_delete_link(
+        gVisibleWaylandPopupWindows, gVisibleWaylandPopupWindows);
+  }
+}
+
+void nsWindow::HideWaylandPopupAndAllChildren() {
+  if (g_list_find(gVisibleWaylandPopupWindows, this) == nullptr) {
+    NS_WARNING("Popup window isn't in wayland popup list!");
+    return;
+  }
+
+  while (gVisibleWaylandPopupWindows) {
+    nsWindow* window =
+        static_cast<nsWindow*>(gVisibleWaylandPopupWindows->data);
+    bool quit = gVisibleWaylandPopupWindows->data == this;
+    window->HideWaylandWindow();
+    gVisibleWaylandPopupWindows = g_list_delete_link(
+        gVisibleWaylandPopupWindows, gVisibleWaylandPopupWindows);
+    if (quit) break;
   }
 }
 
 // Wayland keeps strong popup window hierarchy. We need to track active
 // (visible) popup windows and make sure we hide popup on the same level
 // before we open another one on that level. It means that every open
 // popup needs to have an unique parent.
 GtkWidget* nsWindow::ConfigureWaylandPopupWindows() {
   // Check if we're already configured.
-  if (gCurrentPopupWindows && g_list_find(gCurrentPopupWindows, this)) {
+  if (gVisibleWaylandPopupWindows &&
+      g_list_find(gVisibleWaylandPopupWindows, this)) {
     return GTK_WIDGET(gtk_window_get_transient_for(GTK_WINDOW(mShell)));
   }
 
   // If we're opening a new window we don't want to attach it to a tooltip
   // as it's short lived temporary window.
-  CloseWaylandTooltips();
+  HideWaylandTooltips();
 
   GtkWindow* parentWidget = nullptr;
-  if (gCurrentPopupWindows) {
+  if (gVisibleWaylandPopupWindows) {
     if (mPopupType == ePopupTypeTooltip) {
       // Attach tooltip window to the latest popup window
       // to have both visible.
-      nsWindow* window = static_cast<nsWindow*>(gCurrentPopupWindows->data);
+      nsWindow* window =
+          static_cast<nsWindow*>(gVisibleWaylandPopupWindows->data);
       parentWidget = GTK_WINDOW(window->GetGtkWidget());
     } else {
+      nsMenuPopupFrame* menuPopupFrame = nullptr;
       nsIFrame* frame = GetFrame();
-      if (!frame) {
-        // We're not fully created yet - just return our default parent.
+      if (frame) {
+        menuPopupFrame = do_QueryFrame(frame);
+      }
+      // The popup is not fully created yet (we're called from
+      // nsWindow::Create()) or we're toplevel popup without parent.
+      // In both cases just use parent which was passed to nsWindow::Create().
+      if (!menuPopupFrame) {
         return GTK_WIDGET(gtk_window_get_transient_for(GTK_WINDOW(mShell)));
       }
-      nsMenuPopupFrame* menuPopupFrame = do_QueryFrame(frame);
-      nsWindow* window =
+
+      nsWindow* parentWindow =
           static_cast<nsWindow*>(menuPopupFrame->GetParentMenuWidget());
-
-      if (!window) {
+      if (!parentWindow) {
         // We're toplevel popup menu attached to another menu. Just use our
         // latest popup as a parent.
-        window = static_cast<nsWindow*>(gCurrentPopupWindows->data);
-        parentWidget = GTK_WINDOW(window->GetGtkWidget());
+        parentWindow =
+            static_cast<nsWindow*>(gVisibleWaylandPopupWindows->data);
+        parentWidget = GTK_WINDOW(parentWindow->GetGtkWidget());
       } else {
         // We're a regular menu in the same frame hierarchy.
-        // Close child popups which are on a different level.
-        parentWidget = GTK_WINDOW(window->GetGtkWidget());
-        do {
-          nsWindow* window = static_cast<nsWindow*>(gCurrentPopupWindows->data);
+        // Close child popups on the same level as we can't have two popups
+        // with one parent on Wayland.
+        parentWidget = GTK_WINDOW(parentWindow->GetGtkWidget());
+        nsWindow* lastChildOnTheSameLevel = nullptr;
+        for (GList* popup = gVisibleWaylandPopupWindows; popup;
+             popup = popup->next) {
+          nsWindow* window =
+              static_cast<nsWindow*>(gVisibleWaylandPopupWindows->data);
           if (GTK_WINDOW(window->GetGtkWidget()) == parentWidget) {
             break;
+          } else {
+            lastChildOnTheSameLevel = window;
           }
-          window->CloseWaylandWindow();
-        } while (gCurrentPopupWindows != nullptr);
+        }
+        if (lastChildOnTheSameLevel) {
+          lastChildOnTheSameLevel->HideWaylandPopupAndAllChildren();
+        }
       }
     }
   }
 
   if (parentWidget) {
     gtk_window_set_transient_for(GTK_WINDOW(mShell), parentWidget);
   } else {
     parentWidget = gtk_window_get_transient_for(GTK_WINDOW(mShell));
   }
-  gCurrentPopupWindows = g_list_prepend(gCurrentPopupWindows, this);
+  gVisibleWaylandPopupWindows =
+      g_list_prepend(gVisibleWaylandPopupWindows, this);
   return GTK_WIDGET(parentWidget);
 }
 
 #ifdef DEBUG
 static void NativeMoveResizeWaylandPopupCallback(
     GdkWindow* window, const GdkRectangle* flipped_rect,
     const GdkRectangle* final_rect, gboolean flipped_x, gboolean flipped_y,
     void* unused) {
@@ -1225,19 +1256,22 @@ void nsWindow::NativeMoveResizeWaylandPo
       GdkWindow*, const GdkRectangle*, GdkGravity, GdkGravity, GdkAnchorHints,
       gint, gint))dlsym(RTLD_DEFAULT, "gdk_window_move_to_rect");
 
   if (aSize) {
     gtk_window_resize(GTK_WINDOW(mShell), aSize->width, aSize->height);
   }
 
   GdkWindow* gdkWindow = gtk_widget_get_window(GTK_WIDGET(mShell));
-  // gdk_window_move_to_rect() is not available, we don't have a valid GdkWindow
-  // (we're not realized yet) - try plain gtk_window_move() at least.
-  if (!sGdkWindowMoveToRect || !gdkWindow) {
+
+  // Use standard gtk_window_move() instead of gdk_window_move_to_rect() when:
+  // - gdk_window_move_to_rect() is not available
+  // - the widget doesn't have a valid GdkWindow
+  // - the widget hasn't beed positioned yet
+  if (!sGdkWindowMoveToRect || !gdkWindow || !AreBoundsSane()) {
     gtk_window_move(GTK_WINDOW(mShell), aPosition->x, aPosition->y);
     return;
   }
 
   GtkWidget* parentWindow = ConfigureWaylandPopupWindows();
   LOG(("nsWindow::NativeMoveResizeWaylandPopup [%p] Set popup parent %p\n",
        (void*)this, parentWindow));
 
@@ -4114,30 +4148,26 @@ void nsWindow::NativeMoveResize() {
 #endif
 
   // Does it need to be shown because bounds were previously insane?
   if (mNeedsShow && mIsShown) {
     NativeShow(true);
   }
 }
 
-void nsWindow::CloseWaylandWindow() {
+void nsWindow::HideWaylandWindow() {
 #ifdef MOZ_WAYLAND
   if (mContainer && moz_container_has_wl_egl_window(mContainer)) {
     // Because wl_egl_window is destroyed on moz_container_unmap(),
     // the current compositor cannot use it anymore. To avoid crash,
     // destroy the compositor & recreate a new compositor on next
     // expose event.
     DestroyLayerManager();
   }
 #endif
-
-  if (mWindowType == eWindowType_popup) {
-    gCurrentPopupWindows = g_list_remove(gCurrentPopupWindows, this);
-  }
   gtk_widget_hide(mShell);
 }
 
 void nsWindow::NativeShow(bool aAction) {
   if (aAction) {
     // unset our flag now that our window has been shown
     mNeedsShow = false;
 
@@ -4153,17 +4183,21 @@ void nsWindow::NativeShow(bool aAction) 
       gtk_widget_show(mShell);
     } else if (mContainer) {
       gtk_widget_show(GTK_WIDGET(mContainer));
     } else if (mGdkWindow) {
       gdk_window_show_unraised(mGdkWindow);
     }
   } else {
     if (!mIsX11Display) {
-      CloseWaylandWindow();
+      if (IsWaylandPopup()) {
+        HideWaylandPopupAndAllChildren();
+      } else {
+        HideWaylandWindow();
+      }
     } else if (mIsTopLevel) {
       // Workaround window freezes on GTK versions before 3.21.2 by
       // ensuring that configure events get dispatched to windows before
       // they are unmapped. See bug 1225044.
       if (gtk_check_version(3, 21, 2) != nullptr && mPendingConfigures > 0) {
         GtkAllocation allocation;
         gtk_widget_get_allocation(GTK_WIDGET(mShell), &allocation);
 
--- a/widget/gtk/nsWindow.h
+++ b/widget/gtk/nsWindow.h
@@ -607,18 +607,19 @@ class nsWindow final : public nsBaseWidg
   void CleanLayerManagerRecursive();
 
   virtual int32_t RoundsWidgetCoordinatesTo() override;
 
   void ForceTitlebarRedraw();
 
   bool IsWaylandPopup();
   GtkWidget* ConfigureWaylandPopupWindows();
-  void CloseWaylandTooltips();
-  void CloseWaylandWindow();
+  void HideWaylandWindow();
+  void HideWaylandTooltips();
+  void HideWaylandPopupAndAllChildren();
 
   /**
    * |mIMContext| takes all IME related stuff.
    *
    * This is owned by the top-level nsWindow or the topmost child
    * nsWindow embedded in a non-Gecko widget.
    *
    * The instance is created when the top level widget is created.  And when