Store: Perform uninstalls on a background thread, to avoid hanging the UI

Core/Util/GameManager.cpp

@@ -95,7 +95,7 @@ bool GameManager::DownloadAndInstall(std::string storeFileUrl) {
 		ERROR_LOG(HLE, "Can only process one download at a time");
 		return false;
 	}
-	if (installInProgress_) {
+	if (InstallInProgress()) {
 		ERROR_LOG(HLE, "Can't download when an install is in progress (yet)");
 		return false;
 	}
@@ -127,13 +127,9 @@ float GameManager::DownloadSpeedKBps() {
 	return 0.0f;
 }
 
-bool GameManager::Uninstall(std::string name) {
-	if (name.empty()) {
-		ERROR_LOG(HLE, "Cannot remove an empty-named game");
-		return false;
-	}
+bool GameManager::UninstallGame(std::string name) {
 	Path gameDir = GetSysDirectory(DIRECTORY_GAME) / name;
-	INFO_LOG(HLE, "Deleting '%s'", gameDir.c_str());
+	INFO_LOG(HLE, "Uninstalling '%s'", gameDir.c_str());
 	if (!File::Exists(gameDir)) {
 		ERROR_LOG(HLE, "Game '%s' not installed, cannot uninstall", name.c_str());
 		return false;
@@ -141,11 +137,13 @@ bool GameManager::Uninstall(std::string name) {
 
 	bool success = File::DeleteDirRecursively(gameDir);
 	if (success) {
-		INFO_LOG(HLE, "Successfully deleted game '%s'", name.c_str());
-		g_Config.CleanRecent();
+		INFO_LOG(HLE, "Successfully uninstalled game '%s'", name.c_str());
+		InstallDone();
+		cleanRecentsAfter_ = true;
 		return true;
 	} else {
-		ERROR_LOG(HLE, "Failed to delete game '%s'", name.c_str());
+		ERROR_LOG(HLE, "Failed to uninstalled game '%s'", name.c_str());
+		InstallDone();
 		return false;
 	}
 }
@@ -170,10 +168,13 @@ void GameManager::Update() {
 		curDownload_.reset();
 	}
 
-	if (installDonePending_) {
-		if (installThread_.joinable())
+	if (installDonePending_.exchange(false)) {
+		if (installThread_.joinable()) {
 			installThread_.join();
-		installDonePending_ = false;
+		}
+		if (cleanRecentsAfter_.exchange(false)) {
+			g_Config.CleanRecent();
+		}
 	}
 }
 
@@ -278,7 +279,7 @@ ZipFileContents DetectZipFileContents(struct zip *z, ZipFileInfo *info) {
 bool GameManager::InstallGame(Path url, Path fileName, bool deleteAfter) {
 	SetCurrentThreadName("InstallGame");
 
-	if (installInProgress_ || installDonePending_) {
+	if (installDonePending_) {
 		ERROR_LOG(HLE, "Cannot have two installs in progress at the same time");
 		return false;
 	}
@@ -299,7 +300,6 @@ bool GameManager::InstallGame(Path url, Path fileName, bool deleteAfter) {
 	}
 
 	auto sy = GetI18NCategory(I18NCat::SYSTEM);
-	installInProgress_ = true;
 
 	Path pspGame = GetSysDirectory(DIRECTORY_GAME);
 	Path dest = pspGame;
@@ -711,13 +711,25 @@ bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &
 }
 
 bool GameManager::InstallGameOnThread(const Path &url, const Path &fileName, bool deleteAfter) {
-	if (installInProgress_ || installDonePending_) {
+	if (InstallInProgress() || installDonePending_) {
 		return false;
 	}
 	installThread_ = std::thread(std::bind(&GameManager::InstallGame, this, url, fileName, deleteAfter));
 	return true;
 }
 
+bool GameManager::UninstallGameOnThread(const std::string &name) {
+	if (name.empty()) {
+		ERROR_LOG(HLE, "Cannot uninstall an empty-named game");
+		return false;
+	}
+	if (InstallInProgress() || installDonePending_ || curDownload_.get() != nullptr) {
+		return false;
+	}
+	installThread_ = std::thread(std::bind(&GameManager::UninstallGame, this, name));
+	return true;
+}
+
 bool GameManager::InstallRawISO(const Path &file, const std::string &originalName, bool deleteAfter) {
 	Path destPath = Path(g_Config.currentDirectory) / originalName;
 	// TODO: To save disk space, we should probably attempt a move first.
@@ -733,12 +745,11 @@ bool GameManager::InstallRawISO(const Path &file, const std::string &originalNam
 }
 
 void GameManager::ResetInstallError() {
-	if (!installInProgress_) {
+	if (!InstallInProgress()) {
 		installError_.clear();
 	}
 }
 
 void GameManager::InstallDone() {
 	installDonePending_ = true;
-	installInProgress_ = false;
 }

Core/Util/GameManager.h

@@ -23,6 +23,7 @@
 #pragma once
 
 #include <thread>
+#include <atomic>
 
 #include "Common/Net/HTTPClient.h"
 #include "Common/File/Path.h"
@@ -46,7 +47,6 @@ public:
 	// This starts off a background process.
 	bool DownloadAndInstall(std::string storeZipUrl);
 	bool IsDownloading(std::string storeZipUrl);
-	bool Uninstall(std::string name);
 
 	// Cancels the download in progress, if any.
 	bool CancelDownload();
@@ -58,7 +58,7 @@ public:
 	void Update();
 
 	GameManagerState GetState() {
-		if (installInProgress_ || installDonePending_)
+		if (InstallInProgress() || installDonePending_)
 			return GameManagerState::INSTALLING;
 		if (curDownload_)
 			return GameManagerState::DOWNLOADING;
@@ -75,6 +75,7 @@ public:
 
 	// Only returns false if there's already an installation in progress.
 	bool InstallGameOnThread(const Path &url, const Path &tempFileName, bool deleteAfter);
+	bool UninstallGameOnThread(const std::string &name);
 
 private:
 	bool InstallGame(Path url, Path tempFileName, bool deleteAfter);
@@ -82,19 +83,25 @@ private:
 	bool InstallMemstickZip(struct zip *z, const Path &zipFile, const Path &dest, const ZipFileInfo &info, bool deleteAfter);
 	bool InstallZippedISO(struct zip *z, int isoFileIndex, const Path &zipfile, bool deleteAfter);
 	bool InstallRawISO(const Path &zipFile, const std::string &originalName, bool deleteAfter);
+	bool UninstallGame(std::string name);
+
 	void InstallDone();
+
 	bool ExtractFile(struct zip *z, int file_index, const Path &outFilename, size_t *bytesCopied, size_t allBytes);
 	bool DetectTexturePackDest(struct zip *z, int iniIndex, Path &dest);
 	void SetInstallError(const std::string &err);
 
+	bool InstallInProgress() const { return installThread_.joinable(); }
+
 	Path GetTempFilename() const;
 	std::string GetGameID(const Path &path) const;
 	std::string GetPBPGameID(FileLoader *loader) const;
 	std::string GetISOGameID(FileLoader *loader) const;
 	std::shared_ptr<http::Request> curDownload_;
 	std::thread installThread_;
-	bool installInProgress_ = false;
-	bool installDonePending_ = false;
+	std::atomic<bool> installDonePending_{};
+	std::atomic<bool> cleanRecentsAfter_{};
+
 	float installProgress_ = 0.0f;
 	std::string installError_;
 };

UI/Store.cpp

@@ -266,7 +266,6 @@ private:
 	void CreateViews();
 	UI::EventReturn OnInstall(UI::EventParams &e);
 	UI::EventReturn OnCancel(UI::EventParams &e);
-	UI::EventReturn OnUninstall(UI::EventParams &e);
 	UI::EventReturn OnLaunchClick(UI::EventParams &e);
 
 	bool IsGameInstalled() {
@@ -309,7 +308,10 @@ void ProductView::CreateViews() {
 		installButton_ = nullptr;
 		speedView_ = nullptr;
 		Add(new TextView(st->T("Already Installed")));
-		Add(new Button(st->T("Uninstall")))->OnClick.Handle(this, &ProductView::OnUninstall);
+		Add(new Button(st->T("Uninstall")))->OnClick.Add([=](UI::EventParams &e) {
+			g_GameManager.UninstallGameOnThread(entry_.file);
+			return UI::EVENT_DONE;
+		});
 		launchButton_ = new Button(st->T("Launch Game"));
 		launchButton_->OnClick.Handle(this, &ProductView::OnLaunchClick);
 		Add(launchButton_);
@@ -387,12 +389,6 @@ UI::EventReturn ProductView::OnCancel(UI::EventParams &e) {
 	return UI::EVENT_DONE;
 }
 
-UI::EventReturn ProductView::OnUninstall(UI::EventParams &e) {
-	g_GameManager.Uninstall(entry_.file);
-	CreateViews();
-	return UI::EVENT_DONE;
-}
-
 UI::EventReturn ProductView::OnLaunchClick(UI::EventParams &e) {
 	if (g_GameManager.GetState() != GameManagerState::IDLE) {
 		// Button should have been disabled. Just a safety check.

UI/Store.h

@@ -74,7 +74,6 @@ protected:
 private:
 	void ParseListing(std::string json);
 	ProductItemView *GetSelectedItem();
-	std::vector<StoreEntry> FilterEntries();
 
 	std::string GetTranslatedString(const json::JsonGet json, std::string key, const char *fallback = nullptr) const;
 
@@ -98,8 +97,8 @@ private:
 	std::string lang_;
 	std::string lastSelectedName_;
 
-	UI::ViewGroup *scrollItemView_;
-	UI::ViewGroup *productPanel_;
-	UI::TextView *titleText_;
+	UI::ViewGroup *scrollItemView_ = nullptr;
+	UI::ViewGroup *productPanel_ = nullptr;
+	UI::TextView *titleText_ = nullptr;
 };