[Android Crash Reporting] Simplify crash report upload code.

chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java

Index: chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpPreparationCallable.java b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java
similarity index 32%
rename from chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpPreparationCallable.java
rename to chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java
index 4529ff255857e533718877aafa692fa3918b6c26..57456b2729b00ea2c9829e9369106a67961823b8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpPreparationCallable.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpLogcatPrepender.java
@@ -4,10 +4,8 @@
 
 package org.chromium.chrome.browser.crash;
 
-import android.content.Context;
-import android.content.Intent;
-
 import org.chromium.base.Log;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.components.minidump_uploader.CrashFileManager;
 
 import java.io.BufferedInputStream;
@@ -20,85 +18,78 @@ import java.io.FileOutputStream;
 import java.io.FileReader;
 import java.io.FileWriter;
 import java.io.IOException;
-import java.util.Collections;
-import java.util.LinkedList;
 import java.util.List;
-import java.util.concurrent.Callable;
 
 /**
- * Callable helper for {@link MinidumpPreparationService}.
- *
- * This class will append a logcat file to a minidump file for upload.
+ * Prepends a logcat file to a minidump file for upload.
  */
-public class MinidumpPreparationCallable implements Callable<Boolean> {
-    private static final String TAG = "DumpPrepCallable";
+public class MinidumpLogcatPrepender {
+    private static final String TAG = "LogcatPrepender";
 
-    private static final String LOGCAT_CONTENT_DISPOSITION =
+    @VisibleForTesting
+    static final String LOGCAT_CONTENT_DISPOSITION =
             "Content-Disposition: form-data; name=\"logcat\"; filename=\"logcat\"";
 
-    private static final String LOGCAT_CONTENT_TYPE = "Content-Type: text/plain";
+    @VisibleForTesting
+    static final String LOGCAT_CONTENT_TYPE = "Content-Type: text/plain";
 
-    private final File mLogcatFile;
-    private final File mMinidumpFile;
-    private final Intent mRedirectIntent;
-    private final Context mContext;
     private final CrashFileManager mFileManager;
+    private final File mMinidumpFile;
+    private final List<String> mLogcat;
 
-    public MinidumpPreparationCallable(
-            Context context, File miniDumpFile, File logcatFile, Intent redirectIntent) {
-        mContext = context;
-        mLogcatFile = logcatFile;
-        mMinidumpFile = miniDumpFile;
-        mRedirectIntent = redirectIntent;
-        mFileManager = new CrashFileManager(context.getCacheDir());
+    public MinidumpLogcatPrepender(
+            CrashFileManager fileManager, File minidumpFile, List<String> logcat) {
+        mFileManager = fileManager;
+        mMinidumpFile = minidumpFile;
+        mLogcat = logcat;
     }
 
     /**
-     * Read the boundary from the first lien of the file.
+     * Read the boundary from the first line of the file.
      */
-    private static String getBoundary(File processMinidumpFile) throws IOException {
-        BufferedReader bReader = null;
+    private static String getBoundary(File minidumpFile) throws IOException {
+        BufferedReader reader = null;
         try {
-            bReader = new BufferedReader(new FileReader(processMinidumpFile));
-            return bReader.readLine();
+            reader = new BufferedReader(new FileReader(minidumpFile));
+            return reader.readLine();
         } finally {
-            if (bReader != null) {
-                bReader.close();
+            if (reader != null) {
+                reader.close();
             }
         }
     }
 
     /**
-     * Write the invoking {@link MinidumpPreparationCallable}s logcat data to
-     * the specified target {@link File}.
+     * Write the logcat data to the specified target {@link File}.
      *
      * Target file is overwritten, not appended to the end.
      *
      * @param targetFile File to which logcat data should be written.
+     * @param logcat The lines of the logcat output.
      * @param boundary String MIME boundary to prepend.
      * @throws IOException if something goes wrong.
      */
     private static void writeLogcat(File targetFile, List<String> logcat, String boundary)
             throws IOException {
-        BufferedWriter bWriter = null;
+        BufferedWriter writer = null;
         try {
-            bWriter = new BufferedWriter(new FileWriter(targetFile, false));
-            bWriter.write(boundary);
-            bWriter.newLine();
+            writer = new BufferedWriter(new FileWriter(targetFile, false));
+            writer.write(boundary);
+            writer.newLine();
             // Next we write the logcat data in a MIME block.
-            bWriter.write(LOGCAT_CONTENT_DISPOSITION);
-            bWriter.newLine();
-            bWriter.write(LOGCAT_CONTENT_TYPE);
-            bWriter.newLine();
-            bWriter.newLine();
+            writer.write(LOGCAT_CONTENT_DISPOSITION);
+            writer.newLine();
+            writer.write(LOGCAT_CONTENT_TYPE);
+            writer.newLine();
+            writer.newLine();
             // Emits the contents of the buffer into the output file.
             for (String ln : logcat) {
-                bWriter.write(ln);
-                bWriter.newLine();
+                writer.write(ln);
+                writer.newLine();
             }
         } finally {
-            if (bWriter != null) {
-                bWriter.close();
+            if (writer != null) {
+                writer.close();
             }
         }
     }
@@ -106,92 +97,69 @@ public class MinidumpPreparationCallable implements Callable<Boolean> {
     /**
      * Append the minidump file data to the specified target {@link File}.
      *
-     * @param processMinidumpFile File containing data to append.
+     * @param minidumpFile File containing data to append.
      * @param targetFile File to which data should be appended.
      * @throws IOException when standard IO errors occur.
      */
-    private static void appendMinidump(File processMinidumpFile, File targetFile)
-            throws IOException {
-        BufferedInputStream bIn = null;
-        BufferedOutputStream bOut = null;
+    private static void appendMinidump(File minidumpFile, File targetFile) throws IOException {
+        BufferedInputStream in = null;
+        BufferedOutputStream out = null;
         try {
             byte[] buf = new byte[256];
-            bIn = new BufferedInputStream(new FileInputStream(processMinidumpFile));
-            bOut = new BufferedOutputStream(new FileOutputStream(targetFile, true));
+            in = new BufferedInputStream(new FileInputStream(minidumpFile));
+            out = new BufferedOutputStream(new FileOutputStream(targetFile, true));
             int count;
-            while ((count = bIn.read(buf)) != -1) {
-                bOut.write(buf, 0, count);
+            while ((count = in.read(buf)) != -1) {
+                out.write(buf, 0, count);
             }
         } finally {
-            if (bIn != null) bIn.close();
-            if (bOut != null) bOut.close();
+            if (in != null) in.close();
+            if (out != null) out.close();
         }
     }
 
-    private boolean augmentTargetFile(List<String> logcat) {
+    /**
+     * Prepends the logcat output to the minidump file.
+     * @return On success, returns the file containing the combined logcat and minidump output.
+     *     On failure, returns the original file containing just the minidump.
+     */
+    public File run() {
+        if (mLogcat.isEmpty()) return mMinidumpFile;
+
+        String targetFileName = mMinidumpFile.getName() + ".try0";
         File targetFile = null;
+        boolean success = false;
         try {
-            targetFile = mFileManager.createNewTempFile(mMinidumpFile.getName() + ".try0");
-
             String boundary = getBoundary(mMinidumpFile);
             if (boundary == null) {
-                return false;
+                return mMinidumpFile;
             }
-            writeLogcat(targetFile, logcat, boundary);
-            // Finally Reopen and append the original minidump MIME sections
-            // including the leading boundary.
+
+            targetFile = mFileManager.createNewTempFile(targetFileName);
+            writeLogcat(targetFile, mLogcat, boundary);
+
+            // Finally reopen and append the original minidump MIME sections, including the leading
+            // boundary.
             appendMinidump(mMinidumpFile, targetFile);
-            if (!mMinidumpFile.delete()) {
-                Log.w(TAG, "Fail to delete minidump file: " + mMinidumpFile.getName());
-            }
-            return true;
+            success = true;
         } catch (IOException e) {
-            String msg = String.format(
-                    "Error while tyring to annotate minidump file %s with logcat data",
-                    mMinidumpFile.getAbsoluteFile());
-            Log.w(TAG, msg, e);
+            Log.w(TAG, "Error while trying to annotate minidump file %s with logcat data",
+                    mMinidumpFile.getAbsoluteFile(), e);
             if (targetFile != null) {
                 CrashFileManager.deleteFile(targetFile);
             }
-            return false;
         }
-    }
 
-    private List<String> getLogcatAsList() throws IOException {
-        BufferedReader r = null;
-        try {
-            List<String> logcat = new LinkedList<String>();
-            if (mLogcatFile != null) {
-                r = new BufferedReader(new FileReader(mLogcatFile));
-                String ln;
-                while ((ln = r.readLine()) != null) {
-                    logcat.add(ln);
-                }
-            }
-            return Collections.unmodifiableList(logcat);
-        } finally {
-            if (r != null) {
-                r.close();
-            }
-        }
-    }
+        if (!success) return mMinidumpFile;
 
-    @Override
-    public Boolean call() throws IOException {
-        // By default set the basic minidump to be uploaded. That way, even if
-        // there are errors augmenting the minidump with logcat data, the service
-        // can still upload the unaugmented minidump.
-        List<String> logcat = getLogcatAsList();
-        boolean success = true;
-        if (!logcat.isEmpty()) {
-            success = augmentTargetFile(logcat);
-            if (success && !mLogcatFile.delete()) {
-                Log.w(TAG, "Failed to delete logcat file: " + mLogcatFile.getName());
-            }
+        // Try to clean up the previous file. Note that this step is best-effort, and failing to
+        // perform the cleanup does not count as an overall failure to prepend the logcat output.
+        if (!mMinidumpFile.delete()) {
+            Log.w(TAG, "Failed to delete minidump file: " + mMinidumpFile.getName());
         }
-        if (mRedirectIntent != null) {
-            mContext.startService(mRedirectIntent);
-        }
-        return success;
+
+        assert targetFile != null;
+        assert targetFile.exists();
+        return targetFile;
     }
 }