[Libreoffice-commits] core.git: android/source

[Michael Weghorn (via logerrit)]

 android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java |   63 ++++++----
 1 file changed, 44 insertions(+), 19 deletions(-)

New commits:
commit 7f838b73e85eb6f0a1dce4647650a5cf5f34ccd2
Author:     Michael Weghorn <m.weghorn at posteo.de>
AuthorDate: Fri Mar 19 15:46:36 2021 +0100
Commit:     Michael Weghorn <m.weghorn at posteo.de>
CommitDate: Mon Mar 22 07:58:11 2021 +0100

    tdf#129833 android: Move reading file to separate thread
    
    Reading the input file from the main thread is problematic
    when that happens over the network.
    
    For example, trying to opening a file in a NextCloud share
    using the system file picker from within the LO Android Viewer
    app (with NextCloud app 3.15.1 serving as DocumentsProvider
    for the NextCloud share in the file chooser) previously
    resulted in a crash, with this in ADB log:
    
        I DownloadFileOperation: Download of /Documents/five_pages.odt to /storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/<USERNAME>@demo2.nextcloud.com/Documents/five_pages.odt: Unexpected exception
        E DocumentsStorageProvider: RemoteOperationResult(mSuccess=false, mHttpCode=-1, mHttpPhrase=null, mException=android.os.NetworkOnMainThreadException, mCode=HOST_NOT_AVAILABLE, message=null, getLogMessage=Unexpected exception)
    
    Moving this to a separate thread fixes the
    NetworkOnMainThreadException and made opening, editing
    and saving the modified file back work successfully
    for that scenario.
    (Using a separate thread when writing back does not
    seem to be necessary, but could be added in a similar
    way.)
    
    This just moves the IO to a new thread and then waits
    for its completion.
    For a better user experience in cases where the copy
    operation may be slow, providing some additional
    feedback in the UI might be useful.
    
    Change-Id: I58e2c4bb1dcd2d59383fba0f216c9614f5d3e3a7
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/112769
    Tested-by: Jenkins
    Reviewed-by: Michael Weghorn <m.weghorn at posteo.de>

diff --git a/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java b/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java
index bf6108edc8af..368d19af1375 100644
--- a/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java
+++ b/android/source/src/java/org/libreoffice/LibreOfficeMainActivity.java
@@ -297,9 +297,7 @@ public class LibreOfficeMainActivity extends AppCompatActivity implements Settin
     }
 
     private boolean copyFileToTemp() {
-        ContentResolver contentResolver = getContentResolver();
-        FileChannel inputChannel = null;
-        FileChannel outputChannel = null;
+        final ContentResolver contentResolver = getContentResolver();
         // CSV files need a .csv suffix to be opened in Calc.
         String suffix = null;
         String intentType = getIntent().getType();
@@ -308,26 +306,53 @@ public class LibreOfficeMainActivity extends AppCompatActivity implements Settin
             suffix = ".csv";
 
         try {
+            mTempFile = File.createTempFile("LibreOffice", suffix, this.getCacheDir());
+            final FileChannel outputChannel = new FileOutputStream(mTempFile).getChannel();
             try {
-                AssetFileDescriptor assetFD = contentResolver.openAssetFileDescriptor(getIntent().getData(), "r");
-                if (assetFD == null) {
-                    Log.e(LOGTAG, "couldn't create assetfiledescriptor from " + getIntent().getDataString());
-                    return false;
-                }
-                inputChannel = assetFD.createInputStream().getChannel();
-                mTempFile = File.createTempFile("LibreOffice", suffix, this.getCacheDir());
+                // need to run copy operation in a separate thread, since network access is not
+                // allowed from main thread, but that may happen here when underlying
+                // DocumentsProvider (like the NextCloud one) does that
+                class CopyThread extends Thread {
+                    /** Whether copy operation was successful. */
+                    private boolean result = false;
 
-                outputChannel = new FileOutputStream(mTempFile).getChannel();
-                long bytesTransferred = 0;
-                // might not  copy all at once, so make sure everything gets copied...
-                while (bytesTransferred < inputChannel.size()) {
-                    bytesTransferred += outputChannel.transferFrom(inputChannel, bytesTransferred, inputChannel.size());
+                    @Override
+                    public void run() {
+                        result = false;
+                        try {
+                            final AssetFileDescriptor assetFD = contentResolver.openAssetFileDescriptor(getIntent().getData(), "r");
+                            if (assetFD == null) {
+                                Log.e(LOGTAG, "couldn't create assetfiledescriptor from " + getIntent().getDataString());
+                                return;
+                            }
+                            FileChannel inputChannel = assetFD.createInputStream().getChannel();
+                            long bytesTransferred = 0;
+                            // might not  copy all at once, so make sure everything gets copied...
+                            while (bytesTransferred < inputChannel.size()) {
+                                bytesTransferred += outputChannel.transferFrom(inputChannel, bytesTransferred, inputChannel.size());
+                            }
+                            Log.e(LOGTAG, "Success copying " + bytesTransferred + " bytes");
+                            inputChannel.close();
+                        } catch (IOException e) {
+                            e.printStackTrace();
+                            return;
+                        }
+                        result = true;
+                    }
+                };
+                CopyThread copyThread = new CopyThread();
+                copyThread.start();
+                try {
+                    // wait for copy operation to finish
+                    // NOTE: might be useful to add some indicator in UI for long copy operations involving network...
+                    copyThread.join();
+                } catch (InterruptedException e) {
+                    e.printStackTrace();
                 }
-                Log.e(LOGTAG, "Success copying " + bytesTransferred + " bytes");
-                return true;
+
+                return copyThread.result;
             } finally {
-                if (inputChannel != null) inputChannel.close();
-                if (outputChannel != null) outputChannel.close();
+                outputChannel.close();
             }
         } catch (FileNotFoundException e) {
             return false;