Componentize parts of minidump uploading for use from WebView.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/crash/CrashFileManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/crash/CrashFileManager.java b/chrome/android/java/src/org/chromium/chrome/browser/crash/CrashFileManager.java
index 55a8956cce099c31499f3e32aa461fac225b62c7..3a6311816c4ad28b9ac4fcf040bf077b01b88f85 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/crash/CrashFileManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/crash/CrashFileManager.java
@@ -34,8 +34,7 @@ public class CrashFileManager {
     @VisibleForTesting
     static final String CRASH_DUMP_LOGFILE = "uploads.log";
 
-    private static final Pattern MINIDUMP_FIRST_TRY_PATTERN =
-            Pattern.compile("\\.dmp([0-9]*)$\\z");
+    private static final Pattern MINIDUMP_FIRST_TRY_PATTERN = Pattern.compile("\\.dmp([0-9]*)$\\z");
 
     private static final Pattern MINIDUMP_MIME_FIRST_TRY_PATTERN =
             Pattern.compile("\\.dmp([0-9]+)$\\z");
@@ -76,7 +75,7 @@ public class CrashFileManager {
      * @return Comparator for prioritizing the more recently modified file
      */
     @VisibleForTesting
-    protected static final Comparator<File> sFileComparator =  new Comparator<File>() {
+    protected static final Comparator<File> sFileComparator = new Comparator<File>() {
         @Override
         public int compare(File lhs, File rhs) {
             if (lhs.lastModified() == rhs.lastModified()) {
@@ -172,8 +171,7 @@ public class CrashFileManager {
             // To avoid out of bound exceptions
             if (tryIndex < filename.length()) {
                 // We don't try more than 3 times.
-                String numTriesString = filename.substring(
-                        tryIndex, tryIndex + 1);
+                String numTriesString = filename.substring(tryIndex, tryIndex + 1);
                 try {
                     return Integer.parseInt(numTriesString);
                 } catch (NumberFormatException ignored) {
@@ -226,13 +224,17 @@ public class CrashFileManager {
     }
 
     private final File mCacheDir;
+    /**
+     * The number of times we will try to upload a crash.
+     */
+    @VisibleForTesting
+    public static final int MAX_TRIES_ALLOWED = 3;

[Ilya Sherman, 2016/10/07 07:08:39]

This concept seems more appropriate for the minidump upload code.  What's the
reason for moving it to the crash file manager?

Also, if you do move it here, please move it to the top of the file, with the
other constants.

[gsennton, 2016/10/07 09:58:13]

Alright, so I chatted a bit with Toby (tobiasjs@) about this and we think it
makes more sense to not use the MAX_TRIES_ALLOWED in this file at all. The
reason I moved it here because we use it in one place (getMatchingFiles) to
ensure files that have been retried too many times aren't returned in minidump
queries.

We think it makes more sense to - instead of incrementing the tries for a file
that we have tried to upload MAX_TRIES_ALLOWED number of times - rename the file
so that it won't be included in the queries for files to upload. In this way the
CrashFileManager doesn't need to know MAX_TRIES_ALLOWED, and any retry limit can
instead be handled by the classes using CrashFileManager (in a way
CrashFileManager shouldn't depend on the rest of the services in this folder
IMO, so removing the dependency on MinidumpUploadService.MAX_TRIES_ALLOWED is
good for the overall code health here :)).

 
     public CrashFileManager(File cacheDir) {
         if (cacheDir == null) {
             throw new NullPointerException("Specified context cannot be null.");
         } else if (!cacheDir.isDirectory()) {
-            throw new IllegalArgumentException(cacheDir.getAbsolutePath()
-                    + " is not a directory.");
+            throw new IllegalArgumentException(cacheDir.getAbsolutePath() + " is not a directory.");
         }
         mCacheDir = cacheDir;
     }
@@ -309,7 +311,7 @@ public class CrashFileManager {
             public boolean accept(File dir, String filename) {
                 Matcher match = pattern.matcher(filename);
                 int tries = readAttemptNumber(filename);
-                return match.find() && tries < MinidumpUploadService.MAX_TRIES_ALLOWED;
+                return match.find() && tries < MAX_TRIES_ALLOWED;
             }
         });
         return minidumps;
@@ -345,8 +347,7 @@ public class CrashFileManager {
             if (f.delete()) {
                 f = new File(getCrashDirectory(), name);
             } else {
-                Log.w(TAG, "Unable to delete previous logfile"
-                        + f.getAbsolutePath());
+                Log.w(TAG, "Unable to delete previous logfile" + f.getAbsolutePath());
             }
         }
         return f;