Merge branch 'gtk-persist-tlw'

Fix bugs affecting wxPersistentTLW under GTK.

See https://github.com/wxWidgets/wxWidgets/pull/1419

include/wx/gtk/toplevel.h

@@ -112,6 +112,14 @@ public:
     // size of WM decorations
     struct DecorSize
     {
+        DecorSize()
+        {
+            left =
+            right =
+            top =
+            bottom = 0;
+        }
+
         int left, right, top, bottom;
     };
     DecorSize m_decorSize;
@@ -156,6 +164,9 @@ private:
     // size hint increments
     int m_incWidth, m_incHeight;
 
+    // position before it last changed
+    wxPoint m_lastPos;
+
     // is the frame currently iconized?
     bool m_isIconized;
 

interface/wx/toplevel.h

@@ -228,6 +228,12 @@ public:
     /**
         Iconizes or restores the window.
 
+        Note that in wxGTK the change to the window state is not immediate,
+        i.e. IsIconized() will typically return @false right after a call to
+        Iconize() and its return value will only change after the control flow
+        returns to the event loop and the notification about the window being
+        really iconized is received.
+
         @param iconize
             If @true, iconizes the window; if @false, shows and restores it.
 
@@ -285,6 +291,9 @@ public:
     /**
         Maximizes or restores the window.
 
+        Note that, just as with Iconize(), the change to the window state is
+        not immediate in at least wxGTK port.
+
         @param maximize
             If @true, maximizes the window, otherwise it restores it.
 

src/gtk/minifram.cpp

@@ -307,7 +307,6 @@ bool wxMiniFrame::Create( wxWindow *parent, wxWindowID id, const wxString &title
     if (style & wxRESIZE_BORDER)
        m_gdkFunc |= GDK_FUNC_RESIZE;
     gtk_window_set_default_size(GTK_WINDOW(m_widget), m_width, m_height);
-    memset(&m_decorSize, 0, sizeof(m_decorSize));
     m_deferShow = false;
 
     if (m_parent && (GTK_IS_WINDOW(m_parent->m_widget)))

src/gtk/toplevel.cpp

@@ -326,6 +326,8 @@ void wxTopLevelWindowGTK::GTKConfigureEvent(int x, int y)
 
     if (m_x != point.x || m_y != point.y)
     {
+        m_lastPos = wxPoint(m_x, m_y);
+
         m_x = point.x;
         m_y = point.y;
         wxMoveEvent event(point, GetId());
@@ -565,7 +567,6 @@ void wxTopLevelWindowGTK::Init()
     m_updateDecorSize = true;
     m_netFrameExtentsTimerId = 0;
     m_incWidth = m_incHeight = 0;
-    memset(&m_decorSize, 0, sizeof(m_decorSize));
 
     m_urgency_hint = -2;
 }
@@ -1496,6 +1497,62 @@ void wxTopLevelWindowGTK::SetIconizeState(bool iconize)
 {
     if ( iconize != m_isIconized )
     {
+        /*
+            We get a configure-event _before_ the window is iconized with dummy
+            (0, 0) coordinates. Unfortunately we can't just ignore this event
+            when we get it, because we can't know if we're going to be iconized
+            or not: even remembering that we should be soon in Iconize() itself
+            doesn't help due to the crazy sequence of events generated by GTK,
+            e.g. under X11 for the sequence of Iconize() and Show() calls we
+            get the following events with GTK2:
+
+            - window-state changes to GDK_WINDOW_STATE_ICONIFIED | WITHDRAWN
+            - window-state changes to just GDK_WINDOW_STATE_ICONIFIED
+            - map event
+            - window-state changes to normal
+            - configure event with normal size
+            - window-state changes to GDK_WINDOW_STATE_ICONIFIED
+            - configure event with normal size
+            - configure event with (0, 0) size
+            - window-state changes to normal (yes, again)
+            - configure event with (0, 0) size
+            - window-state changes to GDK_WINDOW_STATE_ICONIFIED
+
+            So even though we could ignore the first (0, 0) configure event, we
+            still wouldn't be able to ignore the second one, happening after
+            the inexplicable switch to normal state.
+
+            For completeness, with GTK3 the sequence of events is simpler, but
+            still very unhelpful for our purposes:
+
+            - window-state changes to GDK_WINDOW_STATE_ICONIFIED
+            - window-state changes to normal
+            - map event
+            - configure event with normal size
+            - configure event with normal size (yes, because one is not enough)
+            - configure event with (0, 0) size
+            - configure event with (0, 0) size (because why not have 2 of them)
+            - window-state changes to GDK_WINDOW_STATE_ICONIFIED
+
+            Here again we have (0, 0) configure events happening _after_ the
+            window-state switch to normal, that would reset our flag.
+
+            In conclusion, we have no choice but to assume that the window got
+            really moved, but at least we can remember its previous position in
+            order to restore it here if it turns out that it was just iconized.
+         */
+        if ( iconize )
+        {
+            if ( wxPoint(m_x, m_y) == wxPoint(0, 0) )
+            {
+                m_x = m_lastPos.x;
+                m_y = m_lastPos.y;
+
+                // This is not really necessary, but seems tidier.
+                m_lastPos = wxPoint();
+            }
+        }
+
         m_isIconized = iconize;
         (void)SendIconizeEvent(iconize);
     }

tests/persistence/tlw.cpp

@@ -20,6 +20,10 @@
 
 #ifndef WX_PRECOMP
     #include "wx/frame.h"
+
+    #ifdef __WXGTK__
+        #include "wx/stopwatch.h"
+    #endif // __WXGTK__
 #endif // WX_PRECOMP
 
 #include "wx/persist/toplevel.h"
@@ -88,6 +92,7 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]")
     }
 
     // Now try recreating the frame using the restored values.
+    bool checkIconized = true;
     {
         wxFrame* const frame = CreatePersistenceTestFrame();
 
@@ -106,6 +111,21 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]")
         frame->Iconize();
         frame->Show();
 
+#ifdef __WXGTK__
+        wxStopWatch sw;
+        while ( !frame->IsIconized() )
+        {
+            wxYield();
+            if ( sw.Time() > 500 )
+            {
+                // 500ms should be enough for the window to end up iconized.
+                WARN("Frame wasn't iconized as expected");
+                checkIconized = false;
+                break;
+            }
+        }
+#endif // __WXGTK__
+
         delete frame;
     }
 
@@ -115,8 +135,27 @@ TEST_CASE_METHOD(PersistenceTests, "wxPersistTLW", "[persist][tlw]")
 
         CHECK(wxPersistenceManager::Get().RegisterAndRestore(frame));
 
+        // As above, we need to show the frame for it to be actually iconized.
+        frame->Show();
+
         CHECK(!frame->IsMaximized());
+        if ( checkIconized )
+        {
+#ifdef __WXGTK__
+            wxStopWatch sw;
+            while ( !frame->IsIconized() )
+            {
+                wxYield();
+                if ( sw.Time() > 500 )
+                {
+                    INFO("Abandoning wait after " << sw.Time() << "ms");
+                    break;
+                }
+            }
+#endif // __WXGTK__
+
             CHECK(frame->IsIconized());
+        }
 
         frame->Restore();