Merge pull request #18490 from hrydgard/uninstall-on-thread

Homebrew Store: Do uninstalls on a thread

Common/File/AndroidStorage.cpp

@@ -245,8 +245,9 @@ std::vector<File::FileInfo> Android_ListContentUri(const std::string &path, bool
 	env->DeleteLocalRef(fileList);
 
 	double elapsed = time_now_d() - start;
-	if (elapsed > 0.1) {
-		INFO_LOG(FILESYS, "Listing directory on content URI took %0.3f s (%d files)", elapsed, (int)items.size());
+	double threshold = 0.1;
+	if (elapsed >= threshold) {
+		INFO_LOG(FILESYS, "Listing directory on content URI '%s' took %0.3f s (%d files, log threshold = %0.3f)", path.c_str(), elapsed, (int)items.size(), threshold);
 	}
 	return items;
 }

Core/Util/GameDB.cpp

@@ -128,8 +128,12 @@ bool GameDB::GetGameInfos(std::string_view id, std::vector<GameDBInfo> *infos) {
 			// Ignore version and stuff for now
 			if (IDMatches(id, serial)) {
 				GameDBInfo info;
-				sscanf(line.crc.data(), "%08x", &info.crc);
-				sscanf(line.size.data(), "%llu", &info.size);
+				if (1 != sscanf(line.crc.data(), "%08x", &info.crc)) {
+					continue;
+				}
+				if (1 != sscanf(line.size.data(), "%llu", (long long *)&info.size)) {
+					continue;
+				}
 				info.title = line.title;
 				info.foreignTitle = line.foreignTitle;
 				infos->push_back(info);

Core/Util/GameManager.cpp

@@ -36,6 +36,7 @@
 #include "Common/Data/Encoding/Utf8.h"
 #include "Common/Data/Format/IniFile.h"
 #include "Common/Log.h"
+#include "Common/System/OSD.h"
 #include "Common/File/FileUtil.h"
 #include "Common/StringUtils.h"
 #include "Common/Thread/ThreadUtil.h"
@@ -95,7 +96,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,26 +128,32 @@ 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;
-	}
+void GameManager::UninstallGame(std::string name) {
+	SetCurrentThreadName("UninstallGame");
+
+	AndroidJNIThreadContext context;  // Destructor detaches.
+
 	Path gameDir = GetSysDirectory(DIRECTORY_GAME) / name;
-	INFO_LOG(HLE, "Deleting '%s'", gameDir.c_str());
+
+	auto st = GetI18NCategory(I18NCat::STORE);
+
+	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;
+		return;
 	}
-
+	g_OSD.SetProgressBar("install", st->T("Uninstall"), 0.0f, 0.0f, 0.0f, 0.1f);
 	bool success = File::DeleteDirRecursively(gameDir);
+	g_OSD.RemoveProgressBar("install", success, 0.5f);
 	if (success) {
-		INFO_LOG(HLE, "Successfully deleted game '%s'", name.c_str());
-		g_Config.CleanRecent();
-		return true;
+		INFO_LOG(HLE, "Successfully uninstalled game '%s'", name.c_str());
+		InstallDone();
+		cleanRecentsAfter_ = true;
+		return;
 	} else {
-		ERROR_LOG(HLE, "Failed to delete game '%s'", name.c_str());
-		return false;
+		ERROR_LOG(HLE, "Failed to uninstalled game '%s'", name.c_str());
+		InstallDone();
+		return;
 	}
 }
 
@@ -170,13 +177,13 @@ void GameManager::Update() {
 		curDownload_.reset();
 	}
 
-	if (installDonePending_) {
-		if (installThread_.get() != nullptr) {
-			if (installThread_->joinable())
-				installThread_->join();
-			installThread_.reset();
+	if (installDonePending_.exchange(false)) {
+		if (installThread_.joinable()) {
+			installThread_.join();
+		}
+		if (cleanRecentsAfter_.exchange(false)) {
+			g_Config.CleanRecent();
 		}
-		installDonePending_ = false;
 	}
 }
 
@@ -281,65 +288,75 @@ 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;
 	}
 
 	AndroidJNIThreadContext context;  // Destructor detaches.
-
 	if (!File::Exists(fileName)) {
 		ERROR_LOG(HLE, "Game file '%s' doesn't exist", fileName.c_str());
 		return false;
 	}
 
+	auto st = GetI18NCategory(I18NCat::STORE);
+	auto di = GetI18NCategory(I18NCat::DIALOG);
+	auto sy = GetI18NCategory(I18NCat::SYSTEM);
+
+	g_OSD.SetProgressBar("install", di->T("Installing..."), 0.0f, 0.0f, 0.0f, 0.1f);
+
 	std::string extension = url.GetFileExtension();
 	// Examine the URL to guess out what we're installing.
 	if (extension == ".cso" || extension == ".iso") {
 		// It's a raw ISO or CSO file. We just copy it to the destination.
 		std::string shortFilename = url.GetFilename();
-		return InstallRawISO(fileName, shortFilename, deleteAfter);
+		bool success = InstallRawISO(fileName, shortFilename, deleteAfter);
+		g_OSD.RemoveProgressBar("install", success, 0.5f);
+		return success;
 	}
 
-	auto sy = GetI18NCategory(I18NCat::SYSTEM);
-	installInProgress_ = true;
-
 	Path pspGame = GetSysDirectory(DIRECTORY_GAME);
 	Path dest = pspGame;
 	int error = 0;
 
 	struct zip *z = ZipOpenPath(fileName);
 	if (!z) {
+		g_OSD.RemoveProgressBar("install", false, 0.5f);
 		SetInstallError(sy->T("Unable to open zip file"));
 		return false;
 	}
 
 	ZipFileInfo info;
 	ZipFileContents contents = DetectZipFileContents(z, &info);
+	bool success = false;
 	switch (contents) {
 	case ZipFileContents::PSP_GAME_DIR:
 		INFO_LOG(HLE, "Installing '%s' into '%s'", fileName.c_str(), pspGame.c_str());
-		// InstallMemstickGame contains code to close z.
-		return InstallMemstickGame(z, fileName, pspGame, info, false, deleteAfter);
+		// InstallMemstickGame contains code to close (and delete) z.
+		success = InstallMemstickGame(z, fileName, pspGame, info, false, deleteAfter);
+		break;
 	case ZipFileContents::ISO_FILE:
 		INFO_LOG(HLE, "Installing '%s' into its containing directory", fileName.c_str());
 		// InstallZippedISO contains code to close z.
-		return InstallZippedISO(z, info.isoFileIndex, fileName, deleteAfter);
+		success = InstallZippedISO(z, info.isoFileIndex, fileName, deleteAfter);
+		break;
 	case ZipFileContents::TEXTURE_PACK:
 		// InstallMemstickGame contains code to close z, and works for textures too.
 		if (DetectTexturePackDest(z, info.textureIniIndex, dest)) {
-			INFO_LOG(HLE, "Installing '%s' into '%s'", fileName.c_str(), dest.c_str());
+			INFO_LOG(HLE, "Installing texture pack '%s' into '%s'", fileName.c_str(), dest.c_str());
 			File::CreateFullPath(dest);
-			// Install as a zip file if textures.ini is in the root.  Performs better on Android.
-			if (info.stripChars == 0)
-				return InstallMemstickZip(z, fileName, dest / "textures.zip", info, deleteAfter);
-			File::CreateEmptyFile(dest / ".nomedia");
-			return InstallMemstickGame(z, fileName, dest, info, true, deleteAfter);
+			// Install as a zip file if textures.ini is in the root. Performs better on Android.
+			if (info.stripChars == 0) {
+				success = InstallMemstickZip(z, fileName, dest / "textures.zip", info, deleteAfter);
+			} else {
+				File::CreateEmptyFile(dest / ".nomedia");
+				success = InstallMemstickGame(z, fileName, dest, info, true, deleteAfter);
+			}
 		} else {
 			zip_close(z);
 			z = nullptr;
 		}
-		return false;
+		break;
 	default:
 		ERROR_LOG(HLE, "File not a PSP game, no EBOOT.PBP found.");
 		SetInstallError(sy->T("Not a PSP game"));
@@ -347,8 +364,10 @@ bool GameManager::InstallGame(Path url, Path fileName, bool deleteAfter) {
 		z = nullptr;
 		if (deleteAfter)
 			File::Delete(fileName);
-		return false;
+		break;
 	}
+	g_OSD.RemoveProgressBar("install", success, 0.5f);
+	return success;
 }
 
 bool GameManager::DetectTexturePackDest(struct zip *z, int iniIndex, Path &dest) {
@@ -601,11 +620,11 @@ bool GameManager::InstallMemstickGame(struct zip *z, const Path &zipfile, const
 		}
 	}
 	INFO_LOG(HLE, "Extracted %d files from zip (%d bytes / %d).", info.numFiles, (int)bytesCopied, (int)allBytes);
-
 	zip_close(z);
 	z = nullptr;
 	installProgress_ = 1.0f;
 	if (deleteAfter) {
+		INFO_LOG(HLE, "Deleting '%s' after extraction", zipfile.c_str());
 		File::Delete(zipfile);
 	}
 	InstallDone();
@@ -682,7 +701,6 @@ bool GameManager::InstallMemstickZip(struct zip *z, const Path &zipfile, const P
 
 bool GameManager::InstallZippedISO(struct zip *z, int isoFileIndex, const Path &zipfile, bool deleteAfter) {
 	// Let's place the output file in the currently selected Games directory.
-
 	std::string fn = zip_get_name(z, isoFileIndex, 0);
 	size_t nameOffset = fn.rfind('/');
 	if (nameOffset == std::string::npos) {
@@ -714,10 +732,22 @@ 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_.reset(new std::thread(std::bind(&GameManager::InstallGame, this, url, fileName, deleteAfter)));
+	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;
 }
 
@@ -736,12 +766,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,26 +75,34 @@ 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:
+	// TODO: The return value on this is a bit pointless, we can't get at it.
 	bool InstallGame(Path url, Path tempFileName, bool deleteAfter);
 	bool InstallMemstickGame(struct zip *z, const Path &zipFile, const Path &dest, const ZipFileInfo &info, bool allowRoot, bool deleteAfter);
 	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);
+	void 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::shared_ptr<std::thread> installThread_;
-	bool installInProgress_ = false;
-	bool installDonePending_ = false;
+	std::thread installThread_;
+	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() {
@@ -275,6 +274,7 @@ private:
 	std::string DownloadURL();
 
 	StoreEntry entry_;
+	UI::Button *uninstallButton_ = nullptr;
 	UI::Button *installButton_ = nullptr;
 	UI::Button *launchButton_ = nullptr;
 	UI::Button *cancelButton_ = nullptr;
@@ -301,6 +301,7 @@ void ProductView::CreateViews() {
 		LinearLayout *progressDisplay = new LinearLayout(ORIENT_HORIZONTAL);
 		installButton_ = progressDisplay->Add(new Button(st->T("Install")));
 		installButton_->OnClick.Handle(this, &ProductView::OnInstall);
+		uninstallButton_ = nullptr;
 
 		speedView_ = progressDisplay->Add(new TextView(""));
 		speedView_->SetVisibility(isDownloading ? V_VISIBLE : V_GONE);
@@ -309,7 +310,11 @@ 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);
+		uninstallButton_ = new Button(st->T("Uninstall"));
+		Add(uninstallButton_)->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_);
@@ -339,6 +344,9 @@ void ProductView::Update() {
 	if (installButton_) {
 		installButton_->SetEnabled(g_GameManager.GetState() == GameManagerState::IDLE);
 	}
+	if (uninstallButton_) {
+		uninstallButton_->SetEnabled(g_GameManager.GetState() == GameManagerState::IDLE);
+	}
 	if (g_GameManager.GetState() == GameManagerState::DOWNLOADING) {
 		if (speedView_) {
 			float speed = g_GameManager.DownloadSpeedKBps();
@@ -387,12 +395,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;
 };