Android: Only count a "name (N)" result as a name conflict

The provider answers a taken name with a renamed copy, "name (1)" or
"name (1).ext", but it also replaces characters FAT doesn't allow with '_',
which isn't a conflict. Create, copy and rename now only report
ALREADY_EXISTS (and undo) for the numbered form of the sanitised name.

Also take the image picker's result from ClipData when there's no data URI,
like the document picker does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

@@ -73,6 +73,53 @@ public class ContentUri {
 		}
 	}
 
+	// Whether the provider made a renamed copy because the name was taken. It answers that with "name (1)", or
+	// "name (1).ext" for a file (FileUtils.buildUniqueFile), and before that replaces the characters FAT doesn't
+	// allow with '_' (buildValidFatFilename). The replacement alone isn't a conflict, the name was free.
+	private static boolean isRenamedCopy(String requested, String actual) {
+		if (actual == null || actual.equals(requested)) {
+			return false;
+		}
+		final String sanitized = fatSanitized(requested);
+		// Directories, and files the provider didn't split an extension off, get the number at the end.
+		if (isNumbered(actual, sanitized, "")) {
+			return true;
+		}
+		final int dot = sanitized.lastIndexOf('.');
+		return dot > 0 && isNumbered(actual, sanitized.substring(0, dot), sanitized.substring(dot));
+	}
+
+	// Whether name is "base (N)ext".
+	private static boolean isNumbered(String name, String base, String ext) {
+		final int digitsStart = base.length() + 2;
+		final int digitsEnd = name.length() - ext.length() - 1;
+		if (digitsEnd <= digitsStart || !name.startsWith(base + " (") || !name.endsWith(")" + ext)) {
+			return false;
+		}
+		for (int i = digitsStart; i < digitsEnd; i++) {
+			final char ch = name.charAt(i);
+			if (ch < '0' || ch > '9') {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	private static String fatSanitized(String name) {
+		final StringBuilder sb = new StringBuilder(name.length());
+		for (int i = 0; i < name.length(); i++) {
+			final char ch = name.charAt(i);
+			sb.append((ch <= 0x1f || ch == 0x7f || "\"*/:<>?\\|".indexOf(ch) >= 0) ? '_' : ch);
+		}
+		return sb.toString();
+	}
+
+	private static void logIfNameChanged(String what, String requested, String actual) {
+		if (actual != null && !actual.equals(requested)) {
+			Log.w(TAG, what + ": the provider named '" + requested + "' '" + actual + "'");
+		}
+	}
+
 	// Without a client of our own, every query and DocumentsContract call acquires the storage provider and
 	// releases it again, and when our reference count drops to zero, each of those is another binder call,
 	// to the system. Holding one keeps the count up. Unstable, so the provider dying doesn't take our process
@@ -315,11 +362,12 @@ public class ContentUri {
 			}
 			// Null if the provider wouldn't say, which isn't a conflict.
 			final String createdName = documentName(activity, created);
-			if (createdName != null && !name.equals(createdName)) {
+			if (isRenamedCopy(name, createdName)) {
 				Log.w(TAG, "createDocument: '" + name + "' is taken, removing the renamed copy " + created);
 				DocumentsContract.deleteDocument(resolver, created);
 				return STORAGE_ERROR_ALREADY_EXISTS;
 			}
+			logIfNameChanged("createDocument", name, createdName);
 			return STORAGE_ERROR_SUCCESS;
 		} catch (Exception e) {
 			Log.e(TAG, "createDocument exception: " + e);
@@ -378,7 +426,7 @@ public class ContentUri {
 			// A taken name gets a renamed copy, see createDocument.
 			final String name = documentName(activity, srcUri);
 			final String copyName = documentName(activity, copy);
-			if (name != null && copyName != null && !name.equals(copyName)) {
+			if (name != null && isRenamedCopy(name, copyName)) {
 				Log.w(TAG, "contentUriCopyFile: '" + name + "' is taken, removing the renamed copy " + copy);
 				DocumentsContract.deleteDocument(resolver, copy);
 				return STORAGE_ERROR_ALREADY_EXISTS;
@@ -427,11 +475,12 @@ public class ContentUri {
 			final Uri newUri = DocumentsContract.renameDocument(resolver, uri, newName);
 			// A taken name gets "name (1)" instead, see createDocument. Put the old name back.
 			final String renamedName = newUri != null ? documentName(activity, newUri) : null;
-			if (oldName != null && renamedName != null && !newName.equals(renamedName)) {
+			if (oldName != null && isRenamedCopy(newName, renamedName)) {
 				Log.w(TAG, "contentUriRenameFileTo: '" + newName + "' is taken, renaming back to '" + oldName + "'");
 				DocumentsContract.renameDocument(resolver, newUri, oldName);
 				return STORAGE_ERROR_ALREADY_EXISTS;
 			}
+			logIfNameChanged("contentUriRenameFileTo", newName, renamedName);
 			return STORAGE_ERROR_SUCCESS;
 		} catch (Exception e) {
 			Log.e(TAG, "contentUriRenameFile exception: " + e);

android/src/org/ppsspp/ppsspp/ImageResultProxyActivity.java

@@ -24,7 +24,6 @@ public class ImageResultProxyActivity extends AppCompatActivity {
 	private String copyAndDownscaleToCache(Uri selectedImage) {
 		Log.i(TAG, "Selected image: " + selectedImage);
 		if (selectedImage == null) {
-			// Some pickers only return a ClipData.
 			Log.e(TAG, "No image URI in the result");
 			return null;
 		}
@@ -147,7 +146,13 @@ public class ImageResultProxyActivity extends AppCompatActivity {
 
 				String localPath;
 				if (result.getResultCode() == Activity.RESULT_OK && result.getData() != null) {
-					localPath = copyAndDownscaleToCache(result.getData().getData());
+					Intent data = result.getData();
+					Uri uri = data.getData();
+					// Some pickers only return a ClipData.
+					if (uri == null && data.getClipData() != null && data.getClipData().getItemCount() > 0) {
+						uri = data.getClipData().getItemAt(0).getUri();
+					}
+					localPath = copyAndDownscaleToCache(uri);
 				} else {
 					localPath = null;
 				}

docs/android-todo.md

@@ -9,9 +9,7 @@ thread. It opens the picked image's stream three times (bounds, orientation, dec
 then writes the downscaled copy to the cache. With a cloud-backed provider (Google Photos fetching the
 original) or a very large image, that can take long enough to ANR.
 
-Fix: do the copy on a background thread, then call `returnWithResult` back on the UI thread. While
-there: it doesn't fall back to `getClipData()` when `getData()` is null, though the comment near the top
-says some pickers do that (`DocumentResultProxyActivity` handles it).
+Fix: do the copy on a background thread, then call `returnWithResult` back on the UI thread.
 
 ## VR: `System.exit(0)` in `onDestroy`
 
@@ -42,10 +40,6 @@ threads read.
 
 ## Smaller ones
 
-- **Sanitised names read as conflicts:** `ContentUri` treats a name read back that differs from the
-  one asked for as "name taken" (the provider made `name (1)` instead) in `createDocument`, copy and
-  rename. `ExternalStorageProvider` also replaces FAT-invalid characters, which gives a false
-  `ALREADY_EXISTS`. Only report a conflict when the result looks like `name (N)`.
 - **Denied permissions get no answer:** a denied location or microphone permission sends nothing to
   native, so the game's request just stays silent. Storage and local network do report denial.
 - **Picker result after an activity recreate:** if the activity was destroyed and recreated while a