Enabling sampling of UMA and crash reports on Android.

chrome/browser/android/metrics/uma_session_stats.cc

Index: chrome/browser/android/metrics/uma_session_stats.cc
diff --git a/chrome/browser/android/metrics/uma_session_stats.cc b/chrome/browser/android/metrics/uma_session_stats.cc
index e209d4fddba7e04dd1df6da2e6c62fa15ad0fb12..e26a54c499bb45b1ef25e8a27022697a96df64b0 100644
--- a/chrome/browser/android/metrics/uma_session_stats.cc
+++ b/chrome/browser/android/metrics/uma_session_stats.cc
@@ -100,27 +100,28 @@ void UmaSessionStats::RegisterSyntheticMultiGroupFieldTrial(
 //   This happens when we've got permission to upload on Wi-Fi but we're on a
 //   mobile connection (for example).
 // * Logs are neither being recorded or uploaded.

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

Could you please document |consent_change| as well?

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

This list sounds like there ought to be an assertion: DCHECK(may_record ||
!may_upload);

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

Done.

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

Re-worked some of the code to avoid that parameter.

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

Now may_record is also unused.  Is it still needed here?  Is it important that
the code in ChangeMetricsReportingConsent() happens before the code in
UpdateMetricsServiceState()?  If so, is there something that guarantees that
ordering?  It would be good to document, as well, if there is an ordering
constraint.

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

Well, it's needed for the DCHECK ;P. Note, there is a deeper place in
MetricsServicesManager to move that DCHECK, but I'm not going to because it
violates how we use C++ side api.

There is an ordering constraint, if ChangeMetricsReportingConsent() gets called,
UpdateMetricsServiceState() should be called after it to affect the metrics
services. That's currently enforced on the Java side by
changeMetricsReportingConsent() calling updateMetricsServiceState() at the end.
I will add something to the comment for Change...().

-static void UpdateMetricsServiceState(JNIEnv* env,
-                                      const JavaParamRef<jobject>& obj,
+static void UpdateMetricsServiceState(JNIEnv*,
+                                      const JavaParamRef<jclass>&,
+                                      jboolean consent_change,
                                       jboolean may_record,
                                       jboolean may_upload) {
-  metrics::MetricsService* metrics = g_browser_process->metrics_service();
-  DCHECK(metrics);
-
-  if (metrics->recording_active() != may_record) {
+  if (consent_change) {
     UpdateMetricsPrefsOnPermissionChange(may_record);
 
-    // This function puts a consent file with the ClientID in the
-    // data directory. The ID is passed to the renderer for crash
-    // reporting when things go wrong.
+    // This function ensures a consent file in the data directory is either
+    // created, or deleted, depending on may_record. Starting up metrics
+    // services will ensure that the consent file contains the ClientID. The ID
+    // is passed to the renderer for crash reporting when things go wrong.
     content::BrowserThread::GetBlockingPool()->PostTask(FROM_HERE,
         base::Bind(
             base::IgnoreResult(GoogleUpdateSettings::SetCollectStatsConsent),
             may_record));
   }
 
-  g_browser_process->GetMetricsServicesManager()->UpdatePermissions(
-      may_record, may_upload);
+  // This will also apply the consent state, taken from Chrome Local State
+  // prefs.
+  g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(
+      may_upload);
 }
 
 // Renderer process crashed in the foreground.