Record more download resumption UMAs to analyze the how download is resumed

chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java

Index: chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
index e80e93c9f2d958e2563c402045f2124557da8b95..0e33f709a0bb340e1c9e6a0104fdc5650ab583c8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadManagerService.java
@@ -79,6 +79,8 @@ public class DownloadManagerService extends BroadcastReceiver implements
     private static final String UNKNOWN_MIME_TYPE = "application/unknown";
     private static final String DOWNLOAD_UMA_ENTRY = "DownloadUmaEntry";
     private static final String DOWNLOAD_RETRY_COUNT_FILE_NAME = "DownloadRetryCount";
+    private static final String DOWNLOAD_MANUAL_RETRY_SUFFIX = ".Manual";
+    private static final String DOWNLOAD_TOTAL_RETRY_SUFFIX = ".Total";
     private static final long UPDATE_DELAY_MILLIS = 1000;
     // Wait 10 seconds to resume all downloads, so that we won't impact tab loading.
     private static final long RESUME_DELAY_MILLIS = 10000;
@@ -783,7 +785,8 @@ public class DownloadManagerService extends BroadcastReceiver implements
             case DOWNLOAD_STATUS_CANCELLED:
                 recordDownloadFinishedUMA(downloadStatus, downloadItem.getId(),
                         downloadItem.getDownloadInfo().getBytesReceived());
-                clearDownloadRetryCount(downloadItem.getId());
+                clearDownloadRetryCount(downloadItem.getId(), true);
+                clearDownloadRetryCount(downloadItem.getId(), false);
                 break;
             case DOWNLOAD_STATUS_INTERRUPTED:
                 entry = getUmaStatsEntry(downloadItem.getId());
@@ -1196,14 +1199,17 @@ public class DownloadManagerService extends BroadcastReceiver implements
             if (!progress.mCanDownloadWhileMetered) {
                 progress.mCanDownloadWhileMetered = isActiveNetworkMetered(mContext);
             }
-            clearDownloadRetryCount(item.getId());
+            incrementDownloadRetryCount(item.getId(), true);
+            clearDownloadRetryCount(item.getId(), true);
         } else {
-            int count = incrementAndReturnDownloadRetryCount(item.getId());
-            if (count > getAutoResumptionLimit()) {
+            SharedPreferences sharedPrefs = getAutoRetryCountSharedPreference(mContext);
+            int count = sharedPrefs.getInt(item.getId(), 0);
+            if (count >= getAutoResumptionLimit()) {
                 removeAutoResumableDownload(item.getId());
                 onDownloadInterrupted(item.getDownloadInfo(), false);
                 return;
             }
+            incrementDownloadRetryCount(item.getId(), false);
         }
         nativeResumeDownload(getNativeDownloadManagerService(), item.getId(),
                 item.getDownloadInfo().isOffTheRecord());
@@ -1771,29 +1777,69 @@ public class DownloadManagerService extends BroadcastReceiver implements
 
     /**
      * Increments the interruption count for a download. If the interruption count reaches a certain
-     * threshold, the download will no longer auto resume unless user click the resume button
-     * to clear the count.
+     * threshold, the download will no longer auto resume unless user click the resume button to
+     * clear the count.
+     *
      * @param downloadGuid Download GUID.
-     * @return The interruption count of the download, after incrementation.
+     * @param hasUserGesture Whether the retry is caused by user gesture.
      */
-    private int incrementAndReturnDownloadRetryCount(String downloadGuid) {
+    private void incrementDownloadRetryCount(String downloadGuid, boolean hasUserGesture) {
+        String name = getDownloadRetryCountSharedPrefName(downloadGuid, hasUserGesture, false);
+        incrementDownloadRetrySharedPreferenceCount(name);
+        name = getDownloadRetryCountSharedPrefName(downloadGuid, hasUserGesture, true);
+        incrementDownloadRetrySharedPreferenceCount(name);
+    }
+
+    /**
+     * Helper method to increment the retry count for a SharedPreference entry.
+     * @param sharedPreferenceName Name of the SharedPreference entry.
+     */
+    private void incrementDownloadRetrySharedPreferenceCount(String sharedPreferenceName) {
         SharedPreferences sharedPrefs = getAutoRetryCountSharedPreference(mContext);
-        int count = sharedPrefs.getInt(downloadGuid, 0);
-        count++;
+        int count = sharedPrefs.getInt(sharedPreferenceName, 0);
         SharedPreferences.Editor editor = sharedPrefs.edit();
-        editor.putInt(downloadGuid, count);
+        count++;
+        editor.putInt(sharedPreferenceName, count);
         editor.apply();
-        return count;
     }
 
     /**
-     * clears the interruption count for a download.
+     * Helper method to retrieve the SharedPreference name for different download retry types.
+     * @param downloadGuid Guid of the download.
+     * @param hasUserGesture Whether the SharedPreference is for manual retry attempts.
+     * @param isTotalCount Whether the SharedPreference is for total retry attempts.
+     */
+    private String getDownloadRetryCountSharedPrefName(
+            String downloadGuid, boolean hasUserGesture, boolean isTotalCount) {
+        if (isTotalCount) return downloadGuid + DOWNLOAD_TOTAL_RETRY_SUFFIX;
+        if (hasUserGesture) return downloadGuid + DOWNLOAD_MANUAL_RETRY_SUFFIX;
+        return downloadGuid;
+    }
+
+    /**
+     * clears the retry count for a download.
+     *
      * @param downloadGuid Download GUID.
+     * @param isAutoRetryOnly Whether to clear the auto retry count only.
      */
-    private void clearDownloadRetryCount(String downloadGuid) {
+    private void clearDownloadRetryCount(String downloadGuid, boolean isAutoRetryOnly) {
         SharedPreferences sharedPrefs = getAutoRetryCountSharedPreference(mContext);
+        String name = getDownloadRetryCountSharedPrefName(downloadGuid, !isAutoRetryOnly, false);
+        int count = sharedPrefs.getInt(name, 0);
         SharedPreferences.Editor editor = sharedPrefs.edit();
-        editor.remove(downloadGuid);
+        editor.remove(name);
+        if (isAutoRetryOnly) {
+            RecordHistogram.recordCustomCountHistogram(
+                    "MobileDownload.ResumptionsCount.Automatic", count, 1, 10, 10);

[Ilya Sherman, 2017/04/17 22:18:43]

nit: Mebbe use sparse histograms, with some manual clamping as a sanity-check,
rather than preallocating buckets?  This should give you a great tradeoff
between client-side memory usage and server-side data granularity.

[qinmin, 2017/04/17 23:41:53]

Done.

+        } else {
+            RecordHistogram.recordCustomCountHistogram(
+                    "MobileDownload.ResumptionsCount.Manual", count, 1, 200, 20);
+            name = getDownloadRetryCountSharedPrefName(downloadGuid, false, true);
+            count = sharedPrefs.getInt(name, 0);
+            RecordHistogram.recordCustomCountHistogram(
+                    "MobileDownload.ResumptionsCount.Total", count, 1, 200, 20);

[Ilya Sherman, 2017/04/17 22:18:43]

(For these as well)

[qinmin, 2017/04/17 23:41:53]

Done.

+            editor.remove(name);
+        }
         editor.apply();
     }