Merge branch 'safer-sizers'

Improvements to the safety of wxSizer API.

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

Closes #18611.

interface/wx/sizer.h

@@ -1098,7 +1098,19 @@ public:
     /**
         Set the window to be tracked by this item.
 
-        The old window isn't deleted as it is now owned by the sizer item.
+        @note This is a low-level method which is dangerous if used
+            incorrectly, avoid using it if possible, i.e. if higher level
+            methods such as wxSizer::Replace() can be used instead.
+
+        If the sizer item previously contained a window, it is dissociated from
+        the sizer containing this sizer item (if any), but this object doesn't
+        have the pointer to the containing sizer and so it's the caller's
+        responsibility to call wxWindow::SetContainingSizer() on @a window.
+        Failure to do this can result in memory corruption when the window is
+        destroyed later, so it is crucial to not forget to do it.
+
+        Also note that the previously contained window is @e not deleted, so
+        it's also the callers responsibility to do it, if necessary.
     */
     void AssignWindow(wxWindow *window);
 

src/common/sizer.cpp

@@ -869,7 +869,7 @@ bool wxSizer::Replace( wxSizer *oldsz, wxSizer *newsz, bool recursive )
 bool wxSizer::Replace( size_t old, wxSizerItem *newitem )
 {
     wxCHECK_MSG( old < m_children.GetCount(), false, wxT("Replace index is out of range") );
-    wxASSERT_MSG( newitem, wxT("Replacing with NULL item") );
+    wxCHECK_MSG( newitem, false, wxT("Replacing with NULL item") );
 
     wxSizerItemList::compatibility_iterator node = m_children.Item( old );
 
@@ -878,11 +878,14 @@ bool wxSizer::Replace( size_t old, wxSizerItem *newitem )
     wxSizerItem *item = node->GetData();
     node->SetData(newitem);
 
-    if (item->IsWindow() && item->GetWindow())
+    if (wxWindow* const w = item->GetWindow())
-        item->GetWindow()->SetContainingSizer(NULL);
+        w->SetContainingSizer(NULL);
 
     delete item;
 
+    if (wxWindow* const w = newitem->GetWindow())
+        w->SetContainingSizer(this);
+
     return true;
 }
 

tests/sizers/boxsizer.cpp

@@ -24,6 +24,8 @@
 
 #include "asserthelper.h"
 
+#include "wx/scopedptr.h"
+
 // ----------------------------------------------------------------------------
 // test fixture
 // ----------------------------------------------------------------------------
@@ -367,7 +369,9 @@ TEST_CASE_METHOD(BoxSizerTestCase, "BoxSizer::IncompatibleFlags", "[sizer]")
 #define ASSERT_SIZER_INVALID_FLAGS(f, msg) \
     WX_ASSERT_FAILS_WITH_ASSERT_MESSAGE( \
             "Expected assertion not generated for " msg, \
-            sizer->Add(10, 10, 0, f) \
+            wxScopedPtr<wxSizerItem> item(new wxSizerItem(10, 10, 0, f)); \
+            sizer->Add(item.get()); \
+            item.release() \
         )
 
 #define ASSERT_SIZER_INCOMPATIBLE_FLAGS(f1, f2) \
@@ -440,3 +444,9 @@ TEST_CASE_METHOD(BoxSizerTestCase, "BoxSizer::IncompatibleFlags", "[sizer]")
 #undef ASSERT_SIZER_INCOMPATIBLE_FLAGS
 #undef ASSERT_SIZER_INVALID_FLAGS
 }
+
+TEST_CASE_METHOD(BoxSizerTestCase, "BoxSizer::Replace", "[sizer]")
+{
+    m_sizer->AddSpacer(1);
+    m_sizer->Replace(0, new wxSizerItem(new wxWindow(m_win, wxID_ANY)));
+}