Enabling sampling of UMA and crash reports on Android.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadCallable.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadCallable.java b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadCallable.java
index d21a02a6553a98b47f7835c725a31586d9081a56..89433ad5b47a414b96e9dadc33475bd0beedb577 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadCallable.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/crash/MinidumpUploadCallable.java
@@ -56,13 +56,15 @@ public class MinidumpUploadCallable implements Callable<Integer> {
         UPLOAD_SUCCESS,
         UPLOAD_FAILURE,
         UPLOAD_USER_DISABLED,
-        UPLOAD_COMMANDLINE_DISABLED
+        UPLOAD_COMMANDLINE_DISABLED,
+        UPLOAD_DISABLED_BY_SAMPLING
     })
     public @interface MinidumpUploadStatus {}
     public static final int UPLOAD_SUCCESS = 0;
     public static final int UPLOAD_FAILURE = 1;
     public static final int UPLOAD_USER_DISABLED = 2;
     public static final int UPLOAD_COMMANDLINE_DISABLED = 3;
+    public static final int UPLOAD_DISABLED_BY_SAMPLING = 4;
 
     private final File mFileToUpload;
     private final File mLogfile;
@@ -107,6 +109,11 @@ public class MinidumpUploadCallable implements Callable<Integer> {
                 Log.i(TAG, "Minidump cannot currently be uploaded due to constraints.");
                 return UPLOAD_FAILURE;
             }
+
+            if (!mPermManager.isClientInMetricsSample()) {
+                Log.i(TAG, "Minidump cannot currently be uploaded due to sampling.");

[Ilya Sherman, 2016/08/23 21:32:17]

nit: "currently" seems somewhat misleading -- or do clients flipflop in and out
of the sample?

[jwd, 2016/08/23 23:00:58]

It's not ideal to have it change, but it's not strictly permanent. I'm fine
changing it though. It's certainly not as meaningful as it is for the upload
limited version.

+                return UPLOAD_DISABLED_BY_SAMPLING;

[Ilya Sherman, 2016/08/23 21:32:17]

What code handles this return value?  Will the system just keep retrying this
upload indefinitely?

[gayane -on leave until 09-2017, 2016/08/23 22:43:51]

Ok, looking at this a little bit more, I see that for example in case of
UPLOAD_USER_DISABLED they call cleanupMinidumpFile() so that it doesn't get
retried.

[jwd, 2016/08/23 23:00:58]

Hm, yes, I believe it will...
I can make not do that by renaming or deleting the file.
Maybe for now, deleting it is the correct thing to do, since we don't have the
manual upload functionality, and it's non trivial to get these minidumps off the
filesystem. The simplest thing would be to treat it the same as a failure, but
that would still try to re-upload a few times, so it's not so good. Thoughts?

[Ilya Sherman, 2016/08/24 05:02:50]

I think Gayane's suggestion, i.e. call cleanupMinidumpFile(), makes some sense,
since it parallels the case where the upload is not permitted by the user.

It might be even better to somehow distinguish files that were not uploaded
because they were not allowed by the user vs. because they were sampled out, but
that's probably best to save for a follow-up CL.

[jwd, 2016/08/24 15:06:27]

Done.

+            }

[Ilya Sherman, 2016/08/23 21:32:17]

Optional: This kind of feels like it belongs before the "isLimited" check.  I
don't think the ultimate logic actually changes; but it just seems,
conceptually, like it should be checked earlier.

[jwd, 2016/08/23 23:00:58]

Done.

         }
 
         HttpURLConnection connection =