Fix that removing splits results in code model issues

The code model is listening to editorsClosed signals, and checks for
which editors are actually visible when that signal is sent.
For this to work correctly, the signal must be sent only at the very end
after deconstructing all the views, because otherwise we might be in a
state where temporarily no editors are "visible".

Fix-up after 80ae992c91985389f6d870ac0201c16d895bc5cc which exposes the
issue.

Task-number: QTCREATORBUG-20464
Change-Id: I9ec5e579d681f2c12924be3d7b89aab398579505
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@qt.io>

4 files changed, 48 insertions, 23 deletions

diff --git a/src/plugins/coreplugin/editormanager/editormanager.cpp b/src/plugins/coreplugin/editormanager/editormanager.cpp
index ce67e84d58..da0772dc44 100644
--- a/src/plugins/coreplugin/editormanager/editormanager.cpp
+++ b/src/plugins/coreplugin/editormanager/editormanager.cpp
@@ -1537,7 +1537,7 @@ void EditorManagerPrivate::closeView(EditorView *view)
     if (!view)
         return;
 
-    emptyView(view);
+    const QList<IEditor *> editorsToDelete = emptyView(view);
 
     SplitterOrView *splitterOrView = view->parentSplitterOrView();
     Q_ASSERT(splitterOrView);
@@ -1552,15 +1552,23 @@ void EditorManagerPrivate::closeView(EditorView *view)
     EditorView *newCurrent = splitter->findFirstView();
     if (newCurrent)
         EditorManagerPrivate::activateView(newCurrent);
+    deleteEditors(editorsToDelete);
 }
 
-void EditorManagerPrivate::emptyView(EditorView *view)
+/*!
+    Removes all editors from the view and from the document model, taking care of
+    the handling of editors that are the last ones for the document.
+    Returns the list of editors that were actually removed from the document model and
+    need to be deleted with EditorManagerPrivate::deleteEditors.
+    \internal
+*/
+const QList<IEditor *> EditorManagerPrivate::emptyView(EditorView *view)
 {
     if (!view)
-        return;
-
-    QList<IEditor *> editors = view->editors();
-    foreach (IEditor *editor, editors) {
+        return {};
+    const QList<IEditor *> editors = view->editors();
+    QList<IEditor *> removedEditors;
+    for (IEditor *editor : editors) {
         if (DocumentModel::editorsForDocument(editor->document()).size() == 1) {
             // it's the only editor for that file
             // so we need to keep it around (--> in the editor model)
@@ -1569,19 +1577,26 @@ void EditorManagerPrivate::emptyView(EditorView *view)
                 setCurrentView(view);
                 setCurrentEditor(nullptr);
             }
-            editors.removeAll(editor);
             view->removeEditor(editor);
-            continue; // don't close the editor
+        } else {
+            emit m_instance->editorAboutToClose(editor);
+            removeEditor(editor, true /*=removeSuspendedEntry, but doesn't matter since it's not the last editor anyhow*/);
+            view->removeEditor(editor);
+            removedEditors.append(editor);
         }
-        emit m_instance->editorAboutToClose(editor);
-        removeEditor(editor, true /*=removeSuspendedEntry, but doesn't matter since it's not the last editor anyhow*/);
-        view->removeEditor(editor);
     }
+    return removedEditors;
+}
+
+/*!
+    Signals editorsClosed and deletes the editors.
+    \internal
+*/
+void EditorManagerPrivate::deleteEditors(const QList<IEditor *> &editors)
+{
     if (!editors.isEmpty()) {
         emit m_instance->editorsClosed(editors);
-        foreach (IEditor *editor, editors) {
-            delete editor;
-        }
+        qDeleteAll(editors);
     }
 }
 
diff --git a/src/plugins/coreplugin/editormanager/editormanager_p.h b/src/plugins/coreplugin/editormanager/editormanager_p.h
index 9b29d9d3ca..07c81e9ca3 100644
--- a/src/plugins/coreplugin/editormanager/editormanager_p.h
+++ b/src/plugins/coreplugin/editormanager/editormanager_p.h
@@ -126,7 +126,8 @@ public:
     static EditorWindow *createEditorWindow();
     static void splitNewWindow(Internal::EditorView *view);
     static void closeView(Internal::EditorView *view);
-    static void emptyView(Internal::EditorView *view);
+    static const QList<IEditor *> emptyView(Internal::EditorView *view);
+    static void deleteEditors(const QList<IEditor *> &editors);
 
     static void updateActions();
 
diff --git a/src/plugins/coreplugin/editormanager/editorview.cpp b/src/plugins/coreplugin/editormanager/editorview.cpp
index 8c35d26527..1394572c57 100644
--- a/src/plugins/coreplugin/editormanager/editorview.cpp
+++ b/src/plugins/coreplugin/editormanager/editorview.cpp
@@ -641,7 +641,7 @@ SplitterOrView::~SplitterOrView()
     delete m_layout;
     m_layout = nullptr;
     if (m_view)
-        EditorManagerPrivate::emptyView(m_view);
+        EditorManagerPrivate::deleteEditors(EditorManagerPrivate::emptyView(m_view));
     delete m_view;
     m_view = nullptr;
     delete m_splitter;
@@ -772,7 +772,7 @@ void SplitterOrView::unsplitAll()
     }
     m_splitter->hide();
     m_layout->removeWidget(m_splitter); // workaround Qt bug
-    unsplitAll_helper();
+    const QList<IEditor *> editorsToDelete = unsplitAll_helper();
     m_view = currentView;
     m_layout->addWidget(m_view);
     delete m_splitter;
@@ -785,19 +785,27 @@ void SplitterOrView::unsplitAll()
         else
             m_view->setFocus();
     }
+    EditorManagerPrivate::deleteEditors(editorsToDelete);
     emit splitStateChanged();
 }
 
-void SplitterOrView::unsplitAll_helper()
+/*!
+    Recursively empties all views.
+    Returns the editors to delete with EditorManagerPrivate::deleteEditors.
+    \internal
+*/
+const QList<IEditor *> SplitterOrView::unsplitAll_helper()
 {
     if (m_view)
-        EditorManagerPrivate::emptyView(m_view);
+        return EditorManagerPrivate::emptyView(m_view);
+    QList<IEditor *> editorsToDelete;
     if (m_splitter) {
         for (int i = 0; i < m_splitter->count(); ++i) {
             if (SplitterOrView *splitterOrView = qobject_cast<SplitterOrView*>(m_splitter->widget(i)))
-                splitterOrView->unsplitAll_helper();
+                editorsToDelete.append(splitterOrView->unsplitAll_helper());
         }
     }
+    return editorsToDelete;
 }
 
 void SplitterOrView::unsplit()
@@ -809,7 +817,7 @@ void SplitterOrView::unsplit()
     SplitterOrView *childSplitterOrView = qobject_cast<SplitterOrView*>(m_splitter->widget(0));
     QSplitter *oldSplitter = m_splitter;
     m_splitter = nullptr;
-
+    QList<IEditor *> editorsToDelete;
     if (childSplitterOrView->isSplitter()) {
         Q_ASSERT(childSplitterOrView->view() == nullptr);
         m_splitter = childSplitterOrView->takeSplitter();
@@ -825,7 +833,7 @@ void SplitterOrView::unsplit()
                 m_view->addEditor(e);
                 m_view->setCurrentEditor(e);
             }
-            EditorManagerPrivate::emptyView(childView);
+            editorsToDelete = EditorManagerPrivate::emptyView(childView);
         } else {
             m_view = childSplitterOrView->takeView();
             m_view->setParentSplitterOrView(this);
@@ -849,6 +857,7 @@ void SplitterOrView::unsplit()
         EditorManagerPrivate::activateView(newCurrent);
     else
         EditorManagerPrivate::setCurrentView(nullptr);
+    EditorManagerPrivate::deleteEditors(editorsToDelete);
     emit splitStateChanged();
 }
 
diff --git a/src/plugins/coreplugin/editormanager/editorview.h b/src/plugins/coreplugin/editormanager/editorview.h
index 3d0c95e6a6..457df776e5 100644
--- a/src/plugins/coreplugin/editormanager/editorview.h
+++ b/src/plugins/coreplugin/editormanager/editorview.h
@@ -202,7 +202,7 @@ signals:
     void splitStateChanged();
 
 private:
-    void unsplitAll_helper();
+    const QList<IEditor *> unsplitAll_helper();
     QStackedLayout *m_layout;
     EditorView *m_view;
     QSplitter *m_splitter;