From e60b38b6cb05bcfb05f25c76b64271aa6be18ccf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 27 Feb 2021 22:18:43 +0100 Subject: [PATCH] Address feedback, remove probably-misleading comment (had already fixed it). --- Core/Config.cpp | 2 -- Core/Loaders.h | 9 +++++-- android/jni/app-android.cpp | 25 +++++++++++++------ .../src/org/ppsspp/ppsspp/PpssppActivity.java | 7 ++++-- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Core/Config.cpp b/Core/Config.cpp index 5a789f6d7e..bf95896234 100644 --- a/Core/Config.cpp +++ b/Core/Config.cpp @@ -1529,8 +1529,6 @@ void Config::RemoveRecent(const std::string &file) { } } -// TODO: This can be called during shutdown at a point where we have no NativeActivity object. -// That causes us to drop all content URI isos since they fail to open. void Config::CleanRecent() { std::vector cleanedRecent; for (size_t i = 0; i < recentIsos.size(); i++) { diff --git a/Core/Loaders.h b/Core/Loaders.h index a076cfa675..fecb941a6d 100644 --- a/Core/Loaders.h +++ b/Core/Loaders.h @@ -100,8 +100,7 @@ public: class ProxiedFileLoader : public FileLoader { public: - ProxiedFileLoader(FileLoader *backend) : backend_(backend) { - } + ProxiedFileLoader(FileLoader *backend) : backend_(backend) {} ~ProxiedFileLoader() override { // Takes ownership. delete backend_; @@ -131,6 +130,12 @@ public: std::string LatestError() const override { return backend_->LatestError(); } + size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override { + return backend_->ReadAt(absolutePos, bytes, count, data, flags); + } + size_t ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags = Flags::NONE) override { + return backend_->ReadAt(absolutePos, bytes, data, flags); + } protected: FileLoader *backend_; diff --git a/android/jni/app-android.cpp b/android/jni/app-android.cpp index 664f69dc72..c1f40f5e95 100644 --- a/android/jni/app-android.cpp +++ b/android/jni/app-android.cpp @@ -246,19 +246,29 @@ int Android_OpenContentUriFd(const std::string &filename) { return fd; } -void Android_CloseContentUriFd(int fd) { - if (!fd) { - return; +class ContentURIFileLoader : public ProxiedFileLoader { +public: + ContentURIFileLoader(const std::string &filename) + : ProxiedFileLoader(nullptr) { // we overwrite the nullptr below + int fd = Android_OpenContentUriFd(filename); + INFO_LOG(SYSTEM, "Fd %d for content URI: '%s'", fd, filename.c_str()); + backend_ = new LocalFileLoader(fd, filename); } -} + + bool ExistsFast() override { + if (!nativeActivity) { + // Assume it does if we don't have a NativeActivity right now. + return true; + } + return backend_->ExistsFast(); + } +}; class AndroidContentLoaderFactory : public FileLoaderFactory { public: AndroidContentLoaderFactory() {} FileLoader *ConstructFileLoader(const std::string &filename) override { - int fd = Android_OpenContentUriFd(filename); - INFO_LOG(SYSTEM, "Fd %d for content URI: '%s'", fd, filename.c_str()); - return new LocalFileLoader(fd, filename); + return new ContentURIFileLoader(filename); } }; @@ -688,7 +698,6 @@ extern "C" void Java_org_ppsspp_ppsspp_NativeApp_init NativeInit((int)args.size(), &args[0], user_data_path.c_str(), externalStorageDir.c_str(), cacheDir.c_str()); - std::unique_ptr factory(new AndroidContentLoaderFactory()); // Register a content URI file loader. diff --git a/android/src/org/ppsspp/ppsspp/PpssppActivity.java b/android/src/org/ppsspp/ppsspp/PpssppActivity.java index 24c3e77502..d03fcdab47 100644 --- a/android/src/org/ppsspp/ppsspp/PpssppActivity.java +++ b/android/src/org/ppsspp/ppsspp/PpssppActivity.java @@ -118,8 +118,11 @@ public class PpssppActivity extends NativeActivity { try { Uri uri = Uri.parse(uriString); ParcelFileDescriptor filePfd = getContentResolver().openFileDescriptor(uri, "r"); - int fd = filePfd.detachFd(); // Take ownership of the fd. - return fd; + if (filePfd == null) { + Log.e(TAG, "Failed to get file descriptor for " + uriString); + return -1; + } + return filePfd.detachFd(); // Take ownership of the fd. } catch (Exception e) { Log.e(TAG, "Exception opening content uri: " + e.toString()); return -1;