[Android Crash Reporting] Allow uploading minidumps via the JobScheduler

components/minidump_uploader/android/java/src/org/chromium/components/minidump_uploader/CrashFileManager.java

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.components.minidump_uploader;
 
 import android.support.annotation.Nullable;
 
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
@@ skipping 31 matching lines @@
     public static final String CRASH_DUMP_DIR = "Crash Reports";
 
     // This should mirror the C++ CrashUploadList::kReporterLogFilename variable.
     @VisibleForTesting
     public static final String CRASH_DUMP_LOGFILE = "uploads.log";
 
     private static final Pattern MINIDUMP_MIME_FIRST_TRY_PATTERN =
             Pattern.compile("\\.dmp([0-9]+)$\\z");
 
     private static final Pattern MINIDUMP_PATTERN =
-            Pattern.compile("\\.dmp([0-9]*)(\\.try([0-9]+))?\\z");
+            Pattern.compile("\\.(dmp|forced)([0-9]*)(\\.try([0-9]+))?\\z");
 
+    // TODO(isherman): Is this pattern incorrect? Typical uploaded filenames include a ".tryN"
+    // suffix...

[Ilya Sherman, 2017/03/14 02:22:03]

By the way, Gustav, I noticed that uploaded files didn't seem to be being
deleted by the cleanup code... and found what I think might be a bug: The
pattern for uploaded files doesn't match the actual uploaded filenames.  I'm
guessing this is probably pretty important to fix for Webview for M58, yeah?

[gsennton, 2017/03/14 18:17:28]

Huh, that would indeed be important to fix for 58 :) (and to get it into some
part of the Beta).
Could you please file a bug?
I can try to take a look tomorrow (but after that I'll be OOO for the rest of
the week :/).

[Ilya Sherman, 2017/03/15 02:13:35]

One better: I sent you a CL =) https://codereview.chromium.org/2751903002/

[gsennton, 2017/03/15 12:57:43]

Awesome, thanks a lot! :)
(you can remove this comment from the latest PS now :P)

[Ilya Sherman, 2017/03/16 01:36:57]

Done.

     private static final Pattern UPLOADED_MINIDUMP_PATTERN = Pattern.compile("\\.up([0-9]*)\\z");
 
     private static final String NOT_YET_UPLOADED_MINIDUMP_SUFFIX = ".dmp";
 
     private static final String UPLOADED_MINIDUMP_SUFFIX = ".up";
 
     private static final String UPLOAD_SKIPPED_MINIDUMP_SUFFIX = ".skipped";
 
     private static final String UPLOAD_FORCED_MINIDUMP_SUFFIX = ".forced";
 
@@ skipping 66 matching lines @@
     public static String tryIncrementAttemptNumber(File mFileToUpload) {
         String newName = filenameWithIncrementedAttemptNumber(mFileToUpload.getPath());
         return mFileToUpload.renameTo(new File(newName)) ? newName : null;
     }
 
     /**
      * @return The file name to rename to after an addition attempt to upload
      */
     @VisibleForTesting
     public static String filenameWithIncrementedAttemptNumber(String filename) {
-        int numTried = readAttemptNumber(filename);
+        int numTried = readAttemptNumberInternal(filename);
         if (numTried >= 0) {
             int newCount = numTried + 1;
             return filename.replace(
                     UPLOAD_ATTEMPT_DELIMITER + numTried, UPLOAD_ATTEMPT_DELIMITER + newCount);
         } else {
-            // readAttemptNumber returning -1 means there is no UPLOAD_ATTEMPT_DELIMITER in the file
-            // name (or that there is a delimiter but no attempt number). So, we have to add the
-            // delimiter and attempt number ourselves.
+            // readAttemptNumberInternal returning -1 means there is no UPLOAD_ATTEMPT_DELIMITER in
+            // the file name (or that there is a delimiter but no attempt number). So, we have to
+            // add the delimiter and attempt number ourselves.
             return filename + UPLOAD_ATTEMPT_DELIMITER + "1";
         }
     }
 
     /**
      * Attempts to rename the given file to mark it as a forced upload. This is useful for allowing
      * users to manually initiate previously skipped uploads.
      *
      * @return The renamed file, or null if renaming failed.
      */
@@ skipping 25 matching lines @@
             filename = filename.replace(
                     UPLOAD_ATTEMPT_DELIMITER + numTried, UPLOAD_ATTEMPT_DELIMITER + 0);
         }
         filename = filename.replace(UPLOAD_SKIPPED_MINIDUMP_SUFFIX, UPLOAD_FORCED_MINIDUMP_SUFFIX);
         return filename.replace(NOT_YET_UPLOADED_MINIDUMP_SUFFIX, UPLOAD_FORCED_MINIDUMP_SUFFIX);
     }
 
     /**
      * Returns how many times we've tried to upload a certain Minidump file.
      * @return the number of attempts to upload the given Minidump file, parsed from its file name,
      *     returns -1 if an attempt-number cannot be parsed from the file-name.

[gsennton, 2017/03/14 18:17:28]

Update comment please ;)

[Ilya Sherman, 2017/03/15 02:13:35]

Done.

      */
     @VisibleForTesting
     public static int readAttemptNumber(String filename) {
+        int numTries = readAttemptNumberInternal(filename);
+        return numTries >= 0 ? numTries : 0;
+    }
+
+    /**
+     * Returns how many times we've tried to upload a certain Minidump file.
+     * @return the number of attempts to upload the given Minidump file, parsed from its file name,
+     *     returns -1 if an attempt-number cannot be parsed from the file-name.
+     */
+    @VisibleForTesting
+    public static int readAttemptNumberInternal(String filename) {

[Maria, 2017/03/14 19:03:32]

can this be package-visible instead?

[Ilya Sherman, 2017/03/15 02:13:35]

Done.

         int tryIndex = filename.lastIndexOf(UPLOAD_ATTEMPT_DELIMITER);
         if (tryIndex >= 0) {
             tryIndex += UPLOAD_ATTEMPT_DELIMITER.length();
             String numTriesString = filename.substring(tryIndex);
             Scanner numTriesScanner = new Scanner(numTriesString).useDelimiter("[^0-9]+");
             try {
                 int nextInt = numTriesScanner.nextInt();
                 // Only return the number if it occurs just after the UPLOAD_ATTEMPT_DELIMITER.
                 return numTriesString.indexOf(Integer.toString(nextInt)) == 0 ? nextInt : -1;
             } catch (NoSuchElementException e) {
@@ skipping 358 matching lines @@
     /**
      * Create a temporary file to store a minidump in before renaming it with a real minidump name.
      * @return a new temporary file with prefix {@param prefix} stored in the directory
      * {@param directory}.
      *
      */
     private static File createMinidumpTmpFile(File directory) throws IOException {
         return File.createTempFile("webview_minidump", TMP_SUFFIX, directory);
     }
 }