Qt: Get rid of nested event loop in updater

src/duckstation-qt/autoupdaterwindow.cpp

@@ -81,6 +81,7 @@ AutoUpdaterWindow::AutoUpdaterWindow() : QWidget()
 {
   m_ui.setupUi(this);
   setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
+  setDownloadSectionVisibility(false);
 
   connect(m_ui.downloadAndInstall, &QPushButton::clicked, this, &AutoUpdaterWindow::downloadUpdateClicked);
   connect(m_ui.skipThisUpdate, &QPushButton::clicked, this, &AutoUpdaterWindow::skipThisUpdateClicked);
@@ -235,7 +236,7 @@ std::string AutoUpdaterWindow::getDefaultTag()
 #endif
 }
 
-std::string AutoUpdaterWindow::getCurrentUpdateTag() const
+std::string AutoUpdaterWindow::getCurrentUpdateTag()
 {
 #ifdef UPDATE_CHECKER_SUPPORTED
   return Host::GetBaseStringSettingValue("AutoUpdater", "UpdateTag", THIS_RELEASE_TAG);
@@ -244,6 +245,16 @@ std::string AutoUpdaterWindow::getCurrentUpdateTag() const
 #endif
 }
 
+void AutoUpdaterWindow::setDownloadSectionVisibility(bool visible)
+{
+  m_ui.downloadProgress->setVisible(visible);
+  m_ui.downloadStatus->setVisible(visible);
+  m_ui.downloadButtonBox->setVisible(visible);
+  m_ui.downloadAndInstall->setVisible(!visible);
+  m_ui.skipThisUpdate->setVisible(!visible);
+  m_ui.remindMeLater->setVisible(!visible);
+}
+
 void AutoUpdaterWindow::reportError(const std::string_view msg)
 {
   QtUtils::MessageBoxCritical(this, tr("Updater Error"), QtUtils::StringViewToQString(msg));
@@ -555,66 +566,59 @@ void AutoUpdaterWindow::downloadUpdateClicked()
 #endif
 #ifdef AUTO_UPDATER_SUPPORTED
   // Prevent multiple clicks of the button.
-  if (!m_ui.downloadAndInstall->isEnabled())
+  if (m_download_progress_callback)
     return;
-  m_ui.downloadAndInstall->setEnabled(false);
 
-  std::optional<bool> download_result;
-  QtModalProgressCallback progress(this);
-  progress.SetTitle(tr("Automatic Updater").toUtf8().constData());
-  progress.SetStatusText(tr("Downloading %1...").arg(m_latest_sha).toUtf8().constData());
-  progress.GetDialog().setWindowIcon(windowIcon());
-  progress.SetCancellable(true);
-  progress.MakeVisible();
+  setDownloadSectionVisibility(true);
 
+  m_download_progress_callback = new QtProgressCallback(this);
+  m_download_progress_callback->connectWidgets(m_ui.downloadStatus, m_ui.downloadProgress,
+                                               m_ui.downloadButtonBox->button(QDialogButtonBox::Cancel));
+  m_download_progress_callback->SetStatusText(TRANSLATE_SV("AutoUpdaterWindow", "Downloading Update..."));
+
+  ensureHttpReady();
   m_http->CreateRequest(
     m_download_url.toStdString(),
-    [this, &download_result](s32 status_code, const Error& error, const std::string&, std::vector<u8> response) {
+    [this](s32 status_code, const Error& error, const std::string&, std::vector<u8> response) {
+      m_download_progress_callback->SetStatusText(TRANSLATE_SV("AutoUpdaterWindow", "Processing Update..."));
+      m_download_progress_callback->SetProgressRange(1);
+      m_download_progress_callback->SetProgressValue(1);
+      DebugAssert(m_download_progress_callback);
+      delete m_download_progress_callback;
+      m_download_progress_callback = nullptr;
+
       if (status_code == HTTPDownloader::HTTP_STATUS_CANCELLED)
+      {
+        setDownloadSectionVisibility(false);
         return;
+      }
 
       if (status_code != HTTPDownloader::HTTP_STATUS_OK)
       {
         reportError(fmt::format("Download failed: {}", error.GetDescription()));
-        download_result = false;
+        setDownloadSectionVisibility(false);
         return;
       }
 
       if (response.empty())
       {
         reportError("Download failed: Update is empty");
-        download_result = false;
+        setDownloadSectionVisibility(false);
         return;
       }
 
-      download_result = processUpdate(response);
+      if (processUpdate(response))
+      {
+        // updater started, request exit
+        g_main_window->requestExit(false);
+      }
+      else
+      {
+        // allow user to try again
+        setDownloadSectionVisibility(false);
+      }
     },
-    &progress);
-
-  // Since we're going to block, don't allow the timer to poll, otherwise the progress callback can cause the timer
-  // to run, and recursively poll again.
-  m_http_poll_timer->stop();
-
-  // Block until completion.
-  QtUtils::ProcessEventsWithSleep(
-    QEventLoop::AllEvents,
-    [this]() {
-      m_http->PollRequests();
-      return m_http->HasAnyRequests();
-    },
-    HTTP_POLL_INTERVAL);
-
-  if (download_result.value_or(false))
-  {
-    // updater started. since we're a modal on the main window, we have to queue this.
-    QMetaObject::invokeMethod(g_main_window, &MainWindow::requestExit, Qt::QueuedConnection, false);
-    close();
-  }
-  else
-  {
-    // update failed, re-enable download button
-    m_ui.downloadAndInstall->setEnabled(true);
-  }
+    m_download_progress_callback);
 #endif
 }
 
@@ -956,8 +960,8 @@ bool AutoUpdaterWindow::processUpdate(const std::vector<u8>& update_data)
       return false;
     }
 
-    // We do this as a manual write here, rather than using WriteAtomicUpdatedFile(), because we want to write the file
-    // and set the permissions as one atomic operation.
+    // We do this as a manual write here, rather than using WriteAtomicUpdatedFile(), because we want to write the
+    // file and set the permissions as one atomic operation.
     FileSystem::ManagedCFilePtr fp = FileSystem::OpenManagedCFile(new_appimage_path.c_str(), "wb", &error);
     bool success = static_cast<bool>(fp);
     if (fp)

src/duckstation-qt/autoupdaterwindow.h

@@ -14,10 +14,10 @@
 #include <QtCore/QDateTime>
 #include <QtCore/QStringList>
 #include <QtCore/QTimer>
-#include <QtWidgets/QDialog>
 
 class Error;
 class HTTPDownloader;
+class QtProgressCallback;
 
 class EmuThread;
 
@@ -36,6 +36,7 @@ public:
   static bool canInstallUpdate();
   static QStringList getTagList();
   static std::string getDefaultTag();
+  static std::string getCurrentUpdateTag();
   static void cleanupAfterUpdate();
   static bool isOfficialBuild();
   static void warnAboutUnofficialBuild();
@@ -47,6 +48,7 @@ protected:
   void closeEvent(QCloseEvent* event) override;
 
 private:
+  void setDownloadSectionVisibility(bool visible);
   void httpPollTimerPoll();
 
   void downloadUpdateClicked();
@@ -58,7 +60,6 @@ private:
   bool ensureHttpReady();
 
   bool updateNeeded() const;
-  std::string getCurrentUpdateTag() const;
 
   void getLatestTagComplete(s32 status_code, const Error& error, std::vector<u8> response, bool display_errors);
   void getLatestReleaseComplete(s32 status_code, const Error& error, std::vector<u8> response);
@@ -80,6 +81,7 @@ private:
 
   std::unique_ptr<HTTPDownloader> m_http;
   QTimer* m_http_poll_timer = nullptr;
+  QtProgressCallback* m_download_progress_callback = nullptr;
   QString m_latest_sha;
   QString m_download_url;
   int m_download_size = 0;

src/duckstation-qt/autoupdaterwindow.ui

@@ -85,8 +85,8 @@
        </property>
        <property name="sizeHint" stdset="0">
         <size>
-         <width>40</width>
-         <height>20</height>
+         <width>0</width>
+         <height>0</height>
         </size>
        </property>
       </spacer>
@@ -120,6 +120,27 @@
      </item>
     </layout>
    </item>
+   <item>
+    <layout class="QGridLayout" name="downloadLayout" columnstretch="1,0">
+     <item row="1" column="0">
+      <widget class="QProgressBar" name="downloadProgress"/>
+     </item>
+     <item row="1" column="1">
+      <widget class="QDialogButtonBox" name="downloadButtonBox">
+       <property name="standardButtons">
+        <set>QDialogButtonBox::StandardButton::Cancel</set>
+       </property>
+      </widget>
+     </item>
+     <item row="0" column="0" colspan="2">
+      <widget class="QLabel" name="downloadStatus">
+       <property name="text">
+        <string/>
+       </property>
+      </widget>
+     </item>
+    </layout>
+   </item>
   </layout>
  </widget>
  <resources>

src/duckstation-qt/qtprogresscallback.cpp

@@ -107,6 +107,67 @@ void QtModalProgressCallback::MakeVisible()
     m_dialog.show();
 }
 
+QtProgressCallback::QtProgressCallback(QObject* parent /* = nullptr */) : QObject(parent)
+{
+}
+
+QtProgressCallback::~QtProgressCallback() = default;
+
+bool QtProgressCallback::IsCancelled() const
+{
+  return m_ts_cancelled.load(std::memory_order_acquire);
+}
+
+void QtProgressCallback::SetTitle(const std::string_view title)
+{
+  emit titleUpdated(QtUtils::StringViewToQString(title));
+}
+
+void QtProgressCallback::SetStatusText(const std::string_view text)
+{
+  ProgressCallback::SetStatusText(text);
+  emit statusTextUpdated(QtUtils::StringViewToQString(text));
+}
+
+void QtProgressCallback::SetProgressRange(u32 range)
+{
+  const u32 prev_range = m_progress_range;
+  ProgressCallback::SetProgressRange(range);
+  if (m_progress_range == prev_range)
+    return;
+
+  emit progressRangeUpdated(0, static_cast<int>(m_progress_range));
+}
+
+void QtProgressCallback::SetProgressValue(u32 value)
+{
+  const u32 prev_value = m_progress_value;
+  ProgressCallback::SetProgressValue(value);
+  if (m_progress_value == prev_value)
+    return;
+
+  emit progressValueUpdated(static_cast<int>(m_progress_value));
+}
+
+void QtProgressCallback::connectWidgets(QLabel* const status_label, QProgressBar* const progress_bar,
+                                        QAbstractButton* const cancel_button)
+{
+  if (status_label)
+    connect(this, &QtProgressCallback::statusTextUpdated, status_label, &QLabel::setText);
+  if (progress_bar)
+  {
+    connect(this, &QtProgressCallback::progressRangeUpdated, progress_bar, &QProgressBar::setRange);
+    connect(this, &QtProgressCallback::progressValueUpdated, progress_bar, &QProgressBar::setValue);
+  }
+  if (cancel_button)
+  {
+    // force direct connection so it executes on the calling thread
+    connect(
+      cancel_button, &QAbstractButton::clicked, this,
+      [this]() { m_ts_cancelled.store(true, std::memory_order_release); }, Qt::DirectConnection);
+  }
+}
+
 QtAsyncTaskWithProgress::QtAsyncTaskWithProgress(const QString& initial_title, const QString& initial_status_text,
                                                  bool cancellable, int range, int value, float show_delay,
                                                  QWidget* dialog_parent, WorkCallback callback)

src/duckstation-qt/qtprogresscallback.h

@@ -14,6 +14,7 @@
 #include <QtWidgets/QProgressDialog>
 #include <atomic>
 
+class QAbstractButton;
 class QLabel;
 class QProgressBar;
 class QPushButton;
@@ -50,6 +51,33 @@ private:
   float m_show_delay;
 };
 
+class QtProgressCallback final : public QObject, public ProgressCallback
+{
+  Q_OBJECT
+
+public:
+  explicit QtProgressCallback(QObject* parent = nullptr);
+  ~QtProgressCallback() override;
+
+  bool IsCancelled() const override;
+  void SetTitle(const std::string_view title) override;
+  void SetStatusText(const std::string_view text) override;
+  void SetProgressRange(u32 range) override;
+  void SetProgressValue(u32 value) override;
+
+  void connectWidgets(QLabel* const status_label, QProgressBar* const progress_bar,
+                      QAbstractButton* const cancel_button);
+
+Q_SIGNALS:
+  void titleUpdated(const QString& title);
+  void statusTextUpdated(const QString& status);
+  void progressRangeUpdated(int min, int max);
+  void progressValueUpdated(int value);
+
+private:
+  std::atomic_bool m_ts_cancelled{false};
+};
+
 class QtAsyncTaskWithProgress final : public QObject, private ProgressCallback
 {
   Q_OBJECT