From b88d5e08ce88ee9321e8ca1180995a5ef726a83f Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Tue, 18 Sep 2018 00:33:59 +0200
Subject: [PATCH] Fix adding pages without associated window to wxTreebook

wxTreebook is supposed to allow not specifying any valid window for the
top-level pages, but this didn't work any longer, probably since the
changes of 02a92e23f35fe183901273ddfca2d6d604b921da (see #4379), as a
possibly null page was dereferenced without checking, resulting in a
crash.

Fix this by adding a missing check.

Also rename DoGetNonNullPage() to TryGetNonNullPage() to make it more
clear that this function can return null and add a unit test checking
that calling AddPage(NULL) really works (or at least doesn't crash).

See https://github.com/wxWidgets/wxWidgets/pull/921
---
 include/wx/bookctrl.h           |  5 +++--
 include/wx/treebook.h           |  2 +-
 src/common/bookctrl.cpp         | 15 +++++++++++----
 src/generic/treebkg.cpp         |  2 +-
 tests/controls/treebooktest.cpp | 16 ++++++++++++++++
 5 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/include/wx/bookctrl.h b/include/wx/bookctrl.h
index 267e81b692..638f7c6436 100644
--- a/include/wx/bookctrl.h
+++ b/include/wx/bookctrl.h
@@ -293,8 +293,9 @@ protected:
     // For classes that allow null pages, we also need a way to find the
     // closest non-NULL page corresponding to the given index, e.g. the first
     // leaf item in wxTreebook tree and this method must be overridden to
-    // return it if AllowNullPage() is overridden.
-    virtual wxWindow *DoGetNonNullPage(size_t page) { return m_pages[page]; }
+    // return it if AllowNullPage() is overridden. Note that it can still
+    // return null if there are no valid pages after this one.
+    virtual wxWindow *TryGetNonNullPage(size_t page) { return m_pages[page]; }
 
     // Remove the page and return a pointer to it.
     //
diff --git a/include/wx/treebook.h b/include/wx/treebook.h
index 119eb7dddd..0bbc5eef8c 100644
--- a/include/wx/treebook.h
+++ b/include/wx/treebook.h
@@ -142,7 +142,7 @@ protected:
 
     // This subclass of wxBookCtrlBase accepts NULL page pointers (empty pages)
     virtual bool AllowNullPage() const wxOVERRIDE { return true; }
-    virtual wxWindow *DoGetNonNullPage(size_t page) wxOVERRIDE;
+    virtual wxWindow *TryGetNonNullPage(size_t page) wxOVERRIDE;
 
     // event handlers
     void OnTreeSelectionChange(wxTreeEvent& event);
diff --git a/src/common/bookctrl.cpp b/src/common/bookctrl.cpp
index 3ed66c98a1..64adcab544 100644
--- a/src/common/bookctrl.cpp
+++ b/src/common/bookctrl.cpp
@@ -493,11 +493,18 @@ int wxBookCtrlBase::DoSetSelection(size_t n, int flags)
         if ( allowed )
         {
             if ( oldSel != wxNOT_FOUND )
-                DoShowPage(DoGetNonNullPage(oldSel), false);
+            {
+                if ( wxWindow* const oldPage = TryGetNonNullPage(oldSel) )
+                {
+                    DoShowPage(oldPage, false);
+                }
+            }
 
-            wxWindow* const page = DoGetNonNullPage(n);
-            page->SetSize(GetPageRect());
-            DoShowPage(page, true);
+            if ( wxWindow* const page = TryGetNonNullPage(n) )
+            {
+                page->SetSize(GetPageRect());
+                DoShowPage(page, true);
+            }
 
             // change selection now to ignore the selection change event
             m_selection = n;
diff --git a/src/generic/treebkg.cpp b/src/generic/treebkg.cpp
index 0abc57f62c..75a7ef8a77 100644
--- a/src/generic/treebkg.cpp
+++ b/src/generic/treebkg.cpp
@@ -542,7 +542,7 @@ void wxTreebook::MakeChangedEvent(wxBookCtrlEvent &event)
     event.SetEventType(wxEVT_TREEBOOK_PAGE_CHANGED);
 }
 
-wxWindow *wxTreebook::DoGetNonNullPage(size_t n)
+wxWindow *wxTreebook::TryGetNonNullPage(size_t n)
 {
     wxWindow* page = wxBookCtrlBase::GetPage(n);
 
diff --git a/tests/controls/treebooktest.cpp b/tests/controls/treebooktest.cpp
index 41ff507a42..bf17ac5ff0 100644
--- a/tests/controls/treebooktest.cpp
+++ b/tests/controls/treebooktest.cpp
@@ -43,11 +43,13 @@ private:
         wxBOOK_CTRL_BASE_TESTS();
         CPPUNIT_TEST( Image );
         CPPUNIT_TEST( SubPages );
+        CPPUNIT_TEST( ContainerPage );
         CPPUNIT_TEST( Expand );
         CPPUNIT_TEST( Delete );
     CPPUNIT_TEST_SUITE_END();
 
     void SubPages();
+    void ContainerPage();
     void Expand();
     void Delete();
 
@@ -92,6 +94,20 @@ void TreebookTestCase::SubPages()
     CPPUNIT_ASSERT_EQUAL(3, m_treebook->GetPageParent(5));
 }
 
+void TreebookTestCase::ContainerPage()
+{
+    // Get rid of the pages added in setUp().
+    m_treebook->DeleteAllPages();
+    CHECK( m_treebook->GetPageCount() == 0 );
+
+    // Adding a page without the associated window should be allowed.
+    REQUIRE_NOTHROW( m_treebook->AddPage(NULL, "Container page") );
+    CHECK( m_treebook->GetPageParent(0) == -1 );
+
+    m_treebook->AddSubPage(new wxPanel(m_treebook), "Child page");
+    CHECK( m_treebook->GetPageParent(1) == 0 );
+}
+
 void TreebookTestCase::Expand()
 {
     wxPanel* subpanel1 = new wxPanel(m_treebook);