New UMA settings fragment for Chrome on Android.

chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
index d5b5a2258f6f8a9dfec5497c09f3a4bb2ab48bd3..46e40f8d4474f7c9aa837ae6940d31e6a4bfd9d2 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/preferences/privacy/PrivacyPreferencesManager.java
@@ -11,6 +11,7 @@ import android.net.NetworkInfo;
 import android.preference.PreferenceManager;
 
 import org.chromium.base.CommandLine;
+import org.chromium.base.FieldTrialList;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.ChromeSwitches;
 import org.chromium.chrome.R;
@@ -41,6 +42,8 @@ public class PrivacyPreferencesManager implements CrashReportingPermissionManage
     private final String mCrashDumpWifiOnlyUpload;
     private final String mCrashDumpAlwaysUpload;
 
+    private Boolean mIsCellularEnabledByExperiment;
+
     @VisibleForTesting
     PrivacyPreferencesManager(Context context) {
         mContext = context;
@@ -51,6 +54,13 @@ public class PrivacyPreferencesManager implements CrashReportingPermissionManage
         mCrashDumpAlwaysUpload = context.getString(R.string.crash_dump_always_upload_value);
     }
 
+    /*
+    * Sets whether cellular experiment is enabled or not. Used for testing.
+    */

[gone, 2015/03/09 19:53:28]

Add @VisibleForTesting, ForTesting

[gayane -on leave until 09-2017, 2015/03/09 23:07:26]

Done.

+    public void setCellularExperimentFortesting(boolean isCellularEnabledByExperiment) {
+        mIsCellularEnabledByExperiment = new Boolean(isCellularEnabledByExperiment);
+    }
+
     public static PrivacyPreferencesManager getInstance(Context context) {
         if (sInstance == null) {
             sInstance = new PrivacyPreferencesManager(context);
@@ -210,16 +220,68 @@ public class PrivacyPreferencesManager implements CrashReportingPermissionManage
      * @return boolean to whether to allow uploading crash dump.
      */
     private boolean allowUploadCrashDump() {
+        PrefServiceBridge prefServiceBridge = PrefServiceBridge.getInstance();
+
+        if (isCellularEnabledByExperiment() && prefServiceBridge.isSetMetricsReporting()) {
+            return prefServiceBridge.isMetricsReportingEnabled();
+        } else {

[Alexei Svitkine (slow), 2015/03/09 18:26:27]

If you have an early return, no need for else. Same below.

[gayane -on leave until 09-2017, 2015/03/09 23:07:26]

Done.

+            if (!isMobileNetworkCapable()) {
+                return mSharedPreferences.getBoolean(PREF_CRASH_DUMP_UPLOAD_NO_CELLULAR, false);

[gone, 2015/03/09 19:53:29]

why did you flip these from down in isNeverUploadCrashDump()?  making the
positive the top case means you don't have to ! the condition

[gayane -on leave until 09-2017, 2015/03/09 23:07:25]

I am not sure I understand you correctly. isNeverUploadCrashDump was not used
here and I have added only the first "if" in this function. 

Anyway, I flipped the check with isMobileNetworkCapable(). If this is correct
then i will do the same change in other functions as well.

[gone, 2015/03/09 23:11:00]

It wasn't used here, but it looks like the function body was copied over.  Yeah,
I was just asking you to flip the conditional.

[gayane -on leave until 09-2017, 2015/03/10 15:56:21]

Done.

+            } else {
+                String option =
+                        mSharedPreferences.getString(PREF_CRASH_DUMP_UPLOAD, mCrashDumpNeverUpload);
+                return option.equals(mCrashDumpAlwaysUpload)
+                        || (option.equals(mCrashDumpWifiOnlyUpload) && isWiFiOrEthernetNetwork());
+            }
+        }
+    }
+
+    /**
+    * Checks whether uploads are allowed by new preference or old preference
+    * based on the experiment assigned to the user. Also sets the new pref if it
+    * needs to be set.
+    *
+    * @return boolean to whether uploads are allowed at all or not.
+    **/

[gone, 2015/03/09 19:53:29]

nit: none of these javadocs should have a **/ on the last line.  Should be */
also indentation is off in the comments, everywhere.

[gayane -on leave until 09-2017, 2015/03/09 23:07:26]

Done.

+    public boolean isUsageAndCrashReportingEnabled() {
+        boolean isCellularEnabledByExperiment = isCellularEnabledByExperiment();
+        PrefServiceBridge prefServiceBridge = PrefServiceBridge.getInstance();
+
+        if (isCellularEnabledByExperiment && prefServiceBridge.isSetMetricsReporting()) {
+            return prefServiceBridge.isMetricsReportingEnabled();
+        } else {
+            boolean isEnabled = isUploadCrashDumpEnabled();
+            if (isCellularEnabledByExperiment && !prefServiceBridge.isSetMetricsReporting())
+                prefServiceBridge.setMetricsReportingEnabled(isEnabled);
+            return isEnabled;
+        }
+    }
+
+    /**
+    * Checks whether old Crash_dump_upload pref allows any (e.g. always on or
+    * only wifi) uploads or not.
+    *
+    * @return boolean to whether uploads are enabled by old pref.
+    **/
+    public boolean isUploadCrashDumpEnabled() {
         if (!isMobileNetworkCapable()) {
             return mSharedPreferences.getBoolean(PREF_CRASH_DUMP_UPLOAD_NO_CELLULAR, false);
         } else {
             String option =
                     mSharedPreferences.getString(PREF_CRASH_DUMP_UPLOAD, mCrashDumpNeverUpload);
-            return option.equals(mCrashDumpAlwaysUpload)
-                    || (option.equals(mCrashDumpWifiOnlyUpload) && isWiFiOrEthernetNetwork());
+            if (option.equals(mCrashDumpNeverUpload)) return false;
+            return true;
         }
     }
 
+    public boolean isCellularEnabledByExperiment() {

[gone, 2015/03/09 19:53:29]

nit: add javadoc 

[gayane -on leave until 09-2017, 2015/03/09 23:07:25]

Done.

+        if (mIsCellularEnabledByExperiment == null) {
+            String group_name = FieldTrialList.findFullName("UMA_EnableCellularLogUpload");
+            mIsCellularEnabledByExperiment = new Boolean(group_name.equals("Enabled"));
+        }
+        return mIsCellularEnabledByExperiment;
+    }
+
     /**
      * Sets the crash upload preference, which determines whether crash dumps will be uploaded
      * always, never, or only on wifi.
@@ -242,22 +304,6 @@ public class PrivacyPreferencesManager implements CrashReportingPermissionManage
     }
 
     /**
-     * Check whether crash dump upload preference is set to NEVER only.
-     *
-     * @return boolean {@code true} if the option is set to NEVER
-     */
-    public boolean isNeverUploadCrashDump() {
-        boolean option;
-        if (isMobileNetworkCapable()) {
-            option = mSharedPreferences.getString(PREF_CRASH_DUMP_UPLOAD, mCrashDumpNeverUpload)
-                    .equals(mCrashDumpNeverUpload);
-        } else {
-            option = !mSharedPreferences.getBoolean(PREF_CRASH_DUMP_UPLOAD_NO_CELLULAR, false);
-        }
-        return option;
-    }
-
-    /**
      * Sets the initial value for whether crash stacks may be uploaded.
      * This should be called only once, the first time Chrome is launched.
      */