Merge bitcoin-core/gui#342: wallet: Move wallets loading out from the main GUI thread

2fe69ef qt, wallet: Drop no longer used WalletController::getOpenWallets() (Hennadii Stepanov)
f6991cb qt, wallet: Add LoadWalletsActivity class (Hennadii Stepanov)
4a024fc qt, wallet, refactor: Move connection to QObject::deleteLater to ctor (Hennadii Stepanov)
f9b633e qt, wallet: Move activity progress dialog from data member to local (Hennadii Stepanov)

Pull request description:

  This PR improves the GUI responsiveness during initial wallets loading at startup (especially ones that have tons of txs), and shows a standard progress dialog for long loading:

  ![DeepinScreenshot_select-area_20210522230626](https://user-images.githubusercontent.com/32963518/119239625-0b3a9380-bb53-11eb-9a54-34980d8a1194.png)

  Fixes #247.

ACKs for top commit:
  ryanofsky:
    Code review ACK 2fe69ef. Just suggested changes since last review: squashing commits and dropping unused method (thanks!)
  shaavan:
    reACK 2fe69ef
  promag:
    Code review ACK 2fe69ef.

Tree-SHA512: 2ac3cb48886e0005fc36b3fd0c2b35abd557186be16db3145d753c34d94188e4f4ff14dc07fb0fb7558944f84498204a3988f8284fd56c6d85b47bc9081e71a6

Author: hebasto

src/qt/bitcoingui.cpp

@@ -107,7 +107,6 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
         walletFrame = new WalletFrame(_platformStyle, this);
         connect(walletFrame, &WalletFrame::createWalletButtonClicked, [this] {
             auto activity = new CreateWalletActivity(getWalletController(), this);
-            connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater);
             activity->create();
         });
         connect(walletFrame, &WalletFrame::message, [this](const QString& title, const QString& message, unsigned int style) {
@@ -418,7 +417,6 @@ void BitcoinGUI::createActions()
                 connect(action, &QAction::triggered, [this, path] {
                     auto activity = new OpenWalletActivity(m_wallet_controller, this);
                     connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet);
-                    connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater);
                     activity->open(path);
                 });
             }
@@ -433,7 +431,6 @@ void BitcoinGUI::createActions()
         connect(m_create_wallet_action, &QAction::triggered, [this] {
             auto activity = new CreateWalletActivity(m_wallet_controller, this);
             connect(activity, &CreateWalletActivity::created, this, &BitcoinGUI::setCurrentWallet);
-            connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater);
             activity->create();
         });
         connect(m_close_all_wallets_action, &QAction::triggered, [this] {
@@ -664,9 +661,8 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller)
     GUIUtil::ExceptionSafeConnect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
     connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);
 
-    for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) {
-        addWallet(wallet_model);
-    }
+    auto activity = new LoadWalletsActivity(m_wallet_controller, this);
+    activity->load();
 }
 
 WalletController* BitcoinGUI::getWalletController()

src/qt/walletcontroller.cpp

@@ -41,10 +41,6 @@ WalletController::WalletController(ClientModel& client_model, const PlatformStyl
         getOrCreateWallet(std::move(wallet));
     });
 
-    for (std::unique_ptr<interfaces::Wallet>& wallet : m_node.walletClient().getWallets()) {
-        getOrCreateWallet(std::move(wallet));
-    }
-
     m_activity_worker->moveToThread(m_activity_thread);
     m_activity_thread->start();
     QTimer::singleShot(0, m_activity_worker, []() {
@@ -61,12 +57,6 @@ WalletController::~WalletController()
     delete m_activity_worker;
 }
 
-std::vector<WalletModel*> WalletController::getOpenWallets() const
-{
-    QMutexLocker locker(&m_mutex);
-    return m_wallets;
-}
-
 std::map<std::string, bool> WalletController::listWalletDir() const
 {
     QMutexLocker locker(&m_mutex);
@@ -191,33 +181,23 @@ WalletControllerActivity::WalletControllerActivity(WalletController* wallet_cont
     , m_wallet_controller(wallet_controller)
     , m_parent_widget(parent_widget)
 {
-}
-
-WalletControllerActivity::~WalletControllerActivity()
-{
-    delete m_progress_dialog;
+    connect(this, &WalletControllerActivity::finished, this, &QObject::deleteLater);
 }
 
 void WalletControllerActivity::showProgressDialog(const QString& label_text)
 {
-    assert(!m_progress_dialog);
-    m_progress_dialog = new QProgressDialog(m_parent_widget);
-
-    m_progress_dialog->setLabelText(label_text);
-    m_progress_dialog->setRange(0, 0);
-    m_progress_dialog->setCancelButton(nullptr);
-    m_progress_dialog->setWindowModality(Qt::ApplicationModal);
-    GUIUtil::PolishProgressDialog(m_progress_dialog);
+    auto progress_dialog = new QProgressDialog(m_parent_widget);
+    progress_dialog->setAttribute(Qt::WA_DeleteOnClose);
+    connect(this, &WalletControllerActivity::finished, progress_dialog, &QWidget::close);
+
+    progress_dialog->setLabelText(label_text);
+    progress_dialog->setRange(0, 0);
+    progress_dialog->setCancelButton(nullptr);
+    progress_dialog->setWindowModality(Qt::ApplicationModal);
+    GUIUtil::PolishProgressDialog(progress_dialog);
     // The setValue call forces QProgressDialog to start the internal duration estimation.
     // See details in https://bugreports.qt.io/browse/QTBUG-47042.
-    m_progress_dialog->setValue(0);
-}
-
-void WalletControllerActivity::destroyProgressDialog()
-{
-    assert(m_progress_dialog);
-    delete m_progress_dialog;
-    m_progress_dialog = nullptr;
+    progress_dialog->setValue(0);
 }
 
 CreateWalletActivity::CreateWalletActivity(WalletController* wallet_controller, QWidget* parent_widget)
@@ -279,8 +259,6 @@ void CreateWalletActivity::createWallet()
 
 void CreateWalletActivity::finish()
 {
-    destroyProgressDialog();
-
     if (!m_error_message.empty()) {
         QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message.translated));
     } else if (!m_warning_message.empty()) {
@@ -329,8 +307,6 @@ OpenWalletActivity::OpenWalletActivity(WalletController* wallet_controller, QWid
 
 void OpenWalletActivity::finish()
 {
-    destroyProgressDialog();
-
     if (!m_error_message.empty()) {
         QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message.translated));
     } else if (!m_warning_message.empty()) {
@@ -356,3 +332,21 @@ void OpenWalletActivity::open(const std::string& path)
         QTimer::singleShot(0, this, &OpenWalletActivity::finish);
     });
 }
+
+LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget)
+    : WalletControllerActivity(wallet_controller, parent_widget)
+{
+}
+
+void LoadWalletsActivity::load()
+{
+    showProgressDialog(tr("Loading wallets…"));
+
+    QTimer::singleShot(0, worker(), [this] {
+        for (auto& wallet : node().walletClient().getWallets()) {
+            m_wallet_controller->getOrCreateWallet(std::move(wallet));
+        }
+
+        QTimer::singleShot(0, this, [this] { Q_EMIT finished(); });
+    });
+}

src/qt/walletcontroller.h

@@ -52,9 +52,6 @@ class WalletController : public QObject
     WalletController(ClientModel& client_model, const PlatformStyle* platform_style, QObject* parent);
     ~WalletController();
 
-    //! Returns wallet models currently open.
-    std::vector<WalletModel*> getOpenWallets() const;
-
     WalletModel* getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet);
 
     //! Returns all wallet names in the wallet dir mapped to whether the wallet
@@ -90,7 +87,7 @@ class WalletControllerActivity : public QObject
 
 public:
     WalletControllerActivity(WalletController* wallet_controller, QWidget* parent_widget);
-    virtual ~WalletControllerActivity();
+    virtual ~WalletControllerActivity() = default;
 
 Q_SIGNALS:
     void finished();
@@ -100,11 +97,9 @@ class WalletControllerActivity : public QObject
     QObject* worker() const { return m_wallet_controller->m_activity_worker; }
 
     void showProgressDialog(const QString& label_text);
-    void destroyProgressDialog();
 
     WalletController* const m_wallet_controller;
     QWidget* const m_parent_widget;
-    QProgressDialog* m_progress_dialog{nullptr};
     WalletModel* m_wallet_model{nullptr};
     bilingual_str m_error_message;
     std::vector<bilingual_str> m_warning_message;
@@ -150,4 +145,14 @@ class OpenWalletActivity : public WalletControllerActivity
     void finish();
 };
 
+class LoadWalletsActivity : public WalletControllerActivity
+{
+    Q_OBJECT
+
+public:
+    LoadWalletsActivity(WalletController* wallet_controller, QWidget* parent_widget);
+
+    void load();
+};
+
 #endif // BITCOIN_QT_WALLETCONTROLLER_H