[Scummvm-git-logs] scummvm master -> 4bda9bf1121537277b333de40fca9a71f99aac31

antoniou79 a.antoniou79 at gmail.com
Sun Nov 1 17:38:20 UTC 2020


This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
8fe6aae59e ANDROID: Use SAF for folder and file creation when normal way fails
7aa38e197a ANDROID: Callback for onDestroy at ScummVM thread end
4bda9bf112 ANDROID: Use TextUtils.isEmpty() instead of String's isEmpty()


Commit: 8fe6aae59e8a654f9251982804f3fb2e4fe5a1cd
    https://github.com/scummvm/scummvm/commit/8fe6aae59e8a654f9251982804f3fb2e4fe5a1cd
Author: antoniou (a.antoniou79 at gmail.com)
Date: 2020-11-01T19:38:07+02:00

Commit Message:
ANDROID: Use SAF for folder and file creation when normal way fails

Should affect only external "secondary" storage (eg. physical SD card)

Changed paths:
    backends/fs/posix/posix-fs-factory.cpp
    backends/fs/posix/posix-fs.cpp
    backends/fs/posix/posix-iostream.cpp
    backends/fs/posix/posix-iostream.h
    backends/platform/android/jni-android.cpp
    backends/platform/android/jni-android.h
    backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
    backends/platform/android/org/scummvm/scummvm/ScummVM.java
    backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
    dists/android/build.gradle
    dists/android/res/values/strings.xml


diff --git a/backends/fs/posix/posix-fs-factory.cpp b/backends/fs/posix/posix-fs-factory.cpp
index e0eebbdbaf..cd6c4bb7dd 100644
--- a/backends/fs/posix/posix-fs-factory.cpp
+++ b/backends/fs/posix/posix-fs-factory.cpp
@@ -42,6 +42,8 @@ AbstractFSNode *POSIXFilesystemFactory::makeRootFileNode() const {
 
 AbstractFSNode *POSIXFilesystemFactory::makeCurrentDirectoryFileNode() const {
 #if defined(__ANDROID__)
+	// Keep this here if we still want to maintain support for the Android SDL port, since this affects that too
+	//
 	// For Android it does not make sense to have "." in Search Manager as a current directory file node, so we skip it here
 	// Otherwise this can potentially lead to a crash since, in Android getcwd() returns the root path "/"
 	// and when SearchMan is used (eg. SearchSet::createReadStreamForMember) and it tries to search root path (and calls POSIXFilesystemNode::getChildren())
diff --git a/backends/fs/posix/posix-fs.cpp b/backends/fs/posix/posix-fs.cpp
index a6cc818eae..7fcc7cf990 100644
--- a/backends/fs/posix/posix-fs.cpp
+++ b/backends/fs/posix/posix-fs.cpp
@@ -55,7 +55,7 @@
 #include <os2.h>
 #endif
 
-#if defined(__ANDROID__) && !defined(ANDROIDSDL)
+#if defined(ANDROID_PLAIN_PORT)
 #include "backends/platform/android/jni-android.h"
 #endif
 
@@ -172,7 +172,7 @@ bool POSIXFilesystemNode::getChildren(AbstractFSList &myList, ListMode mode, boo
 	}
 #endif
 
-#if defined(__ANDROID__) && !defined(ANDROIDSDL)
+#if defined(ANDROID_PLAIN_PORT)
 	if (_path == "/") {
 		Common::Array<Common::String> list = JNI::getAllStorageLocations();
 		for (Common::Array<Common::String>::const_iterator it = list.begin(), end = list.end(); it != end; ++it) {
@@ -297,6 +297,17 @@ Common::WriteStream *POSIXFilesystemNode::createWriteStream() {
 bool POSIXFilesystemNode::createDirectory() {
 	if (mkdir(_path.c_str(), 0755) == 0)
 		setFlags();
+#if defined(ANDROID_PLAIN_PORT)
+	else {
+		// TODO eventually android specific stuff should be moved to an Android backend for fs
+		//      peterkohaut already has some work on that in his fork (moving the port to more native code)
+		//      However, I have not found a way to do this Storage Access Framework stuff natively yet.
+		if (JNI::createDirectoryWithSAF(_path)) {
+			setFlags();
+		}
+	}
+#endif // ANDROID_PLAIN_PORT
+
 
 	return _isValid && _isDirectory;
 }
diff --git a/backends/fs/posix/posix-iostream.cpp b/backends/fs/posix/posix-iostream.cpp
index 59f14142b5..befd1ef6d3 100644
--- a/backends/fs/posix/posix-iostream.cpp
+++ b/backends/fs/posix/posix-iostream.cpp
@@ -26,17 +26,84 @@
 
 #include <sys/stat.h>
 
+#if defined(ANDROID_PLAIN_PORT)
+#include "backends/platform/android/jni-android.h"
+#include <unistd.h>
+#endif
+
+
 PosixIoStream *PosixIoStream::makeFromPath(const Common::String &path, bool writeMode) {
 	FILE *handle = fopen(path.c_str(), writeMode ? "wb" : "rb");
 
 	if (handle)
 		return new PosixIoStream(handle);
 
+#if defined(ANDROID_PLAIN_PORT)
+	else {
+		// TODO also address case for writeMode false
+
+		// TODO eventually android specific stuff should be moved to an Android backend for fs
+		//      peterkohaut already has some work on that in his fork (moving the port to more native code)
+		//      However, I have not found a way to do this Storage Access Framework stuff natively yet.
+
+		// if we are here we are only interested in hackyFilenames -- which mean we went through SAF. Otherwise we ignore the case
+		if (writeMode) {
+			Common::String hackyFilename = JNI::createFileWithSAF(path);
+			// https://stackoverflow.com/questions/59000390/android-accessing-files-in-native-c-c-code-with-google-scoped-storage-api
+			//warning ("PosixIoStream::makeFromPath() JNI::createFileWithSAF returned: %s", hackyFilename.c_str() );
+			if (strstr(hackyFilename.c_str(), "/proc/self/fd/") == hackyFilename.c_str()) {
+				//warning ("PosixIoStream::makeFromPath() match with hacky prefix!" );
+				int fd = atoi(hackyFilename.c_str() + 14);
+				if (fd != 0) {
+					//warning ("PosixIoStream::makeFromPath() got fd int: %d!", fd );
+					// Why dup(fd) below: if we called fdopen() on the
+					// original fd value, and the native code closes
+					// and tries to re-open that file, the second fdopen(fd)
+					// would fail, return NULL - after closing the
+					// original fd received from Android, it's no longer valid.
+					FILE *safHandle = fdopen(dup(fd), "wb");
+					// Why rewind(fp): if the native code closes and
+					// opens again the file, the file read/write position
+					// would not change, because with dup(fd) it's still
+					// the same file...
+					rewind(safHandle);
+					if (safHandle) {
+						return new PosixIoStream(safHandle, true, hackyFilename);
+					}
+				}
+			}
+	   }
+	}
+#endif // ANDROID_PLAIN_PORT
+
 	return nullptr;
 }
 
+
+#if defined(ANDROID_PLAIN_PORT)
+PosixIoStream::PosixIoStream(void *handle, bool bCreatedWithSAF, Common::String sHackyFilename) :
+		StdioStream(handle) {
+	createdWithSAF = bCreatedWithSAF;
+	hackyfilename = sHackyFilename;
+}
+
+PosixIoStream::~PosixIoStream() {
+	//warning("PosixIoStream::~PosixIoStream() closing file");
+	if (createdWithSAF && !hackyfilename.empty() ) {
+		JNI::closeFileWithSAF(hackyfilename);
+	}
+	// we'leave the base class destructor to close the FILE
+	// it does not seem to matter that the operation is done
+	// after the JNI call to close the descriptor on the Java side
+}
+#endif // ANDROID_PLAIN_PORT
+
 PosixIoStream::PosixIoStream(void *handle) :
 		StdioStream(handle) {
+#if defined(ANDROID_PLAIN_PORT)
+	createdWithSAF = false;
+	hackyfilename = "";
+#endif // ANDROID_PLAIN_PORT
 }
 
 int32 PosixIoStream::size() const {
diff --git a/backends/fs/posix/posix-iostream.h b/backends/fs/posix/posix-iostream.h
index 638d7b17cd..c258186cfe 100644
--- a/backends/fs/posix/posix-iostream.h
+++ b/backends/fs/posix/posix-iostream.h
@@ -30,8 +30,17 @@
  */
 class PosixIoStream : public StdioStream {
 public:
+#if defined(ANDROID_PLAIN_PORT)
+	bool createdWithSAF;
+	Common::String hackyfilename;
+#endif
+
 	static PosixIoStream *makeFromPath(const Common::String &path, bool writeMode);
 	PosixIoStream(void *handle);
+#if defined(ANDROID_PLAIN_PORT)
+	PosixIoStream(void *handle, bool bCreatedWithSAF, Common::String sHackyFilename);
+	~PosixIoStream();
+#endif
 
 	int32 size() const override;
 };
diff --git a/backends/platform/android/jni-android.cpp b/backends/platform/android/jni-android.cpp
index c5c98df91e..bf8d14493d 100644
--- a/backends/platform/android/jni-android.cpp
+++ b/backends/platform/android/jni-android.cpp
@@ -91,6 +91,9 @@ jmethodID JNI::_MID_convertEncoding = 0;
 jmethodID JNI::_MID_getAllStorageLocations = 0;
 jmethodID JNI::_MID_initSurface = 0;
 jmethodID JNI::_MID_deinitSurface = 0;
+jmethodID JNI::_MID_createDirectoryWithSAF = 0;
+jmethodID JNI::_MID_createFileWithSAF = 0;
+jmethodID JNI::_MID_closeFileWithSAF = 0;
 
 jmethodID JNI::_MID_EGL10_eglSwapBuffers = 0;
 
@@ -583,6 +586,9 @@ void JNI::create(JNIEnv *env, jobject self, jobject asset_manager,
 	FIND_METHOD(, convertEncoding, "(Ljava/lang/String;Ljava/lang/String;[B)[B");
 	FIND_METHOD(, initSurface, "()Ljavax/microedition/khronos/egl/EGLSurface;");
 	FIND_METHOD(, deinitSurface, "()V");
+	FIND_METHOD(, createDirectoryWithSAF, "(Ljava/lang/String;)Z");
+	FIND_METHOD(, createFileWithSAF, "(Ljava/lang/String;)Ljava/lang/String;");
+	FIND_METHOD(, closeFileWithSAF, "(Ljava/lang/String;)V");
 
 	_jobj_egl = env->NewGlobalRef(egl);
 	_jobj_egl_display = env->NewGlobalRef(egl_display);
@@ -801,5 +807,62 @@ Common::Array<Common::String> JNI::getAllStorageLocations() {
 	return *res;
 }
 
+bool JNI::createDirectoryWithSAF(const Common::String &dirPath) {
+	JNIEnv *env = JNI::getEnv();
+	jstring javaDirPath = env->NewStringUTF(dirPath.c_str());
+
+	bool created = env->CallBooleanMethod(_jobj, _MID_createDirectoryWithSAF, javaDirPath);
+
+	if (env->ExceptionCheck()) {
+		LOGE("JNI - Failed to create directory with SAF enhanced method");
+
+		env->ExceptionDescribe();
+		env->ExceptionClear();
+		created = false;
+	}
+
+	return created;
+
+}
+
+Common::String JNI::createFileWithSAF(const Common::String &filePath) {
+	JNIEnv *env = JNI::getEnv();
+	jstring javaFilePath = env->NewStringUTF(filePath.c_str());
+
+	jstring hackyFilenameJSTR = (jstring)env->CallObjectMethod(_jobj, _MID_createFileWithSAF, javaFilePath);
+
+
+	if (env->ExceptionCheck()) {
+		LOGE("JNI - Failed to create file with SAF enhanced method");
+
+		env->ExceptionDescribe();
+		env->ExceptionClear();
+		hackyFilenameJSTR = env->NewStringUTF("");
+	}
+
+	Common::String hackyFilenameStr = convertFromJString(env, hackyFilenameJSTR, "UTF-8");
+
+	//LOGD("JNI - _MID_createFileWithSAF returned %s", hackyFilenameStr.c_str());
+	env->DeleteLocalRef(hackyFilenameJSTR);
+
+	return hackyFilenameStr;
+
+}
+
+void JNI::closeFileWithSAF(const Common::String &hackyFilename) {
+	JNIEnv *env = JNI::getEnv();
+	jstring javaHackyFilename = env->NewStringUTF(hackyFilename.c_str());
+
+	env->CallVoidMethod(_jobj, _MID_closeFileWithSAF, javaHackyFilename);
+
+	if (env->ExceptionCheck()) {
+		LOGE("JNI - Failed to close file with SAF enhanced method");
+
+		env->ExceptionDescribe();
+		env->ExceptionClear();
+	}
+
+}
+
 
 #endif
diff --git a/backends/platform/android/jni-android.h b/backends/platform/android/jni-android.h
index a54ba3acf9..393c78ad6c 100644
--- a/backends/platform/android/jni-android.h
+++ b/backends/platform/android/jni-android.h
@@ -85,6 +85,10 @@ public:
 
 	static Common::Array<Common::String> getAllStorageLocations();
 
+	static bool createDirectoryWithSAF(const Common::String &dirPath);
+	static Common::String createFileWithSAF(const Common::String &filePath);
+	static void closeFileWithSAF(const Common::String &hackyFilename);
+
 private:
 	static JavaVM *_vm;
 	// back pointer to (java) peer instance
@@ -114,6 +118,9 @@ private:
 	static jmethodID _MID_getAllStorageLocations;
 	static jmethodID _MID_initSurface;
 	static jmethodID _MID_deinitSurface;
+	static jmethodID _MID_createDirectoryWithSAF;
+	static jmethodID _MID_createFileWithSAF;
+	static jmethodID _MID_closeFileWithSAF;
 
 	static jmethodID _MID_EGL10_eglSwapBuffers;
 
diff --git a/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java b/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
index db0f0c0de4..c6c60f5e41 100644
--- a/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
+++ b/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
@@ -456,7 +456,7 @@ public class ExternalStorage {
 		mMounts.clear();
 
 		if (Environment.getDataDirectory() != null
-		    && !Environment.getDataDirectory().getAbsolutePath().isEmpty()) {
+			&& !Environment.getDataDirectory().getAbsolutePath().isEmpty()) {
 			File dataFilePath = new File(Environment.getDataDirectory().getAbsolutePath());
 			if (dataFilePath.exists() && dataFilePath.isDirectory()) {
 				map.add(DATA_DIRECTORY);
diff --git a/backends/platform/android/org/scummvm/scummvm/ScummVM.java b/backends/platform/android/org/scummvm/scummvm/ScummVM.java
index e082c48991..e99357f76e 100644
--- a/backends/platform/android/org/scummvm/scummvm/ScummVM.java
+++ b/backends/platform/android/org/scummvm/scummvm/ScummVM.java
@@ -70,11 +70,13 @@ public abstract class ScummVM implements SurfaceHolder.Callback, Runnable {
 	abstract protected byte[] convertEncoding(String to, String from, byte[] string) throws UnsupportedEncodingException;
 	abstract protected String[] getAllStorageLocations();
 	abstract protected String[] getAllStorageLocationsNoPermissionRequest();
+	abstract protected boolean createDirectoryWithSAF(String dirPath);
+	abstract protected String createFileWithSAF(String filePath);
+	abstract protected void closeFileWithSAF(String hackyFilename);
 
 	public ScummVM(AssetManager asset_manager, SurfaceHolder holder) {
 		_asset_manager = asset_manager;
 		_sem_surface = new Object();
-
 		holder.addCallback(this);
 	}
 
diff --git a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
index ee9453aa8a..cc6cd1c779 100644
--- a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
+++ b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
@@ -8,12 +8,11 @@ import android.content.ClipboardManager;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.content.Intent;
+import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
 import android.content.res.AssetManager;
 import android.content.res.Configuration;
 import android.graphics.Rect;
-//import android.inputmethodservice.Keyboard;
-//import android.inputmethodservice.KeyboardView;
 import android.media.AudioManager;
 import android.net.Uri;
 import android.net.wifi.WifiInfo;
@@ -21,7 +20,9 @@ import android.net.wifi.WifiManager;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Environment;
+import android.os.ParcelFileDescriptor;
 import android.os.SystemClock;
+import android.text.TextUtils;
 import android.util.DisplayMetrics;
 import android.util.Log;
 import android.util.TypedValue;
@@ -41,6 +42,7 @@ import android.widget.Toast;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.RequiresApi;
+import androidx.documentfile.provider.DocumentFile;
 
 import java.io.BufferedReader;
 import java.io.File;
@@ -61,9 +63,6 @@ import java.util.TreeSet;
 
 import static android.content.res.Configuration.KEYBOARD_QWERTY;
 
-//import android.os.Environment;
-//import java.util.List;
-
 public class ScummVMActivity extends Activity implements OnKeyboardVisibilityListener {
 
 	/* Establish whether the hover events are available */
@@ -79,6 +78,10 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 	boolean _externalPathAvailableForReadAccess;
 //	private File _usingLogFile;
 
+	// SAF related
+	private LinkedHashMap<String, ParcelFileDescriptor> hackyNameToOpenFileDescriptorList;
+	public final static int REQUEST_SAF = 50000;
+
 	/**
 	 * Ids to identify an external storage read (and write) request.
 	 * They are app-defined int constants. The callback method gets the result of the request.
@@ -558,6 +561,7 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 
 
 	private class MyScummVM extends ScummVM {
+
 		public MyScummVM(SurfaceHolder holder) {
 			super(ScummVMActivity.this.getAssets(), holder);
 		}
@@ -678,9 +682,10 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 		@Override
 		protected String[] getAllStorageLocations() {
 			if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M
-				&& checkSelfPermission(Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED
+			    && (checkSelfPermission(Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED
+			        || checkSelfPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED)
 			) {
-				requestPermissions(new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, MY_PERMISSIONS_REQUEST_READ_EXT_STORAGE);
+				requestPermissions(MY_PERMISSIONS_STR_LIST, MY_PERMISSION_ALL);
 			} else {
 				return ExternalStorage.getAllStorageLocations(getApplicationContext()).toArray(new String[0]);
 			}
@@ -698,6 +703,149 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 			//      but for now, just return nothing
 			return new String[0]; // an array of zero length
 		}
+
+		// In this method we first try the old method for creating directories (mkdirs())
+		// That should work with app spaces but will probably have issues with external physical "secondary" storage locations
+		// (eg user SD Card) on some devices, anyway.
+		@Override
+		protected boolean createDirectoryWithSAF(String dirPath) {
+			final boolean[] retRes = {false};
+
+			Log.d(ScummVM.LOG_TAG, "Attempt to create folder on path: " + dirPath);
+			File folderToCreate = new File (dirPath);
+//			if (folderToCreate.canWrite()) {
+//				Log.d(ScummVM.LOG_TAG, "This file node has write permission!" + dirPath);
+//			}
+//
+//			if (folderToCreate.canRead()) {
+//				Log.d(ScummVM.LOG_TAG, "This file node has read permission!" + dirPath);
+//
+//			}
+//
+//			if (folderToCreate.getParentFile() != null) {
+//				if( folderToCreate.getParentFile().canWrite()) {
+//					Log.d(ScummVM.LOG_TAG, "The parent of this node permits write operation!" + dirPath);
+//				}
+//
+//				if (folderToCreate.getParentFile().canRead()) {
+//					Log.d(ScummVM.LOG_TAG, "The parent of this node permits read operation!" + dirPath);
+//
+//				}
+//			}
+
+			if (folderToCreate.mkdirs()) {
+				Log.d(ScummVM.LOG_TAG, "Folder created with the simple mkdirs() command!");
+			} else {
+				Log.d(ScummVM.LOG_TAG, "Folder creation with mkdirs() failed!");
+				if (getStorageAccessFrameworkTreeUri() == null) {
+					requestStorageAccessFramework(dirPath);
+					Log.d(ScummVM.LOG_TAG, "Requested Storage Access via Storage Access Framework!");
+				} else {
+					Log.d(ScummVM.LOG_TAG, "Already requested Storage Access (Storage Access Framework) in the past (share prefs saved)!");
+				}
+
+				if (canWriteFile(folderToCreate, true)) {
+					// TODO we should only need the callback if we want to do something with the file descriptor
+					//  (the writeFile will close it afterwards if keepFileDescriptorOpen is false)
+					Log.d(ScummVM.LOG_TAG, "(post SAF request) Writing is possible for this directory node");
+					writeFile(folderToCreate, true, false, new MyWriteFileCallback() {
+						@Override
+						public void handle(Boolean created, String hackyFilename) {
+							//Log.d(ScummVM.LOG_TAG, "Via callback: file operation success: " + created);
+							retRes[0] = created;
+						}
+					});
+				} else {
+					Log.d(ScummVM.LOG_TAG, "(post SAF request) Error - writing is still not possible for this directory node");
+
+				}
+			}
+
+//			// debug purpose
+//			if (folderToCreate.canWrite()) {
+//				// This is expected to return false here (since we don't check via SAF here)
+//				Log.d(ScummVM.LOG_TAG, "(post SAF access) We can write in folder:" + dirPath);
+//			}
+//			if (folderToCreate.canRead()) {
+//				// This will probably return true (at least for Android 28 and below)
+//				Log.d(ScummVM.LOG_TAG, "(post SAF access) We can read from folder:" + dirPath);
+//
+//			}
+
+			return retRes[0];
+		}
+
+		@Override
+		protected String createFileWithSAF(String filePath) {
+			final String[] retResStr = {""};
+			File fileToCreate = new File (filePath);
+
+			Log.d(ScummVM.LOG_TAG, "Attempting file creation for: " + filePath);
+
+			// normal (no SAF) file create attempt
+			boolean needToGoThroughSAF = false;
+			try {
+				if (fileToCreate.exists() || !fileToCreate.createNewFile()) {
+					Log.d(ScummVM.LOG_TAG, "The file already exists!");
+					// already existed
+				} else {
+					Log.d(ScummVM.LOG_TAG, "An empty file was created!");
+
+				}
+			} catch(Exception e) {
+				//e.printStackTrace();
+				needToGoThroughSAF = true;
+			}
+
+			if (needToGoThroughSAF) {
+				Log.d(ScummVM.LOG_TAG, "File creation with createNewFile() failed!");
+				if (getStorageAccessFrameworkTreeUri() == null) {
+					requestStorageAccessFramework(filePath);
+					Log.d(ScummVM.LOG_TAG, "Requested Storage Access via Storage Access Framework!");
+				}
+
+				if (canWriteFile(fileToCreate, false)) {
+					// TODO we should only need the callback if we want to do something with the file descriptor
+					//      (the writeFile will close it afterwards if keepFileDescriptorOpen is false)
+					//      we need the fileDescriptor open for the native to continue the write operation
+					Log.d(ScummVM.LOG_TAG, "(post SAF request check) File writing should be possible");
+					writeFile(fileToCreate, false, true, new MyWriteFileCallback() {
+						@Override
+						public void handle(Boolean created, String hackyFilename) {
+							//Log.d(ScummVM.LOG_TAG, "Via callback: file operation success: " + created + " :: " + hackyFilename);
+							if (created) {
+								retResStr[0] = hackyFilename;
+							} else {
+								retResStr[0] = "";
+							}
+						}
+					});
+				} else {
+					Log.e(ScummVM.LOG_TAG, "(post SAF request) Error - writing is still not possible for this directory node");
+				}
+			}
+			return retResStr[0];
+		}
+
+		@Override
+		protected void closeFileWithSAF(String hackyFileName) {
+			if (hackyNameToOpenFileDescriptorList.containsKey(hackyFileName)) {
+				ParcelFileDescriptor openFileDescriptor = hackyNameToOpenFileDescriptorList.get(hackyFileName);
+
+				Log.d(ScummVM.LOG_TAG, "Closing file descriptor for " + hackyFileName);
+				if (openFileDescriptor != null) {
+					try {
+						openFileDescriptor.close();
+					} catch (IOException e) {
+						Log.e(ScummVM.LOG_TAG, e.getMessage());
+						e.printStackTrace();
+					}
+				}
+				hackyNameToOpenFileDescriptorList.remove(hackyFileName);
+			}
+		}
+
+		// TODO do we also need SAF enabled methods for deletion (file/folder) and reading (for files), listing of files (for folders)?
 	}
 
 	private MyScummVM _scummvm;
@@ -710,12 +858,14 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 	public void onCreate(Bundle savedInstanceState) {
 		super.onCreate(savedInstanceState);
 
+		hackyNameToOpenFileDescriptorList = new LinkedHashMap<>();
+
 		hideSystemUI();
 
 		_videoLayout = new FrameLayout(this);
 		SetLayerType.get().setLayerType(_videoLayout);
-		setContentView(_videoLayout);
 		getWindow().addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON);
+		setContentView(_videoLayout);
 		_videoLayout.setFocusable(true);
 		_videoLayout.setFocusableInTouchMode(true);
 		_videoLayout.requestFocus();
@@ -864,6 +1014,23 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 
 		super.onDestroy();
 
+		// close any open file descriptors due to the SAF code
+		for (String hackyFileName : hackyNameToOpenFileDescriptorList.keySet()) {
+			Log.d(ScummVM.LOG_TAG, "Destroy: Closing file descriptor for " + hackyFileName);
+
+			ParcelFileDescriptor openFileDescriptor = hackyNameToOpenFileDescriptorList.get(hackyFileName);
+
+			if (openFileDescriptor != null) {
+				try {
+					openFileDescriptor.close();
+				} catch (IOException e) {
+					Log.e(ScummVM.LOG_TAG, e.getMessage());
+					e.printStackTrace();
+				}
+			}
+		}
+		hackyNameToOpenFileDescriptorList.clear();
+
 		if (_events != null) {
 			_events.clearEventHandler();
 			_events.sendQuitEvent();
@@ -1887,6 +2054,234 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 			}
 		}
 	}
+
+	// -------------------------------------------------------------------------------------------
+	// Start of SAF enabled code
+	// Code borrows parts from open source project: OpenLaucher's SharedUtil class
+	// https://github.com/OpenLauncherTeam/openlauncher
+	// https://github.com/OpenLauncherTeam/openlauncher/blob/master/app/src/main/java/net/gsantner/opoc/util/ShareUtil.java
+	// as well as StackOverflow threads:
+	// https://stackoverflow.com/questions/43066117/android-m-write-to-sd-card-permission-denied
+	// https://stackoverflow.com/questions/59000390/android-accessing-files-in-native-c-c-code-with-google-scoped-storage-api
+	// -------------------------------------------------------------------------------------------
+	public void onActivityResult(int requestCode, int resultCode, Intent resultData) {
+		if (resultCode != RESULT_OK)
+			return;
+		else {
+			if (requestCode == REQUEST_SAF) {
+				if (resultCode == RESULT_OK && resultData != null && resultData.getData() != null) {
+					Uri treeUri = resultData.getData();
+					//SharedPreferences sharedPref = getApplicationContext().getSharedPreferences(getApplicationContext().getPackageName() + "_preferences", Context.MODE_PRIVATE);
+					SharedPreferences sharedPref = getPreferences(Context.MODE_PRIVATE);
+
+					SharedPreferences.Editor editor = sharedPref.edit();
+					editor.putString(getString(R.string.preference_saf_tree_key), treeUri.toString());
+					editor.apply();
+
+					if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
+						getContentResolver().takePersistableUriPermission(treeUri, Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
+					}
+					return;
+				}
+			}
+		}
+	}
+
+	/***
+	 * Request storage access. The user needs to press "Select storage" at the correct storage.
+	 */
+	public void requestStorageAccessFramework(String dirPathSample) {
+
+		_scummvm.displayMessageOnOSD(getString(R.string.saf_request_prompt) + dirPathSample);
+
+		if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.LOLLIPOP) {
+			Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT_TREE);
+			intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION
+			                | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
+			                | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION
+			                | Intent.FLAG_GRANT_PREFIX_URI_PERMISSION
+			);
+			startActivityForResult(intent, REQUEST_SAF);
+		}
+	}
+
+	/**
+	 * Get storage access framework tree uri. The user must have granted access via requestStorageAccessFramework
+	 *
+	 * @return Uri or null if not granted yet
+	 */
+	public Uri getStorageAccessFrameworkTreeUri() {
+		SharedPreferences sharedPref = getPreferences(Context.MODE_PRIVATE);
+		String treeStr = sharedPref.getString(getString(R.string.preference_saf_tree_key), null);
+
+		if (!TextUtils.isEmpty(treeStr)) {
+			try {
+				Log.d(ScummVM.LOG_TAG, "getStorageAccessFrameworkTreeUri: " + treeStr);
+				return Uri.parse(treeStr);
+			} catch (Exception ignored) {
+			}
+		}
+		return null;
+	}
+
+	public File getStorageRootFolder(final File file) {
+		String filepath;
+		try {
+			filepath = file.getCanonicalPath();
+		} catch (Exception ignored) {
+			return null;
+		}
+
+		for (String storagePath : _scummvm.getAllStorageLocationsNoPermissionRequest() ) {
+			if (filepath.startsWith(storagePath)) {
+				return new File(storagePath);
+			}
+		}
+		return null;
+	}
+
+	// TODO we need to implement support for reading access somewhere too
+	@SuppressWarnings({"ResultOfMethodCallIgnored", "StatementWithEmptyBody"})
+	public void writeFile(final File file, final boolean isDirectory, final boolean keepFileDescriptorOpen, final MyWriteFileCallback writeFileCallback ) {
+		try {
+			// TODO we need code for read access too (even though currently API28 reading works without SAF, just with the runtime permissions)
+			String hackyFilename = "";
+
+			ParcelFileDescriptor pfd = null;
+			if (file.canWrite() || (!file.exists() && file.getParentFile().canWrite())) {
+				if (isDirectory) {
+					file.mkdirs();
+				} else {
+					// If we are here this means creating a new file can be done with fopen from native
+					//fileOutputStream = new FileOutputStream(file);
+					Log.d(ScummVM.LOG_TAG, "writeFile() file can be created normally -- (not created here)" );
+					hackyFilename = "";
+				}
+			} else {
+				DocumentFile dof = getDocumentFile(file, isDirectory);
+				if (dof != null && dof.getUri() != null && dof.canWrite()) {
+					if (isDirectory) {
+						// Nothing more to do
+					} else {
+						pfd = getContentResolver().openFileDescriptor(dof.getUri(), "w");
+						if (pfd != null) {
+							// https://stackoverflow.com/questions/59000390/android-accessing-files-in-native-c-c-code-with-google-scoped-storage-api
+							int fd = pfd.getFd();
+							hackyFilename = "/proc/self/fd/" + fd;
+							hackyNameToOpenFileDescriptorList.put(hackyFilename, pfd);
+							Log.d(ScummVM.LOG_TAG, "writeFile() file created with SAF -- hacky name: " + hackyFilename );
+						}
+					}
+				}
+			}
+
+			// TODO the idea of a callback is to work with the output (or input) streams, then return here and close the streams and the descriptors properly
+			//      however since we are interacting with native this would not work for those cases
+
+			if (writeFileCallback != null) {
+				writeFileCallback.handle( (isDirectory && file.exists()) || (!isDirectory && file.exists() && file.isFile() ), hackyFilename);
+
+			}
+
+			// TODO We need to close the file descriptor when we are done with it from native
+			//		- what if the call is not from native but from the activity?
+			//      - directory operations don't create or need a file descriptor
+			if (!keepFileDescriptorOpen && pfd != null) {
+				if (hackyNameToOpenFileDescriptorList.containsKey(hackyFilename)) {
+					hackyNameToOpenFileDescriptorList.remove(hackyFilename);
+				}
+				pfd.close();
+			}
+		} catch (Exception e) {
+			e.printStackTrace();
+		}
+	}
+
+	/**
+	 * Get a DocumentFile object out of a normal java File object.
+	 * When used on a external storage (SD), use requestStorageAccessFramework()
+	 * first to get access. Otherwise this will fail.
+	 *
+	 * @param file  The file/folder to convert
+	 * @param isDir Whether or not file is a directory. For non-existing (to be created) files this info is not known hence required.
+	 * @return A DocumentFile object or null if file cannot be converted
+	 */
+	@SuppressWarnings("RegExpRedundantEscape")
+	public DocumentFile getDocumentFile(final File file, final boolean isDir) {
+		// On older versions use fromFile
+		if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.KITKAT) {
+			return DocumentFile.fromFile(file);
+		}
+
+		// Get ContextUtils to find storageRootFolder
+		File baseFolderFile = getStorageRootFolder(file);
+
+		String baseFolder = baseFolderFile == null ? null : baseFolderFile.getAbsolutePath();
+		boolean originalDirectory = false;
+		if (baseFolder == null) {
+			return null;
+		}
+
+		String relPath = null;
+		try {
+			String fullPath = file.getCanonicalPath();
+			if (!baseFolder.equals(fullPath)) {
+				relPath = fullPath.substring(baseFolder.length() + 1);
+			} else {
+				originalDirectory = true;
+			}
+		} catch (IOException e) {
+			return null;
+		} catch (Exception ignored) {
+			originalDirectory = true;
+		}
+		Uri treeUri;
+		if ((treeUri = getStorageAccessFrameworkTreeUri()) == null) {
+			return null;
+		}
+		DocumentFile dof = DocumentFile.fromTreeUri(getApplicationContext(), treeUri);
+		if (originalDirectory) {
+			return dof;
+		}
+		String[] parts = relPath.split("\\/");
+		for (int i = 0; i < parts.length; i++) {
+			DocumentFile nextDof = dof.findFile(parts[i]);
+			if (nextDof == null) {
+				try {
+					nextDof = ((i < parts.length - 1) || isDir) ? dof.createDirectory(parts[i]) : dof.createFile("image", parts[i]);
+				} catch (Exception ignored) {
+					nextDof = null;
+				}
+			}
+			dof = nextDof;
+		}
+		return dof;
+	}
+
+	/**
+	 * Check whether or not a file can be written.
+	 * Requires storage access framework permission for external storage (SD)
+	 *
+	 * @param file  The file object (file/folder)
+	 * @param isDirectory Whether or not the given file parameter is a directory
+	 * @return Whether or not the file can be written
+	 */
+	public boolean canWriteFile(final File file, final boolean isDirectory) {
+		if (file == null) {
+			return false;
+		} else if (file.getAbsolutePath().startsWith(Environment.getExternalStorageDirectory().getAbsolutePath())
+		           || file.getAbsolutePath().startsWith(getFilesDir().getAbsolutePath())) {
+			return (!isDirectory && file.getParentFile() != null) ? file.getParentFile().canWrite() : file.canWrite();
+		} else {
+			DocumentFile dof = getDocumentFile(file, isDirectory);
+			return dof != null && dof.canWrite();
+		}
+	}
+	// -------------------------------------------------------------------------------------------
+	// End of SAF enabled code
+	// -------------------------------------------------------------------------------------------
+
+
 } // end of ScummVMActivity
 
 // *** HONEYCOMB / ICS FIX FOR FULLSCREEN MODE, by lmak ***
@@ -1960,3 +2355,8 @@ abstract class SetLayerType {
 		public void setLayerType(final View view) { }
 	}
 }
+
+// Used to define the interface for a callback after a write operation (via the method that is enhanced to use SAF if the normal way fails)
+interface MyWriteFileCallback {
+	public void handle(Boolean created, String hackyFilename);
+}
diff --git a/dists/android/build.gradle b/dists/android/build.gradle
index 523ac1e03e..2a7c6c6fd1 100644
--- a/dists/android/build.gradle
+++ b/dists/android/build.gradle
@@ -81,4 +81,5 @@ android {
 
 dependencies {
     implementation "androidx.annotation:annotation:1.1.0"
+    implementation "androidx.documentfile:documentfile:1.0.1"
 }
diff --git a/dists/android/res/values/strings.xml b/dists/android/res/values/strings.xml
index 9c99e5dfb9..7a9cb9d05c 100644
--- a/dists/android/res/values/strings.xml
+++ b/dists/android/res/values/strings.xml
@@ -53,4 +53,7 @@
 	<string name="customkeyboardview_keycode_enter">Enter</string>
 	<!-- End of copy from AOSP -->
 	<string name="customkeyboardview_popup_close">Close popup</string>
+
+	<string name="saf_request_prompt">Please select the *root* of your external (physical) SD card. This is required for ScummVM to access this path: </string>
+	<string name="preference_saf_tree_key" translatable="false">pref_key__saf_tree_uri</string>
 </resources>


Commit: 7aa38e197a241c889c1ee761b3221f0e327ce47d
    https://github.com/scummvm/scummvm/commit/7aa38e197a241c889c1ee761b3221f0e327ce47d
Author: antoniou (a.antoniou79 at gmail.com)
Date: 2020-11-01T19:38:07+02:00

Commit Message:
ANDROID: Callback for onDestroy at ScummVM thread end

Changed paths:
    backends/platform/android/org/scummvm/scummvm/ScummVM.java
    backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java


diff --git a/backends/platform/android/org/scummvm/scummvm/ScummVM.java b/backends/platform/android/org/scummvm/scummvm/ScummVM.java
index e99357f76e..a5471a8d25 100644
--- a/backends/platform/android/org/scummvm/scummvm/ScummVM.java
+++ b/backends/platform/android/org/scummvm/scummvm/ScummVM.java
@@ -25,6 +25,7 @@ public abstract class ScummVM implements SurfaceHolder.Callback, Runnable {
 	final protected static String LOG_TAG = "ScummVM";
 	final private AssetManager _asset_manager;
 	final private Object _sem_surface;
+	final private MyScummVMDestroyedCallback _svm_destroyed_callback;
 
 	private EGL10 _egl;
 	private EGLDisplay _egl_display = EGL10.EGL_NO_DISPLAY;
@@ -74,9 +75,10 @@ public abstract class ScummVM implements SurfaceHolder.Callback, Runnable {
 	abstract protected String createFileWithSAF(String filePath);
 	abstract protected void closeFileWithSAF(String hackyFilename);
 
-	public ScummVM(AssetManager asset_manager, SurfaceHolder holder) {
+	public ScummVM(AssetManager asset_manager, SurfaceHolder holder, final MyScummVMDestroyedCallback scummVMDestroyedCallback) {
 		_asset_manager = asset_manager;
 		_sem_surface = new Object();
+		_svm_destroyed_callback = scummVMDestroyedCallback;
 		holder.addCallback(this);
 	}
 
@@ -155,8 +157,11 @@ public abstract class ScummVM implements SurfaceHolder.Callback, Runnable {
 		deinitAudio();
 
 		destroy();
-		// On exit, tear everything down for a fresh restart next time.
-		System.exit(res);
+
+		// Don't exit force-ably here!
+		if (_svm_destroyed_callback != null) {
+			_svm_destroyed_callback.handle(res);
+		}
 	}
 
 	private void initEGL() throws Exception {
diff --git a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
index cc6cd1c779..e4d088a520 100644
--- a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
+++ b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
@@ -562,8 +562,8 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 
 	private class MyScummVM extends ScummVM {
 
-		public MyScummVM(SurfaceHolder holder) {
-			super(ScummVMActivity.this.getAssets(), holder);
+		public MyScummVM(SurfaceHolder holder, final MyScummVMDestroyedCallback destroyedCallback) {
+			super(ScummVMActivity.this.getAssets(), holder, destroyedCallback);
 		}
 
 		@Override
@@ -915,7 +915,14 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 		//                            so app's internal space (which would be deleted on uninstall) was set as WORLD_READABLE which is no longer supported in newer versions of Android API
 		//                            In newer APIs we can set that path as Context.MODE_PRIVATE which is the default - but this makes the files inaccessible to other apps
 
-		_scummvm = new MyScummVM(_main_surface.getHolder());
+		_scummvm = new MyScummVM(_main_surface.getHolder(), new MyScummVMDestroyedCallback() {
+		                                                        @Override
+		                                                        public void handle(int exitResult) {
+		                                                        	Log.d(ScummVM.LOG_TAG, "Via callback: ScummVM native terminated with code: " + exitResult);
+		                                                        	// call onDestroy()
+		                                                        	finish();
+		                                                        }
+		                                                    });
 
 		//
 		// seekAndInitScummvmConfiguration() returns false if something went wrong
@@ -2360,3 +2367,8 @@ abstract class SetLayerType {
 interface MyWriteFileCallback {
 	public void handle(Boolean created, String hackyFilename);
 }
+
+// Used to define the interface for a callback after ScummVM thread has finished
+interface MyScummVMDestroyedCallback {
+	public void handle(int exitResult);
+}


Commit: 4bda9bf1121537277b333de40fca9a71f99aac31
    https://github.com/scummvm/scummvm/commit/4bda9bf1121537277b333de40fca9a71f99aac31
Author: antoniou (a.antoniou79 at gmail.com)
Date: 2020-11-01T19:38:07+02:00

Commit Message:
ANDROID: Use TextUtils.isEmpty() instead of String's isEmpty()

Changed paths:
    backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
    backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java


diff --git a/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java b/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
index c6c60f5e41..8a42add48e 100644
--- a/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
+++ b/backends/platform/android/org/scummvm/scummvm/ExternalStorage.java
@@ -322,7 +322,7 @@ public class ExternalStorage {
 	 */
 	private static void addPath(String strNew, Collection<File> paths) {
 		// If one of the arguments is null, fill it in from the other.
-		if (strNew != null && !strNew.isEmpty()) {
+		if (!TextUtils.isEmpty(strNew)) {
 			File fileNew = new File(strNew);
 
 			if (!paths.contains(fileNew) &&
@@ -456,7 +456,7 @@ public class ExternalStorage {
 		mMounts.clear();
 
 		if (Environment.getDataDirectory() != null
-			&& !Environment.getDataDirectory().getAbsolutePath().isEmpty()) {
+			&& !TextUtils.isEmpty(Environment.getDataDirectory().getAbsolutePath())) {
 			File dataFilePath = new File(Environment.getDataDirectory().getAbsolutePath());
 			if (dataFilePath.exists() && dataFilePath.isDirectory()) {
 				map.add(DATA_DIRECTORY);
diff --git a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
index e4d088a520..6a590d60c5 100644
--- a/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
+++ b/backends/platform/android/org/scummvm/scummvm/ScummVMActivity.java
@@ -1571,7 +1571,7 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 				Log.d(ScummVM.LOG_TAG, "ScummVM Config file already exists!");
 				Log.d(ScummVM.LOG_TAG, "Existing ScummVM INI: " + _configScummvmFile.getPath());
 				String existingVersionInfo = getVersionInfoFromScummvmConfiguration(_configScummvmFile.getPath());
-				if (!existingVersionInfo.trim().isEmpty()) {
+				if (!TextUtils.isEmpty(existingVersionInfo) && !TextUtils.isEmpty(existingVersionInfo.trim()) ) {
 					Log.d(ScummVM.LOG_TAG, "Existing ScummVM Version: " + existingVersionInfo.trim());
 					Version tmpOldVersionFound = new Version(existingVersionInfo.trim());
 					if (tmpOldVersionFound.compareTo(maxOldVersionFound) > 0) {
@@ -1603,7 +1603,7 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 					if (oldCandidateScummVMConfig.exists() && oldCandidateScummVMConfig.isFile()) {
 						Log.d(ScummVM.LOG_TAG, "Old config " + oldConfigFileDescription + " ScummVM file was found!");
 						String existingVersionInfo = getVersionInfoFromScummvmConfiguration(oldCandidateScummVMConfig.getPath());
-						if (!existingVersionInfo.trim().isEmpty()) {
+						if (!TextUtils.isEmpty(existingVersionInfo) && !TextUtils.isEmpty(existingVersionInfo.trim())) {
 							Log.d(ScummVM.LOG_TAG, "Old config's ScummVM version: " + existingVersionInfo.trim());
 							Version tmpOldVersionFound = new Version(existingVersionInfo.trim());
 							//
@@ -1927,7 +1927,7 @@ public class ScummVMActivity extends Activity implements OnKeyboardVisibilityLis
 		if (_configScummvmFile.exists() && _configScummvmFile.isFile()) {
 			Log.d(ScummVM.LOG_TAG, "Looking into config file for save path: " + _configScummvmFile.getPath());
 			String persistentGlobalSavePathStr = getSavepathInfoFromScummvmConfiguration(_configScummvmFile.getPath());
-			if (!persistentGlobalSavePathStr.trim().isEmpty()) {
+			if (!TextUtils.isEmpty(persistentGlobalSavePathStr) && !TextUtils.isEmpty(persistentGlobalSavePathStr.trim()) ) {
 				Log.d(ScummVM.LOG_TAG, "Found explicit save path: " + persistentGlobalSavePathStr);
 				persistentGlobalSavePath = new File(persistentGlobalSavePathStr);
 				if (persistentGlobalSavePath.exists() && persistentGlobalSavePath.isDirectory() && persistentGlobalSavePath.listFiles() != null) {




More information about the Scummvm-git-logs mailing list