Merge pull request #22997 from hrydgard/content-uri-open-error

Android storage: Don't treat a failed content URI open as a handle

Common/File/FileUtil.cpp

@@ -640,8 +640,8 @@ bool CreateDir(const Path &path, bool quiet) {
 		break; // OK
 	case PathType::CONTENT_URI:
 	{
-		// NOTE: The Android storage API will simply create a renamed directory (append a number) if it already exists.
-		// We want to avoid that, so let's just return true if the directory already is there.
+		// Given a taken name, the storage API creates "name (1)", which the Java side then deletes again.
+		// Checking first saves that.
 		if (File::Exists(path)) {
 			return true;
 		}
@@ -654,7 +654,9 @@ bool CreateDir(const Path &path, bool quiet) {
 			if (!quiet) {
 				INFO_LOG(Log::IO, "Calling Android_CreateDirectory(%s, %s)", uri.ToString().c_str(), newDirName.c_str());
 			}
-			return Android_CreateDirectory(uri.ToString(), newDirName) == StorageError::SUCCESS;
+			// ALREADY_EXISTS if it showed up since the check, or the check failed.
+			const StorageError created = Android_CreateDirectory(uri.ToString(), newDirName);
+			return created == StorageError::SUCCESS || created == StorageError::ALREADY_EXISTS;
 		} else {
 			// Bad path - can't create this directory.
 			if (!quiet) {

Core/FileSystems/DirectoryFileSystem.cpp

@@ -282,7 +282,8 @@ bool DirectoryFileHandle::Open(const Path &basePath, std::string &fileName, File
 			inGameDir_ = true;
 		}
 		hFile = fd;
-		if (fd != -1) {
+		// Content URI opens fail with any negative StorageError, not just -1.
+		if (fd >= 0) {
 			// Success
 			return true;
 		} else {

Core/FileSystems/MetaFileSystem.cpp

@@ -626,9 +626,10 @@ int MetaFileSystem::ReadEntireFile(const std::string &filename, std::vector<u8>
 		return handle;
 
 	SeekFile(handle, 0, FILEMOVE_END);
-	const s64 fileSize = GetSeekPos(handle);
+	// A failed seek is (size_t)-1, which an s64 only sees as negative on 64-bit.
+	const size_t fileSize = GetSeekPos(handle);
 	SeekFile(handle, 0, FILEMOVE_BEGIN);
-	if (fileSize < 0 || (u64)fileSize > maxSize) {
+	if (fileSize == (size_t)-1 || fileSize > maxSize || fileSize > data.max_size()) {
 		CloseFile(handle);
 		return SCE_KERNEL_ERROR_ERROR;
 	}

android/src/org/ppsspp/ppsspp/ContentUri.java

@@ -175,12 +175,27 @@ public class ContentUri {
 		return context.getContentResolver().query(uri, projection, null, null, null);
 	}
 
+	// Like query. The resolver's own open is slower: it takes a stable reference to the provider for each
+	// call, and drops it after.
+	private static ParcelFileDescriptor openFile(Context context, Uri uri, String mode) throws Exception {
+		final ContentProviderClient c = getClient(context, uri);
+		if (c != null) {
+			try {
+				return c.openFile(uri, mode);
+			} catch (android.os.DeadObjectException e) {
+				Log.w(TAG, "Storage provider died, acquiring it again");
+				dropClient(uri.getAuthority(), c);
+			}
+		}
+		return context.getContentResolver().openFileDescriptor(uri, mode);
+	}
+
 	@Keep
 	@SuppressWarnings("unused")
 	public static int openContentUri(Activity activity, String uriString, String mode) {
 		try {
 			Uri uri = Uri.parse(uriString);
-			try (ParcelFileDescriptor filePfd = activity.getContentResolver().openFileDescriptor(uri, mode)) {
+			try (ParcelFileDescriptor filePfd = openFile(activity, uri, mode)) {
 				if (filePfd == null) {
 					// I'd expect an exception to happen before we get here, so this is probably
 					// never reached.