Revert "EditorManager: Fix crash when closing an editor upon activation"

Deleting an editor while currentEditorChanged is apparently not a good
idea.
Use a queued connection instead.

This reverts commits 243a6259,
447c4ed3,
d9602ca5 and
31034870.

Change-Id: Iccbee25fb77714963faa6f7184d1f5f53ed348a6
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@digia.com>
Reviewed-by: Eike Ziller <eike.ziller@digia.com>

src/plugins/coreplugin/coreplugin.pro

@@ -237,6 +237,6 @@ else:unix {
 OTHER_FILES += editormanager/BinFiles.mimetypes.xml
 
 equals(TEST, 1) {
-    SOURCES += plugintestutils.cpp
-    HEADERS += plugintestutils.h
+    SOURCES += testdatadir.cpp
+    HEADERS += testdatadir.h
 }

src/plugins/coreplugin/coreplugin.qbs

@@ -181,8 +181,8 @@ QtcPlugin {
         name: "Tests"
         condition: project.testsEnabled
         files: [
-            "plugintestutils.cpp",
-            "plugintestutils.h",
+            "testdatadir.cpp",
+            "testdatadir.h",
         ]
     }
 

src/plugins/coreplugin/editormanager/editormanager.cpp

@@ -1147,7 +1147,7 @@ bool EditorManager::closeEditors(const QList<IEditor*> &editorsToClose, bool ask
     emit m_instance->editorsClosed(acceptedEditors.toList());
 
     foreach (IEditor *editor, acceptedEditors)
-        editor->deleteLater();
+        delete editor;
 
     if (currentView && !currentViewHandled) {
         if (IEditor *editor = currentView->currentEditor())

src/plugins/coreplugin/plugintestutils.cpp → src/plugins/coreplugin/testdatadir.cpp

@@ -28,30 +28,14 @@
 ****************************************************************************/
 
 
-#include "plugintestutils.h"
+#include "testdatadir.h"
 
-#include "editormanager/editormanager.h"
-#include "editormanager/ieditor.h"
-
-#include <QCoreApplication>
 #include <QDir>
 #include <QFileInfo>
 #include <QString>
 #include <QTest>
 
-void Core::Tests::closeAndDeleteEditors(QList<IEditor *> editorsToClose)
-{
-    EditorManager::closeEditors(editorsToClose, /*askAboutModifiedEditors=*/ false);
-    // The editors are going to be deleted by the event loop (deleteLater()),
-    // but for tests we need them deleted now.
-    foreach (Core::IEditor *editor, editorsToClose)
-        QCoreApplication::sendPostedEvents(editor, QEvent::DeferredDelete);
-}
-
-void Core::Tests::closeAndDeleteEditor(Core::IEditor *editor)
-{
-    closeAndDeleteEditors(QList<IEditor *>() << editor);
-}
+using namespace Core::Internal::Tests;
 
 static void maybeAppendSlash(QString *string)
 {
@@ -60,7 +44,7 @@ static void maybeAppendSlash(QString *string)
         string->append(slash);
 }
 
-Core::Tests::TestDataDir::TestDataDir(const QString &directory)
+TestDataDir::TestDataDir(const QString &directory)
     : m_directory(directory)
 {
     maybeAppendSlash(&m_directory);
@@ -69,12 +53,12 @@ Core::Tests::TestDataDir::TestDataDir(const QString &directory)
     QVERIFY(fi.isDir());
 }
 
-QString Core::Tests::TestDataDir::file(const QString &fileName) const
+QString TestDataDir::file(const QString &fileName) const
 {
     return directory() + fileName;
 }
 
-QString Core::Tests::TestDataDir::directory(const QString &subdir, bool clean) const
+QString TestDataDir::directory(const QString &subdir, bool clean) const
 {
     QString path = m_directory;
     if (!subdir.isEmpty())

src/plugins/coreplugin/plugintestutils.h → src/plugins/coreplugin/testdatadir.h

@@ -28,25 +28,17 @@
 ****************************************************************************/
 
 
-#ifndef PLUGINTESTUTILS_H
-#define PLUGINTESTUTILS_H
+#ifndef TESTDATADIR_H
+#define TESTDATADIR_H
 
 #include "core_global.h"
 
-#include <QList>
 #include <QString>
 
 namespace Core {
-
-class IEditor;
-
+namespace Internal {
 namespace Tests {
 
-/// Closing and deleting editors
-void CORE_EXPORT closeAndDeleteEditor(Core::IEditor *editor);
-void CORE_EXPORT closeAndDeleteEditors(QList<Core::IEditor *> editorsToClose);
-
-/// Referencing test data
 class CORE_EXPORT TestDataDir
 {
 public:
@@ -61,6 +53,7 @@ private:
 };
 
 } // namespace Tests
+} // namespace Internal
 } // namespace Core
 
-#endif // PLUGINTESTUTILS_H
+#endif // TESTDATADIR_H

src/plugins/cppeditor/cppdoxygen_test.cpp

@@ -30,7 +30,6 @@
 #include "cppeditor.h"
 
 #include <coreplugin/editormanager/editormanager.h>
-#include <coreplugin/plugintestutils.h>
 #include <cplusplus/CppDocument.h>
 #include <cppeditor/cppeditor.h>
 #include <cppeditor/cppeditorplugin.h>
@@ -116,7 +115,8 @@ TestCase::TestCase(const QByteArray &input)
 
 TestCase::~TestCase()
 {
-    Core::Tests::closeAndDeleteEditor(editor);
+    EditorManager::closeEditor(editor, false);
+    QCoreApplication::processEvents(); // process any pending events
 
     // Remove the test file from the code-model
     CppTools::CppModelManagerInterface *mmi = CppTools::CppModelManagerInterface::instance();

src/plugins/cppeditor/cppquickfix_test.cpp

@@ -34,7 +34,6 @@
 #include "cppquickfixassistant.h"
 #include "cppquickfixes.h"
 
-#include <coreplugin/plugintestutils.h>
 #include <cpptools/cppcodestylepreferences.h>
 #include <cpptools/cppmodelmanager.h>
 #include <cpptools/cpppreprocessor.h>
@@ -239,7 +238,6 @@ void TestCase::init(const QStringList &includePaths)
 
         // Rehighlight
         testFile->editorWidget->semanticRehighlight(true);
-
         // Wait for the semantic info from the future
         while (testFile->editorWidget->semanticInfo().doc.isNull())
             QCoreApplication::processEvents();
@@ -265,7 +263,8 @@ TestCase::~TestCase()
         if (testFile->editor)
             editorsToClose << testFile->editor;
     }
-    Core::Tests::closeAndDeleteEditors(editorsToClose);
+    EditorManager::closeEditors(editorsToClose, false);
+    QCoreApplication::processEvents(); // process any pending events
 
     // Remove the test files from the code-model
     CppModelManagerInterface *mmi = CppModelManagerInterface::instance();

src/plugins/cppeditor/followsymbol_switchmethoddecldef_test.cpp

@@ -32,7 +32,6 @@
 #include "cppelementevaluator.h"
 #include "cppvirtualfunctionassistprovider.h"
 
-#include <coreplugin/plugintestutils.h>
 #include <texteditor/codeassist/iassistproposal.h>
 #include <texteditor/codeassist/iassistprocessor.h>
 #include <texteditor/codeassist/basicproposalitemlistmodel.h>
@@ -325,7 +324,8 @@ TestCase::~TestCase()
         if (testFile->editor)
             editorsToClose << testFile->editor;
     }
-    Core::Tests::closeAndDeleteEditors(editorsToClose);
+    EditorManager::closeEditors(editorsToClose, false);
+    QCoreApplication::processEvents(); // process any pending events
 
     // Remove the test files from the code-model
     CppModelManagerInterface *mmi = CppTools::CppModelManagerInterface::instance();

src/plugins/cpptools/cppcompletion_test.cpp

@@ -31,7 +31,6 @@
 #include "cppcompletionassist.h"
 #include "cppmodelmanager.h"
 
-#include <coreplugin/plugintestutils.h>
 #include <texteditor/plaintexteditor.h>
 #include <texteditor/codeassist/iassistproposal.h>
 #include <texteditor/convenience.h>
@@ -92,7 +91,7 @@ public:
 
     ~CompletionTestCase()
     {
-        Core::Tests::closeAndDeleteEditor(editor);
+        EditorManager::closeEditor(editor, /*askAboutModifiedEditors=*/ false);
         cmm->GC();
         QVERIFY(cmm->snapshot().isEmpty());
     }

src/plugins/cpptools/cppheadersource_test.cpp

@@ -30,7 +30,7 @@
 #include "cpptoolsplugin.h"
 #include "cpptoolsreuse.h"
 
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 
 #include <QDir>
 #include <QtTest>
@@ -46,7 +46,7 @@ void CppToolsPlugin::test_headersource()
     QFETCH(QString, headerFileName);
 
     bool wasHeader;
-    Core::Tests::TestDataDir dataDir(
+    Core::Internal::Tests::TestDataDir dataDir(
                 _(SRCDIR "/../../../tests/cppheadersource/") + _(QTest::currentDataTag()));
 
     const QString sourcePath = dataDir.file(sourceFileName);

src/plugins/cpptools/cpplocatorfilter_test.cpp

@@ -36,7 +36,7 @@
 #include "cppmodelmanager.h"
 
 #include <coreplugin/editormanager/editormanager.h>
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 #include <extensionsystem/pluginmanager.h>
 #include <locator/locatorfiltertest.h>
 #include <utils/fileutils.h>
@@ -46,6 +46,7 @@
 #include <QtTest>
 
 using namespace Core;
+using namespace Core::Internal::Tests;
 using namespace CppTools::Internal;
 using namespace ExtensionSystem;
 using namespace Locator;
@@ -57,7 +58,7 @@ Q_DECLARE_METATYPE(ILocatorFilter *)
 
 namespace {
 
-class MyTestDataDir : public Core::Tests::TestDataDir
+class MyTestDataDir : public Core::Internal::Tests::TestDataDir
 {
 public:
     MyTestDataDir(const QString &testDataDirectory)
@@ -124,7 +125,8 @@ private:
 
     virtual void doAfterLocatorRun()
     {
-        Core::Tests::closeAndDeleteEditor(m_editor);
+        EditorManager::closeEditor(m_editor, /*askAboutModifiedEditors=*/ false);
+        QCoreApplication::processEvents();
         QVERIFY(EditorManager::documentModel()->openedDocuments().isEmpty());
         m_modelManager->GC();
         QVERIFY(m_modelManager->snapshot().isEmpty());

src/plugins/cpptools/cppmodelmanager_test.cpp

@@ -33,7 +33,7 @@
 #include "modelmanagertesthelper.h"
 
 #include <coreplugin/editormanager/editormanager.h>
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 #include <projectexplorer/projectexplorer.h>
 #include <projectexplorer/session.h>
 #include <utils/hostosinfo.h>
@@ -62,7 +62,7 @@ namespace {
 
 inline QString _(const QByteArray &ba) { return QString::fromLatin1(ba, ba.size()); }
 
-class MyTestDataDir : public Core::Tests::TestDataDir
+class MyTestDataDir : public Core::Internal::Tests::TestDataDir
 {
 public:
     MyTestDataDir(const QString &dir)
@@ -723,7 +723,7 @@ void CppToolsPlugin::test_modelmanager_gc_if_last_cppeditor_closed()
     helper.waitForRefreshedSourceFiles();
 
     // Close file/editor
-    Core::Tests::closeAndDeleteEditor(editor);
+    Core::EditorManager::closeEditor(editor, /*askAboutModifiedEditors=*/ false);
     helper.waitForFinishedGc();
 
     // Check: File is removed from the snapshpt
@@ -760,7 +760,7 @@ void CppToolsPlugin::test_modelmanager_dont_gc_opened_files()
     QVERIFY(mm->snapshot().contains(file));
 
     // Close editor
-    Core::Tests::closeAndDeleteEditor(editor);
+    Core::EditorManager::closeEditor(editor);
     helper.waitForFinishedGc();
     QVERIFY(mm->snapshot().isEmpty());
 }
@@ -772,7 +772,7 @@ struct EditorCloser {
     ~EditorCloser()
     {
         if (editor)
-            Core::Tests::closeAndDeleteEditor(editor);
+            Core::EditorManager::closeEditor(editor);
     }
 };
 

src/plugins/cpptools/symbolsearcher_test.cpp

@@ -33,7 +33,7 @@
 #include "cppmodelmanager.h"
 #include "searchsymbols.h"
 
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 #include <utils/runextensions.h>
 
 #include <QtTest>
@@ -43,7 +43,7 @@ using namespace CppTools::Internal;
 
 namespace {
 
-class MyTestDataDir : public Core::Tests::TestDataDir
+class MyTestDataDir : public Core::Internal::Tests::TestDataDir
 {
 public:
     MyTestDataDir(const QString &testDataDirectory)

src/plugins/designer/gotoslot_test.cpp

@@ -34,7 +34,7 @@
 #else
 #include "formeditorw.h"
 
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 #include <coreplugin/editormanager/editormanager.h>
 #include <cpptools/cppmodelmanager.h>
 
@@ -47,6 +47,7 @@
 #include <QtTest>
 
 using namespace Core;
+using namespace Core::Internal::Tests;
 using namespace CppTools;
 using namespace CPlusPlus;
 using namespace Designer;
@@ -54,7 +55,7 @@ using namespace Designer::Internal;
 
 namespace {
 
-class MyTestDataDir : public Core::Tests::TestDataDir {
+class MyTestDataDir : public Core::Internal::Tests::TestDataDir {
 public:
     MyTestDataDir(const QString &dir)
         : TestDataDir(QLatin1String(SRCDIR "/../../../tests/designer/") + dir)
@@ -133,10 +134,7 @@ public:
 private:
     void cleanup()
     {
-        DocumentModel *documentModel = EditorManager::documentModel();
-        const QList<IDocument *> documents = documentModel->openedDocuments();
-        const QList<IEditor *> editors = documentModel->editorsForDocuments(documents);
-        Core::Tests::closeAndDeleteEditors(editors);
+        EditorManager::closeAllEditors(/*askAboutModifiedEditors =*/ false);
         QVERIFY(EditorManager::documentModel()->openedDocuments().isEmpty());
 
         m_modelManager->GC();

src/plugins/locator/locator_test.cpp

@@ -33,7 +33,7 @@
 #include "basefilefilter.h"
 #include "locatorfiltertest.h"
 
-#include <coreplugin/plugintestutils.h>
+#include <coreplugin/testdatadir.h>
 #include <utils/fileutils.h>
 
 #include <QDir>
@@ -44,7 +44,7 @@ using namespace Locator::Internal::Tests;
 
 namespace {
 
-class MyTestDataDir : public Core::Tests::TestDataDir
+class MyTestDataDir : public Core::Internal::Tests::TestDataDir
 {
 public:
     MyTestDataDir(const QString &testDataDirectory)

src/plugins/vcsbase/vcsbasesubmiteditor.cpp

@@ -234,9 +234,11 @@ VcsBaseSubmitEditor::VcsBaseSubmitEditor(const VcsBaseSubmitEditorParameters *pa
     connect(VcsPlugin::instance(),
             SIGNAL(settingsChanged(VcsBase::Internal::CommonVcsSettings)),
             this, SLOT(slotUpdateEditorSettings(VcsBase::Internal::CommonVcsSettings)));
+    // Commit data refresh might lead to closing the editor, so use a queued connection
     connect(Core::EditorManager::instance(), SIGNAL(currentEditorChanged(Core::IEditor*)),
-            this, SLOT(slotRefreshCommitData()));
-    connect(Core::ICore::mainWindow(), SIGNAL(windowActivated()), this, SLOT(slotRefreshCommitData()));
+            this, SLOT(slotRefreshCommitData()), Qt::QueuedConnection);
+    connect(Core::ICore::mainWindow(), SIGNAL(windowActivated()),
+            this, SLOT(slotRefreshCommitData()), Qt::QueuedConnection);
 
     Aggregation::Aggregate *aggregate = new Aggregation::Aggregate;
     aggregate->add(new Find::BaseTextFind(descriptionEdit));