From ee6d58abe20ac0195932941f46cc8ab633b86d6a Mon Sep 17 00:00:00 2001
From: Vadim Zeitlin <vadim@wxwidgets.org>
Date: Thu, 14 Jul 2022 23:52:03 +0100
Subject: [PATCH] Don't call wxDocument::OnCloseDocument() twice when closing

When closing the document using wxDocManager::CloseDocument(), which is
what happens when using the standard wxID_CLOSE menu command handler,
OnCloseDocument() was called twice: first when wxDocument::Close() was
called directly from CloseDocument() itself and once again later when it
was implicitly called from DeleteAllViews().

This was unexpected and also resulted in calling DeleteContents() twice,
which is even more so for a function supposed to perform cleanup.

Fix this by not calling Close() itself but instead a new CanClose()
function from CloseDocument() to check whether we can close and then
relying on DeleteAllViews() to actually close the document.
---
 docs/changes.txt       |  6 ++++++
 include/wx/docview.h   |  6 ++++++
 src/common/docview.cpp | 18 +++++++++++++-----
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/docs/changes.txt b/docs/changes.txt
index 969bb9ee95..898516b8f5 100644
--- a/docs/changes.txt
+++ b/docs/changes.txt
@@ -11,6 +11,12 @@ INCOMPATIBLE CHANGES SINCE 3.2.x:
 Changes in behaviour not resulting in compilation errors
 --------------------------------------------------------
 
+- wxDocument::OnCloseDocument() was called twice in previous versions when
+  closing the document from the menu. Now it is only called once and after
+  destroying all the existing document views. If you overrode this function,
+  please check that you don't rely on any views existing when it's called.
+
+
 Changes in behaviour which may result in build errors
 -----------------------------------------------------
 
diff --git a/include/wx/docview.h b/include/wx/docview.h
index c969da7fe3..1d7d329a3c 100644
--- a/include/wx/docview.h
+++ b/include/wx/docview.h
@@ -188,6 +188,12 @@ public:
     // part of the parent document and not a disk file as usual.
     bool IsChildDocument() const { return m_documentParent != NULL; }
 
+    // Ask the user if the document should be saved if it's modified and save
+    // it if necessary.
+    //
+    // Returns false if the user cancelled closing or if saving failed.
+    bool CanClose();
+
 protected:
     wxList                m_documentViews;
     wxString              m_documentFile;
diff --git a/src/common/docview.cpp b/src/common/docview.cpp
index d6a7349cde..fc6ecd445a 100644
--- a/src/common/docview.cpp
+++ b/src/common/docview.cpp
@@ -144,15 +144,14 @@ wxDocument::~wxDocument()
     //DeleteAllViews();
 }
 
-bool wxDocument::Close()
+bool wxDocument::CanClose()
 {
     if ( !OnSaveModified() )
         return false;
 
     // When the parent document closes, its children must be closed as well as
-    // they can't exist without the parent.
+    // they can't exist without the parent, so ask them too.
 
-    // As usual, first check if all children can be closed.
     DocsList::const_iterator it = m_childDocuments.begin();
     for ( DocsList::const_iterator end = m_childDocuments.end(); it != end; ++it )
     {
@@ -163,6 +162,15 @@ bool wxDocument::Close()
         }
     }
 
+    return true;
+}
+
+bool wxDocument::Close()
+{
+    // First check if this document itself and all its children can be closed.
+    if ( !CanClose() )
+        return false;
+
     // Now that they all did, do close them: as m_childDocuments is modified as
     // we iterate over it, don't use the usual for-style iteration here.
     while ( !m_childDocuments.empty() )
@@ -996,12 +1004,12 @@ wxDocManager::~wxDocManager()
 // closes the specified document
 bool wxDocManager::CloseDocument(wxDocument* doc, bool force)
 {
-    if ( !doc->Close() && !force )
+    if ( !doc->CanClose() && !force )
         return false;
 
     // To really force the document to close, we must ensure that it isn't
     // modified, otherwise it would ask the user about whether it should be
-    // destroyed (again, it had been already done by Close() above) and might
+    // destroyed (again, it had been already done by CanClose() above) and might
     // not destroy it at all, while we must do it here.
     doc->Modify(false);