Move more folks to FileUtils.copy().

Also extend API to accept a "count" argument of exactly how many
bytes to copy, and return the actual number of copied bytes.

Improve docs.

Test: bit FrameworksCoreTests:android.os.FileUtilsTest
Test: vogar --mode app_process --benchmark frameworks/base/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java
Bug: 71932978
Change-Id: I8d255e4f97462838c02a8ecb6d2d221188c4eff0

diff --git a/core/java/android/app/WallpaperManager.java b/core/java/android/app/WallpaperManager.java
index f21746c..39bccc3 100644 (file)
--- a/core/java/android/app/WallpaperManager.java
+++ b/core/java/android/app/WallpaperManager.java
@@ -51,6 +51,7 @@ import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.DeadSystemException;
+import android.os.FileUtils;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.Looper;
@@ -1329,11 +1330,7 @@ public class WallpaperManager {
 
     private void copyStreamToWallpaperFile(InputStream data, FileOutputStream fos)
             throws IOException {
-        byte[] buffer = new byte[32768];
-        int amt;
-        while ((amt=data.read(buffer)) > 0) {
-            fos.write(buffer, 0, amt);
-        }
+        FileUtils.copy(data, fos);
     }
 
     /**

diff --git a/core/java/android/os/FileUtils.java b/core/java/android/os/FileUtils.java
index 21d245d..1160415 100644 (file)
--- a/core/java/android/os/FileUtils.java
+++ b/core/java/android/os/FileUtils.java
@@ -33,6 +33,7 @@ import android.util.Slog;
 import android.webkit.MimeTypeMap;
 
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.util.SizedInputStream;
 
 import libcore.io.IoUtils;
 import libcore.util.EmptyArray;
@@ -93,7 +94,7 @@ public class FileUtils {
 
     private static final long COPY_CHECKPOINT_BYTES = 524288;
 
-    public interface CopyListener {
+    public interface ProgressListener {
         public void onProgress(long progress);
     }
 
@@ -202,7 +203,7 @@ public class FileUtils {
     }
 
     /**
-     * @deprecated use {@link #copy(InputStream, OutputStream)} instead.
+     * @deprecated use {@link #copy(File, File)} instead.
      */
     @Deprecated
     public static boolean copyFile(File srcFile, File destFile) {
@@ -215,7 +216,7 @@ public class FileUtils {
     }
 
     /**
-     * @deprecated use {@link #copy(InputStream, OutputStream)} instead.
+     * @deprecated use {@link #copy(File, File)} instead.
      */
     @Deprecated
     public static void copyFileOrThrow(File srcFile, File destFile) throws IOException {
@@ -255,44 +256,124 @@ public class FileUtils {
         }
     }
 
-    public static void copy(File from, File to) throws IOException {
+    /**
+     * Copy the contents of one file to another, replacing any existing content.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull File from, @NonNull File to) throws IOException {
+        return copy(from, to, null, null);
+    }
+
+    /**
+     * Copy the contents of one file to another, replacing any existing content.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @param listener to be periodically notified as the copy progresses.
+     * @param signal to signal if the copy should be cancelled early.
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull File from, @NonNull File to,
+            @Nullable ProgressListener listener, @Nullable CancellationSignal signal)
+            throws IOException {
         try (FileInputStream in = new FileInputStream(from);
                 FileOutputStream out = new FileOutputStream(to)) {
-            copy(in, out);
+            return copy(in, out, listener, signal);
         }
     }
 
-    public static void copy(InputStream in, OutputStream out) throws IOException {
-        copy(in, out, null, null);
+    /**
+     * Copy the contents of one stream to another.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull InputStream in, @NonNull OutputStream out) throws IOException {
+        return copy(in, out, null, null);
     }
 
-    public static void copy(InputStream in, OutputStream out, CopyListener listener,
-            CancellationSignal signal) throws IOException {
+    /**
+     * Copy the contents of one stream to another.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @param listener to be periodically notified as the copy progresses.
+     * @param signal to signal if the copy should be cancelled early.
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull InputStream in, @NonNull OutputStream out,
+            @Nullable ProgressListener listener, @Nullable CancellationSignal signal)
+            throws IOException {
         if (ENABLE_COPY_OPTIMIZATIONS) {
             if (in instanceof FileInputStream && out instanceof FileOutputStream) {
-                copy(((FileInputStream) in).getFD(), ((FileOutputStream) out).getFD(),
+                return copy(((FileInputStream) in).getFD(), ((FileOutputStream) out).getFD(),
                         listener, signal);
             }
         }
 
         // Worse case fallback to userspace
-        copyInternalUserspace(in, out, listener, signal);
+        return copyInternalUserspace(in, out, listener, signal);
     }
 
-    public static void copy(FileDescriptor in, FileDescriptor out) throws IOException {
-        copy(in, out, null, null);
+    /**
+     * Copy the contents of one FD to another.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull FileDescriptor in, @NonNull FileDescriptor out)
+            throws IOException {
+        return copy(in, out, null, null);
+    }
+
+    /**
+     * Copy the contents of one FD to another.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @param listener to be periodically notified as the copy progresses.
+     * @param signal to signal if the copy should be cancelled early.
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull FileDescriptor in, @NonNull FileDescriptor out,
+            @Nullable ProgressListener listener, @Nullable CancellationSignal signal)
+            throws IOException {
+        return copy(in, out, listener, signal, Long.MAX_VALUE);
     }
 
-    public static void copy(FileDescriptor in, FileDescriptor out, CopyListener listener,
-            CancellationSignal signal) throws IOException {
+    /**
+     * Copy the contents of one FD to another.
+     * <p>
+     * Attempts to use several optimization strategies to copy the data in the
+     * kernel before falling back to a userspace copy as a last resort.
+     *
+     * @param listener to be periodically notified as the copy progresses.
+     * @param signal to signal if the copy should be cancelled early.
+     * @param count the number of bytes to copy.
+     * @return number of bytes copied.
+     */
+    public static long copy(@NonNull FileDescriptor in, @NonNull FileDescriptor out,
+            @Nullable ProgressListener listener, @Nullable CancellationSignal signal, long count)
+            throws IOException {
         if (ENABLE_COPY_OPTIMIZATIONS) {
             try {
                 final StructStat st_in = Os.fstat(in);
                 final StructStat st_out = Os.fstat(out);
                 if (S_ISREG(st_in.st_mode) && S_ISREG(st_out.st_mode)) {
-                    copyInternalSendfile(in, out, listener, signal);
+                    return copyInternalSendfile(in, out, listener, signal, count);
                 } else if (S_ISFIFO(st_in.st_mode) || S_ISFIFO(st_out.st_mode)) {
-                    copyInternalSplice(in, out, listener, signal);
+                    return copyInternalSplice(in, out, listener, signal, count);
                 }
             } catch (ErrnoException e) {
                 throw e.rethrowAsIOException();
@@ -300,23 +381,25 @@ public class FileUtils {
         }
 
         // Worse case fallback to userspace
-        copyInternalUserspace(in, out, listener, signal);
+        return copyInternalUserspace(in, out, listener, signal, count);
     }
 
     /**
      * Requires one of input or output to be a pipe.
      */
     @VisibleForTesting
-    public static void copyInternalSplice(FileDescriptor in, FileDescriptor out,
-            CopyListener listener, CancellationSignal signal) throws ErrnoException {
+    public static long copyInternalSplice(FileDescriptor in, FileDescriptor out,
+            ProgressListener listener, CancellationSignal signal, long count)
+            throws ErrnoException {
         long progress = 0;
         long checkpoint = 0;
 
         long t;
-        while ((t = Os.splice(in, null, out, null, COPY_CHECKPOINT_BYTES,
+        while ((t = Os.splice(in, null, out, null, Math.min(count, COPY_CHECKPOINT_BYTES),
                 SPLICE_F_MOVE | SPLICE_F_MORE)) != 0) {
             progress += t;
             checkpoint += t;
+            count -= t;
 
             if (checkpoint >= COPY_CHECKPOINT_BYTES) {
                 if (signal != null) {
@@ -328,21 +411,24 @@ public class FileUtils {
                 checkpoint = 0;
             }
         }
+        return progress;
     }
 
     /**
      * Requires both input and output to be a regular file.
      */
     @VisibleForTesting
-    public static void copyInternalSendfile(FileDescriptor in, FileDescriptor out,
-            CopyListener listener, CancellationSignal signal) throws ErrnoException {
+    public static long copyInternalSendfile(FileDescriptor in, FileDescriptor out,
+            ProgressListener listener, CancellationSignal signal, long count)
+            throws ErrnoException {
         long progress = 0;
         long checkpoint = 0;
 
         long t;
-        while ((t = Os.sendfile(out, in, null, COPY_CHECKPOINT_BYTES)) != 0) {
+        while ((t = Os.sendfile(out, in, null, Math.min(count, COPY_CHECKPOINT_BYTES))) != 0) {
             progress += t;
             checkpoint += t;
+            count -= t;
 
             if (checkpoint >= COPY_CHECKPOINT_BYTES) {
                 if (signal != null) {
@@ -354,17 +440,24 @@ public class FileUtils {
                 checkpoint = 0;
             }
         }
+        return progress;
     }
 
     @VisibleForTesting
-    public static void copyInternalUserspace(FileDescriptor in, FileDescriptor out,
-            CopyListener listener, CancellationSignal signal) throws IOException {
-        copyInternalUserspace(new FileInputStream(in), new FileOutputStream(out), listener, signal);
+    public static long copyInternalUserspace(FileDescriptor in, FileDescriptor out,
+            ProgressListener listener, CancellationSignal signal, long count) throws IOException {
+        if (count != Long.MAX_VALUE) {
+            return copyInternalUserspace(new SizedInputStream(new FileInputStream(in), count),
+                    new FileOutputStream(out), listener, signal);
+        } else {
+            return copyInternalUserspace(new FileInputStream(in),
+                    new FileOutputStream(out), listener, signal);
+        }
     }
 
     @VisibleForTesting
-    public static void copyInternalUserspace(InputStream in, OutputStream out,
-            CopyListener listener, CancellationSignal signal) throws IOException {
+    public static long copyInternalUserspace(InputStream in, OutputStream out,
+            ProgressListener listener, CancellationSignal signal) throws IOException {
         long progress = 0;
         long checkpoint = 0;
         byte[] buffer = new byte[8192];
@@ -386,6 +479,7 @@ public class FileUtils {
                 checkpoint = 0;
             }
         }
+        return progress;
     }
 
     /**
@@ -997,7 +1091,7 @@ public class FileUtils {
                     }
                 }
             } catch (IOException | ErrnoException e) {
-                throw new RuntimeException(e);
+                // Ignored
             } finally {
                 if (sink) {
                     SystemClock.sleep(TimeUnit.SECONDS.toMillis(1));

diff --git a/core/java/android/print/PrintFileDocumentAdapter.java b/core/java/android/print/PrintFileDocumentAdapter.java
index 747400d..a5f9305 100644 (file)
--- a/core/java/android/print/PrintFileDocumentAdapter.java
+++ b/core/java/android/print/PrintFileDocumentAdapter.java
@@ -21,6 +21,8 @@ import android.os.AsyncTask;
 import android.os.Bundle;
 import android.os.CancellationSignal;
 import android.os.CancellationSignal.OnCancelListener;
+import android.os.FileUtils;
+import android.os.OperationCanceledException;
 import android.os.ParcelFileDescriptor;
 import android.util.Log;
 
@@ -114,28 +116,15 @@ public class PrintFileDocumentAdapter extends PrintDocumentAdapter {
 
         @Override
         protected Void doInBackground(Void... params) {
-            InputStream in = null;
-            OutputStream out = new FileOutputStream(mDestination.getFileDescriptor());
-            final byte[] buffer = new byte[8192];
-            try {
-                in = new FileInputStream(mFile);
-                while (true) {
-                    if (isCancelled()) {
-                        break;
-                    }
-                    final int readByteCount = in.read(buffer);
-                    if (readByteCount < 0) {
-                        break;
-                    }
-                    out.write(buffer, 0, readByteCount);
-                }
-             } catch (IOException ioe) {
-                 Log.e(LOG_TAG, "Error writing data!", ioe);
-                 mResultCallback.onWriteFailed(mContext.getString(
-                         R.string.write_fail_reason_cannot_write));
-             } finally {
-                IoUtils.closeQuietly(in);
-                IoUtils.closeQuietly(out);
+            try (InputStream in = new FileInputStream(mFile);
+                    OutputStream out = new FileOutputStream(mDestination.getFileDescriptor())) {
+                FileUtils.copy(in, out, null, mCancellationSignal);
+            } catch (OperationCanceledException e) {
+                // Ignored; already handled below
+            } catch (IOException e) {
+                Log.e(LOG_TAG, "Error writing data!", e);
+                mResultCallback.onWriteFailed(mContext.getString(
+                        R.string.write_fail_reason_cannot_write));
             }
             return null;
         }

diff --git a/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java b/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java
index 4f7c924..5989da7 100644 (file)
--- a/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java
+++ b/core/tests/benchmarks/src/android/os/FileUtilsBenchmark.java
@@ -54,7 +54,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (FileInputStream in = new FileInputStream(mSrc);
                     FileOutputStream out = new FileOutputStream(mDest)) {
-                copyInternalUserspace(in.getFD(), out.getFD(), null, null);
+                copyInternalUserspace(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }
@@ -63,7 +63,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (FileInputStream in = new FileInputStream(mSrc);
                     FileOutputStream out = new FileOutputStream(mDest)) {
-                copyInternalSendfile(in.getFD(), out.getFD(), null, null);
+                copyInternalSendfile(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }
@@ -72,7 +72,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (MemoryPipe in = MemoryPipe.createSource(mData);
                     FileOutputStream out = new FileOutputStream(mDest)) {
-                copyInternalUserspace(in.getFD(), out.getFD(), null, null);
+                copyInternalUserspace(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }
@@ -81,7 +81,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (MemoryPipe in = MemoryPipe.createSource(mData);
                     FileOutputStream out = new FileOutputStream(mDest)) {
-                copyInternalSplice(in.getFD(), out.getFD(), null, null);
+                copyInternalSplice(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }
@@ -90,7 +90,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (FileInputStream in = new FileInputStream(mSrc);
                     MemoryPipe out = MemoryPipe.createSink(mData)) {
-                copyInternalUserspace(in.getFD(), out.getFD(), null, null);
+                copyInternalUserspace(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }
@@ -99,7 +99,7 @@ public class FileUtilsBenchmark {
         for (int i = 0; i < reps; i++) {
             try (FileInputStream in = new FileInputStream(mSrc);
                     MemoryPipe out = MemoryPipe.createSink(mData)) {
-                copyInternalSplice(in.getFD(), out.getFD(), null, null);
+                copyInternalSplice(in.getFD(), out.getFD(), null, null, Long.MAX_VALUE);
             }
         }
     }

diff --git a/core/tests/coretests/src/android/os/FileUtilsTest.java b/core/tests/coretests/src/android/os/FileUtilsTest.java
index b7220b3..0bc3a2d 100644 (file)
--- a/core/tests/coretests/src/android/os/FileUtilsTest.java
+++ b/core/tests/coretests/src/android/os/FileUtilsTest.java
@@ -181,6 +181,27 @@ public class FileUtilsTest {
     }
 
     @Test
+    public void testCopy_ShortPipeToFile() throws Exception {
+        byte[] source = new byte[33_000_000];
+        new Random().nextBytes(source);
+
+        for (int size : DATA_SIZES) {
+            final File dest = new File(mTarget, "dest");
+
+            byte[] expected = Arrays.copyOf(source, size);
+            byte[] actual = new byte[size];
+
+            try (MemoryPipe in = MemoryPipe.createSource(source);
+                    FileOutputStream out = new FileOutputStream(dest)) {
+                FileUtils.copy(in.getFD(), out.getFD(), null, null, size);
+            }
+
+            actual = readFile(dest);
+            assertArrayEquals(expected, actual);
+        }
+    }
+
+    @Test
     public void testIsFilenameSafe() throws Exception {
         assertTrue(FileUtils.isFilenameSafe(new File("foobar")));
         assertTrue(FileUtils.isFilenameSafe(new File("a_b-c=d.e/0,1+23")));