Merge branch 'msw-tlw-state'

Fix showing TLWs under MSW when restoring their geometry.

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

include/wx/msw/private/tlwgeom.h

@@ -77,12 +77,20 @@ public:
         wxCopyRectToRECT(r, m_placement.rcNormalPosition);
 
         // Maximized/minimized state.
+        //
+        // Note the special case of SW_MINIMIZE: while GetWindowPlacement()
+        // returns SW_SHOWMINIMIZED when the window is iconized, we restore it
+        // as SW_MINIMIZE as this is what the code in wxTLW checks to determine
+        // whether the window is supposed to be iconized or not.
+        //
+        // Just to confuse matters further, note that SW_MAXIMIZE is exactly
+        // the same thing as SW_SHOWMAXIMIZED.
         int tmp;
         UINT& show = m_placement.showCmd;
         if ( ser.RestoreField(wxPERSIST_TLW_MAXIMIZED, &tmp) && tmp )
-            show = SW_SHOWMAXIMIZED;
+            show = SW_MAXIMIZE;
         else if ( ser.RestoreField(wxPERSIST_TLW_ICONIZED, &tmp) && tmp )
-            show = SW_SHOWMINIMIZED;
+            show = SW_MINIMIZE;
         else
             show = SW_SHOWNORMAL;
 
@@ -110,6 +118,18 @@ public:
 
     virtual bool ApplyTo(wxTopLevelWindow* tlw) wxOVERRIDE
     {
+        // There is a subtlety here: if the window is currently hidden,
+        // restoring its geometry shouldn't show it, so we must use SW_HIDE as
+        // show command, but showing it later should restore it to the correct
+        // state, so we need to remember it in wxTLW itself. And even if it's
+        // currently shown, we still need to update its show command, so that
+        // it matches the real window state after SetWindowPlacement() call.
+        tlw->MSWSetShowCommand(m_placement.showCmd);
+        if ( !tlw->IsShown() )
+        {
+            m_placement.showCmd = SW_HIDE;
+        }
+
         if ( !::SetWindowPlacement(GetHwndOf(tlw), &m_placement) )
         {
             wxLogLastError(wxS("SetWindowPlacement"));

include/wx/msw/toplevel.h

@@ -116,6 +116,9 @@ public:
     // returns true if the platform should explicitly apply a theme border
     virtual bool CanApplyThemeBorder() const wxOVERRIDE { return false; }
 
+    // This function is only for internal use.
+    void MSWSetShowCommand(WXUINT showCmd) { m_showCmd = showCmd; }
+
 protected:
     // common part of all ctors
     void Init();
@@ -132,9 +135,12 @@ protected:
                       const wxPoint& pos,
                       const wxSize& size);
 
-    // common part of Iconize(), Maximize() and Restore()
+    // Just a wrapper around MSW ShowWindow().
     void DoShowWindow(int nShowCmd);
 
+    // Return true if the window is iconized at MSW level, ignoring m_showCmd.
+    bool MSWIsIconized() const;
+
     // override those to return the normal window coordinates even when the
     // window is minimized
     virtual void DoGetPosition(int *x, int *y) const wxOVERRIDE;
@@ -156,13 +162,11 @@ protected:
                                           int& x, int& y,
                                           int& w, int& h) const wxOVERRIDE;
 
-
+    // This field contains the show command to use when showing the window the
-    // is the window currently iconized?
+    // next time and also indicates whether the window should be considered
-    bool m_iconized;
+    // being iconized or maximized (which may be different from whether it's
-
+    // actually iconized or maximized at MSW level).
-    // should the frame be maximized when it will be shown? set by Maximize()
+    WXUINT m_showCmd;
-    // when it is called while the frame is hidden
-    bool m_maximizeOnShow;
 
     // Data to save/restore when calling ShowFullScreen
     long                  m_fsStyle; // Passed to ShowFullScreen

src/msw/dialog.cpp

@@ -321,7 +321,7 @@ WXLRESULT wxDialog::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM lPar
             switch ( wParam )
             {
                 case SIZE_MINIMIZED:
-                    m_iconized = true;
+                    m_showCmd = SW_MINIMIZE;
                     break;
 
                 case SIZE_MAXIMIZED:
@@ -331,9 +331,9 @@ WXLRESULT wxDialog::MSWWindowProc(WXUINT message, WXWPARAM wParam, WXLPARAM lPar
                     if ( m_hGripper )
                         ShowGripper( wParam == SIZE_RESTORED );
 
-                    if ( m_iconized )
+                    if ( m_showCmd == SW_MINIMIZE )
                         (void)SendIconizeEvent(false);
-                    m_iconized = false;
+                    m_showCmd = SW_RESTORE;
 
                     break;
             }

src/msw/frame.cpp

@@ -261,7 +261,7 @@ void wxFrame::DoGetClientSize(int *x, int *y) const
 // generate an artificial resize event
 void wxFrame::SendSizeEvent(int flags)
 {
-    if ( !m_iconized )
+    if ( !MSWIsIconized() )
     {
         RECT r = wxGetWindowRect(GetHwnd());
 
@@ -667,7 +667,7 @@ void wxFrame::PositionToolBar()
 // on the desktop, but are iconized/restored with it
 void wxFrame::IconizeChildFrames(bool bIconize)
 {
-    m_iconized = bIconize;
+    m_showCmd = bIconize ? SW_MINIMIZE : SW_RESTORE;
 
     for ( wxWindowList::compatibility_iterator node = GetChildren().GetFirst();
           node;
@@ -743,6 +743,12 @@ bool wxFrame::MSWDoTranslateMessage(wxFrame *frame, WXMSG *pMsg)
 
 bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id)
 {
+    // We can get a WM_SIZE when restoring a hidden window using
+    // SetWindowPlacement(), don't do anything in this case as our state will
+    // be really updated later, when (and if) we're shown.
+    if ( !IsShown() )
+        return true;
+
     switch ( id )
     {
         case SIZE_RESTORED:
@@ -750,7 +756,7 @@ bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id)
             // only do it it if we were iconized before, otherwise resizing the
             // parent frame has a curious side effect of bringing it under it's
             // children
-            if ( !m_iconized )
+            if ( m_showCmd != SW_MINIMIZE )
                 break;
 
             // restore all child frames too
@@ -765,7 +771,7 @@ bool wxFrame::HandleSize(int WXUNUSED(x), int WXUNUSED(y), WXUINT id)
             break;
     }
 
-    if ( !m_iconized )
+    if ( m_showCmd != SW_MINIMIZE )
     {
 #if wxUSE_STATUSBAR
         PositionStatusBar();

src/msw/toplevel.cpp

@@ -95,8 +95,7 @@ wxEND_EVENT_TABLE()
 
 void wxTopLevelWindowMSW::Init()
 {
-    m_iconized =
+    m_showCmd = SW_SHOWNORMAL;
-    m_maximizeOnShow = false;
 
     // Data to save/restore when calling ShowFullScreen
     m_fsStyle = 0;
@@ -522,14 +521,6 @@ void wxTopLevelWindowMSW::DoShowWindow(int nShowCmd)
 {
     ::ShowWindow(GetHwnd(), nShowCmd);
 
-    // Hiding the window doesn't change its iconized state.
-    if ( nShowCmd != SW_HIDE )
-    {
-        // Otherwise restoring, maximizing or showing the window normally also
-        // makes it not iconized and only minimizing it does make it iconized.
-        m_iconized = nShowCmd == SW_MINIMIZE;
-    }
-
 #if wxUSE_TOOLTIPS
     // Don't leave a tooltip hanging around if TLW is hidden now.
     wxToolTip::UpdateVisibility();
@@ -541,7 +532,12 @@ void wxTopLevelWindowMSW::ShowWithoutActivating()
     if ( !wxWindowBase::Show(true) )
         return;
 
-    DoShowWindow(SW_SHOWNA);
+    // We can't show the window in a maximized state without activating it, so
+    // the sequence of hiding the window, calling Maximize() and this function
+    // will end up with the window not being maximized -- but this is arguably
+    // better than activating it and is compatible with the historical
+    // behaviour of this function.
+    DoShowWindow(m_showCmd == SW_MINIMIZE ? SW_SHOWMINNOACTIVE : SW_SHOWNA);
 }
 
 bool wxTopLevelWindowMSW::Show(bool show)
@@ -553,18 +549,10 @@ bool wxTopLevelWindowMSW::Show(bool show)
     int nShowCmd;
     if ( show )
     {
-        if ( m_maximizeOnShow )
+        // If we need to minimize or maximize the window, do it now.
+        if ( m_showCmd == SW_MAXIMIZE || m_showCmd == SW_MINIMIZE )
         {
-            // show and maximize
+            nShowCmd = m_showCmd;
-            nShowCmd = SW_MAXIMIZE;
-
-            m_maximizeOnShow = false;
-        }
-        else if ( m_iconized )
-        {
-            // We were iconized while we were hidden, so now we need to show
-            // the window in iconized state.
-            nShowCmd = SW_MINIMIZE;
         }
         else if ( ::IsIconic(GetHwnd()) )
         {
@@ -622,16 +610,19 @@ void wxTopLevelWindowMSW::Raise()
 
 void wxTopLevelWindowMSW::Maximize(bool maximize)
 {
+    // Update m_showCmd to ensure that the window is maximized when it's shown
+    // later even if it's currently hidden.
+    m_showCmd = maximize ? SW_MAXIMIZE : SW_RESTORE;
+
     if ( IsShown() )
     {
         // just maximize it directly
-        DoShowWindow(maximize ? SW_MAXIMIZE : SW_RESTORE);
+        DoShowWindow(m_showCmd);
     }
     else // hidden
     {
         // we can't maximize the hidden frame because it shows it as well,
-        // so just remember that we should do it later in this case
+        // so don't do anything other than updating m_showCmd for now
-        m_maximizeOnShow = maximize;
 
 #if wxUSE_DEFERRED_SIZING
         // after calling Maximize() the client code expects to get the frame
@@ -659,45 +650,66 @@ bool wxTopLevelWindowMSW::IsMaximized() const
 {
     return IsAlwaysMaximized() ||
            (::IsZoomed(GetHwnd()) != 0) ||
-           m_maximizeOnShow;
+           m_showCmd == SW_MAXIMIZE;
 }
 
 void wxTopLevelWindowMSW::Iconize(bool iconize)
 {
-    if ( iconize == m_iconized )
+    if ( iconize == MSWIsIconized() )
     {
         // Do nothing, in particular don't restore non-iconized windows when
         // Iconize(false) is called as this would wrongly un-maximize them.
         return;
     }
 
+    // Note that we can't change m_showCmd yet as wxFrame WM_SIZE handler uses
+    // its value to determine whether the frame had been iconized before or not
+    // and this handler will be called from inside DoShowWindow() below.
+    const UINT showCmd = iconize ? SW_MINIMIZE : SW_RESTORE;
+
+    // We can't actually iconize the window if it's currently hidden, as this
+    // would also show it unexpectedly.
     if ( IsShown() )
     {
-        // change the window state immediately
+        DoShowWindow(showCmd);
-        DoShowWindow(iconize ? SW_MINIMIZE : SW_RESTORE);
-    }
-    else // hidden
-    {
-        // iconizing the window shouldn't show it so just update the internal
-        // state (otherwise it's done by DoShowWindow() itself)
-        m_iconized = iconize;
     }
+
+    // Update the internal flag in any case, so that IsIconized() returns the
+    // correct value, for example. And if the window is currently hidden, this
+    // also ensures that the next call to Show() will show it in an iconized
+    // state instead of showing it normally.
+    m_showCmd = showCmd;
 }
 
 bool wxTopLevelWindowMSW::IsIconized() const
 {
     if ( !IsShown() )
-        return m_iconized;
+    {
+        // Hidden windows are never actually iconized at MSW level, but can be
+        // in wx, so use m_showCmd to determine our status.
+        return m_showCmd == SW_MINIMIZE;
+    }
 
-    // don't use m_iconized, it may be briefly out of sync with the real state
+    // don't use m_showCmd, it may be briefly out of sync with the real state
     // as it's only modified when we receive a WM_SIZE and we could be called
     // from an event handler from one of the messages we receive before it,
     // such as WM_MOVE
+    return MSWIsIconized();
+}
+
+bool wxTopLevelWindowMSW::MSWIsIconized() const
+{
     return ::IsIconic(GetHwnd()) != 0;
 }
 
 void wxTopLevelWindowMSW::Restore()
 {
+    // Forget any previous minimized/maximized status.
+    m_showCmd = SW_SHOW;
+
+    // And actually restore the window to its normal state. Note that here,
+    // unlike in Maximize() and Iconize(), we do it even if the window is
+    // currently hidden, i.e. Restore() is supposed to show it in this case.
     DoShowWindow(SW_RESTORE);
 }
 
@@ -1181,7 +1193,7 @@ void wxTopLevelWindowMSW::DoThaw()
 
 void wxTopLevelWindowMSW::DoSaveLastFocus()
 {
-    if ( m_iconized )
+    if ( MSWIsIconized() )
         return;
 
     // remember the last focused child if it is our child
@@ -1211,7 +1223,7 @@ void wxTopLevelWindowMSW::OnActivate(wxActivateEvent& event)
         // We get WM_ACTIVATE before being restored from iconized state, so we
         // can be still iconized here. In this case, avoid restoring the focus
         // as it doesn't work anyhow and we will do when we're really restored.
-        if ( m_iconized )
+        if ( MSWIsIconized() )
         {
             event.Skip();
             return;

tests/toplevel/toplevel.cpp

@@ -17,6 +17,7 @@
 #endif
 
 #ifndef WX_PRECOMP
+    #include "wx/app.h"
     #include "wx/dialog.h"
     #include "wx/frame.h"
     #include "wx/textctrl.h"
@@ -32,6 +33,7 @@ static void TopLevelWindowShowTest(wxTopLevelWindow* tlw)
 
 // only run this test on platforms where ShowWithoutActivating is implemented.
 #if defined(__WXMSW__) || defined(__WXMAC__)
+    wxTheApp->GetTopWindow()->SetFocus();
     tlw->ShowWithoutActivating();
     CHECK(tlw->IsShown());
     CHECK(!tlw->IsActive());
@@ -53,19 +55,12 @@ static void TopLevelWindowShowTest(wxTopLevelWindow* tlw)
     tlw->Hide();
     CHECK(!tlw->IsShown());
 #ifndef __WXGTK__
-    CHECK(tlw->IsActive());
+    CHECK(!tlw->IsActive());
 #endif
 }
 
 TEST_CASE("wxTopLevel::Show", "[tlw][show]")
 {
-    if ( IsAutomaticTest() )
-    {
-        // For some reason, activation test doesn't work when running under
-        // AppVeyor, so skip it to avoid spurious failures.
-        return;
-    }
-
     SECTION("Dialog")
     {
         wxDialog* dialog = new wxDialog(NULL, -1, "Dialog Test");