Fix crash when canceling save of new database (#2601)

* Fix crash when canceling save of new database
* Standardize use of DatabaseWidget::save() function
* Close new database tabs that are "discarded"
* Fixes #2604. autoSaveOnExit setting
* Re-implement autosave functionality

src/core/Config.cpp

@@ -100,6 +100,11 @@ void Config::upgrade()
             m_settings->remove(setting);
         }
     }
+
+    // > 2.3.4
+    if (m_settings->value("AutoSaveAfterEveryChange").toBool()) {
+        m_settings->setValue("AutoSaveOnExit", true);
+    }
 }
 
 Config::Config(const QString& fileName, QObject* parent)
@@ -167,7 +172,7 @@ void Config::init(const QString& fileName)
     m_defaults.insert("OpenPreviousDatabasesOnStartup", true);
     m_defaults.insert("AutoSaveAfterEveryChange", true);
     m_defaults.insert("AutoReloadOnChange", true);
-    m_defaults.insert("AutoSaveOnExit", false);
+    m_defaults.insert("AutoSaveOnExit", true);
     m_defaults.insert("BackupBeforeSave", false);
     m_defaults.insert("UseAtomicSaves", true);
     m_defaults.insert("SearchLimitGroup", false);

src/gui/ApplicationSettingsWidget.cpp

@@ -79,9 +79,9 @@ ApplicationSettingsWidget::ApplicationSettingsWidget(QWidget* parent)
     connect(this, SIGNAL(rejected()), SLOT(reject()));
 
     // clang-format off
-    connect(m_generalUi->autoSaveAfterEveryChangeCheckBox, SIGNAL(toggled(bool)), SLOT(enableAutoSaveOnExit(bool)));
-    connect(m_generalUi->systrayShowCheckBox, SIGNAL(toggled(bool)), this, SLOT(enableSystray(bool)));
-    connect(m_generalUi->toolbarHideCheckBox, SIGNAL(toggled(bool)), this, SLOT(enableToolbarSettings(bool)));
+    connect(m_generalUi->autoSaveAfterEveryChangeCheckBox, SIGNAL(toggled(bool)), SLOT(autoSaveToggled(bool)));
+    connect(m_generalUi->systrayShowCheckBox, SIGNAL(toggled(bool)), SLOT(systrayToggled(bool)));
+    connect(m_generalUi->toolbarHideCheckBox, SIGNAL(toggled(bool)), SLOT(enableToolbarSettings(bool)));
 
     connect(m_secUi->clearClipboardCheckBox, SIGNAL(toggled(bool)),
             m_secUi->clearClipboardSpinBox, SLOT(setEnabled(bool)));
@@ -308,12 +308,16 @@ void ApplicationSettingsWidget::reject()
     }
 }
 
-void ApplicationSettingsWidget::enableAutoSaveOnExit(bool checked)
+void ApplicationSettingsWidget::autoSaveToggled(bool checked)
 {
+    // Explicitly enable auto-save on exit if it wasn't already
+    if (checked && !m_generalUi->autoSaveOnExitCheckBox->isChecked()) {
+        m_generalUi->autoSaveOnExitCheckBox->setChecked(true);
+    }
     m_generalUi->autoSaveOnExitCheckBox->setEnabled(!checked);
 }
 
-void ApplicationSettingsWidget::enableSystray(bool checked)
+void ApplicationSettingsWidget::systrayToggled(bool checked)
 {
     m_generalUi->systrayDarkIconCheckBox->setEnabled(checked);
     m_generalUi->systrayMinimizeToTrayCheckBox->setEnabled(checked);

src/gui/ApplicationSettingsWidget.h

@@ -53,8 +53,8 @@ public:
 private slots:
     void saveSettings();
     void reject();
-    void enableAutoSaveOnExit(bool checked);
-    void enableSystray(bool checked);
+    void autoSaveToggled(bool checked);
+    void systrayToggled(bool checked);
     void enableToolbarSettings(bool checked);
 
 private:

src/gui/DatabaseTabWidget.cpp

@@ -515,9 +515,12 @@ DatabaseWidget* DatabaseTabWidget::currentDatabaseWidget()
 void DatabaseTabWidget::lockDatabases()
 {
     for (int i = 0, c = count(); i < c; ++i) {
-        if (!databaseWidgetFromIndex(i)->lock()) {
-            return;
+        auto dbWidget = databaseWidgetFromIndex(i);
+        if (dbWidget->lock() && dbWidget->database()->filePath().isEmpty()) {
+            // If we locked a database without a file close the tab
+            closeDatabaseTab(dbWidget);
         }
+
     }
 }
 

src/gui/DatabaseWidget.cpp

@@ -182,6 +182,8 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
 
     connectDatabaseSignals();
 
+    m_blockAutoSave = false;
+
     m_EntrySearcher = new EntrySearcher(false);
     m_searchLimitGroup = config()->get("SearchLimitGroup", false).toBool();
 
@@ -813,10 +815,10 @@ void DatabaseWidget::switchToGroupEdit(Group* group, bool create)
 void DatabaseWidget::connectDatabaseSignals()
 {
     // relayed Database events
-    connect(m_db.data(), SIGNAL(filePathChanged(QString,QString)),
-            this, SIGNAL(databaseFilePathChanged(QString,QString)));
-    connect(m_db.data(), SIGNAL(databaseModified()), this, SIGNAL(databaseModified()));
-    connect(m_db.data(), SIGNAL(databaseSaved()), this, SIGNAL(databaseSaved()));
+    connect(m_db.data(), SIGNAL(filePathChanged(QString,QString)), SIGNAL(databaseFilePathChanged(QString,QString)));
+    connect(m_db.data(), SIGNAL(databaseModified()), SIGNAL(databaseModified()));
+    connect(m_db.data(), SIGNAL(databaseModified()), SLOT(onDatabaseModified()));
+    connect(m_db.data(), SIGNAL(databaseSaved()), SIGNAL(databaseSaved()));
 }
 
 void DatabaseWidget::loadDatabase(bool accepted)
@@ -1114,6 +1116,15 @@ void DatabaseWidget::onGroupChanged(Group* group)
     m_previewView->setGroup(group);
 }
 
+void DatabaseWidget::onDatabaseModified()
+{
+    if (!m_blockAutoSave && config()->get("AutoSaveAfterEveryChange").toBool()) {
+        save();
+    }
+
+    m_blockAutoSave = false;
+}
+
 QString DatabaseWidget::getCurrentSearch()
 {
     return m_lastSearchText;
@@ -1207,10 +1218,10 @@ bool DatabaseWidget::lock()
 
     if (m_db->isModified()) {
         if (config()->get("AutoSaveOnExit").toBool()) {
-            if (!m_db->save(nullptr, false, false)) {
+            if (!save()) {
                 return false;
             }
-        } else if (!isLocked()) {
+        } else {
             QString msg;
             if (!m_db->metadata()->name().toHtmlEscaped().isEmpty()) {
                 msg = tr("\"%1\" was modified.\nSave changes?").arg(m_db->metadata()->name().toHtmlEscaped());
@@ -1219,8 +1230,10 @@ bool DatabaseWidget::lock()
             }
             auto result = MessageBox::question(this, tr("Save changes?"), msg,
                 MessageBox::Save | MessageBox::Discard | MessageBox::Cancel, MessageBox::Save);
-            if (result == MessageBox::Save && !m_db->save(nullptr, false, false)) {
-                return false;
+            if (result == MessageBox::Save) {
+                if (!save()) {
+                    return false;
+                }
             } else if (result == MessageBox::Cancel) {
                 return false;
             }
@@ -1270,6 +1283,8 @@ void DatabaseWidget::reloadDatabaseFile()
         return;
     }
 
+    m_blockAutoSave = true;
+
     if (!config()->get("AutoReloadOnChange").toBool()) {
         // Ask if we want to reload the db
         auto result = MessageBox::question(this,

src/gui/DatabaseWidget.h

@@ -209,6 +209,7 @@ private slots:
     void emitEntryContextMenuRequested(const QPoint& pos);
     void onEntryChanged(Entry* entry);
     void onGroupChanged(Group* group);
+    void onDatabaseModified();
     void connectDatabaseSignals();
     void loadDatabase(bool accepted);
     void unlockDatabase(bool accepted);
@@ -257,6 +258,7 @@ private:
 
     // Autoreload
     QPointer<DelayingFileWatcher> m_fileWatcher;
+    bool m_blockAutoSave;
 };
 
 #endif // KEEPASSX_DATABASEWIDGET_H

tests/gui/TestGui.cpp

@@ -81,6 +81,7 @@ void TestGui::initTestCase()
     Config::createTempFileInstance();
     // Disable autosave so we can test the modified file indicator
     config()->set("AutoSaveAfterEveryChange", false);
+    config()->set("AutoSaveOnExit", false);
     // Enable the tray icon so we can test hiding/restoring the windowQByteArray
     config()->set("GUI/ShowTrayIcon", true);
     // Disable advanced settings mode (activate within individual tests to test advanced settings)