Bug 1263110 - Part 1 - Move crash reporter settings into their own shared pref. r=mfinkle,sebastian draft
authorJan Henning <jh+bugzilla@buttercookie.de>
Mon, 18 Apr 2016 18:56:28 +0200
changeset 356904 c4d2c082f319e119d9821177d3d9b3dc9b659591
parent 356903 c8a63768230b925c88ca3afb6761fbe090feadc7
child 356905 c2f238d98a3808c69fa403d7ec8146b9213ae278
push id16637
push usermozilla@buttercookie.de
push dateWed, 27 Apr 2016 13:50:52 +0000
reviewersmfinkle, sebastian
bugs1263110
milestone49.0a1
Bug 1263110 - Part 1 - Move crash reporter settings into their own shared pref. r=mfinkle,sebastian The crash reporter runs in its own process but uses GeckoApp's shared prefs both to store its own settings and to signal to the main process that it has crashed, which can be somewhat problematic because each process might fail to notice settings changes made by the other process. As the simple solution of enabling Context.MODE_MULTI_PROCESS for accessing the shared prefs is now deprecated, we'll devise an alternative solution instead. In Part 1 we move the settings that are used exclusively by the crash reporter into a separate shared prefs instance. MozReview-Commit-ID: 1QWBAL2Xcn2
mobile/android/base/java/org/mozilla/gecko/CrashReporter.java
mobile/android/base/java/org/mozilla/gecko/GeckoSharedPrefs.java
--- a/mobile/android/base/java/org/mozilla/gecko/CrashReporter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/CrashReporter.java
@@ -130,29 +130,29 @@ public class CrashReporter extends AppCo
         mPendingExtrasFile = new File(pendingDir, extrasFile.getName());
         moveFile(extrasFile, mPendingExtrasFile);
 
         mExtrasStringMap = new HashMap<String, String>();
         readStringsFromFile(mPendingExtrasFile.getPath(), mExtrasStringMap);
 
         // Set the flag that indicates we were stopped as expected, as
         // we will send a crash report, so it is not a silent OOM crash.
-        SharedPreferences prefs = GeckoSharedPrefs.forApp(this);
-        SharedPreferences.Editor editor = prefs.edit();
-        editor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
-        editor.putBoolean(GeckoApp.PREFS_CRASHED, true);
-        editor.apply();
+        SharedPreferences.Editor appPrefsEditor = GeckoSharedPrefs.forApp(this).edit();
+        appPrefsEditor.putBoolean(GeckoApp.PREFS_WAS_STOPPED, true);
+        appPrefsEditor.putBoolean(GeckoApp.PREFS_CRASHED, true);
+        appPrefsEditor.apply();
 
         final CheckBox allowContactCheckBox = (CheckBox) findViewById(R.id.allow_contact);
         final CheckBox includeUrlCheckBox = (CheckBox) findViewById(R.id.include_url);
         final CheckBox sendReportCheckBox = (CheckBox) findViewById(R.id.send_report);
         final EditText commentsEditText = (EditText) findViewById(R.id.comment);
         final EditText emailEditText = (EditText) findViewById(R.id.email);
 
         // Load CrashReporter preferences to avoid redundant user input.
+        SharedPreferences prefs = GeckoSharedPrefs.forCrashReporter(this);
         final boolean sendReport   = prefs.getBoolean(PREFS_SEND_REPORT, true);
         final boolean includeUrl   = prefs.getBoolean(PREFS_INCLUDE_URL, false);
         final boolean allowContact = prefs.getBoolean(PREFS_ALLOW_CONTACT, false);
         final String contactEmail  = prefs.getString(PREFS_CONTACT_EMAIL, "");
 
         allowContactCheckBox.setChecked(allowContact);
         includeUrlCheckBox.setChecked(includeUrl);
         sendReportCheckBox.setChecked(sendReport);
@@ -228,17 +228,17 @@ public class CrashReporter extends AppCo
             @Override
             public void run() {
                 sendReport(mPendingMinidumpFile, mExtrasStringMap, mPendingExtrasFile);
             }
         }, "CrashReporter Thread").start();
     }
 
     private void savePrefs() {
-        SharedPreferences.Editor editor = GeckoSharedPrefs.forApp(this).edit();
+        SharedPreferences.Editor editor = GeckoSharedPrefs.forCrashReporter(this).edit();
 
         final boolean allowContact = ((CheckBox) findViewById(R.id.allow_contact)).isChecked();
         final boolean includeUrl   = ((CheckBox) findViewById(R.id.include_url)).isChecked();
         final boolean sendReport   = ((CheckBox) findViewById(R.id.send_report)).isChecked();
         final String contactEmail  = ((EditText) findViewById(R.id.email)).getText().toString();
 
         editor.putBoolean(PREFS_ALLOW_CONTACT, allowContact);
         editor.putBoolean(PREFS_INCLUDE_URL, includeUrl);
--- a/mobile/android/base/java/org/mozilla/gecko/GeckoSharedPrefs.java
+++ b/mobile/android/base/java/org/mozilla/gecko/GeckoSharedPrefs.java
@@ -16,58 +16,73 @@ import android.content.SharedPreferences
 import android.content.SharedPreferences.Editor;
 import android.os.StrictMode;
 import android.preference.PreferenceManager;
 import android.util.Log;
 
 /**
  * {@code GeckoSharedPrefs} provides scoped SharedPreferences instances.
  * You should use this API instead of using Context.getSharedPreferences()
- * directly. There are three methods to get scoped SharedPreferences instances:
+ * directly. There are four methods to get scoped SharedPreferences instances:
  *
  * forApp()
  *     Use it for app-wide, cross-profile pref keys.
+ * forCrashReporter()
+ *     For the crash reporter, which runs in its own process.
  * forProfile()
  *     Use it to fetch and store keys for the current profile.
  * forProfileName()
  *     Use it to fetch and store keys from/for a specific profile.
  *
  * {@code GeckoSharedPrefs} has a notion of migrations. Migrations can used to
  * migrate keys from one scope to another. You can trigger a new migration by
  * incrementing PREFS_VERSION and updating migrateIfNecessary() accordingly.
  *
  * Migration history:
  *     1: Move all PreferenceManager keys to app/profile scopes
+ *     2: Move the crash reporter's private preferences into their own scope
  */
 @RobocopTarget
 public final class GeckoSharedPrefs {
     private static final String LOGTAG = "GeckoSharedPrefs";
 
     // Increment it to trigger a new migration
-    public static final int PREFS_VERSION = 1;
+    public static final int PREFS_VERSION = 2;
 
     // Name for app-scoped prefs
     public static final String APP_PREFS_NAME = "GeckoApp";
 
+    // Name for crash reporter prefs
+    public static final String CRASH_PREFS_NAME = "CrashReporter";
+
     // Used when fetching profile-scoped prefs.
     public static final String PROFILE_PREFS_NAME_PREFIX = "GeckoProfile-";
 
     // The prefs key that holds the current migration
     private static final String PREFS_VERSION_KEY = "gecko_shared_prefs_migration";
 
     // For disabling migration when getting a SharedPreferences instance
     private static final EnumSet<Flags> disableMigrations = EnumSet.of(Flags.DISABLE_MIGRATIONS);
 
     // The keys that have to be moved from ProfileManager's default
     // shared prefs to the profile from version 0 to 1.
     private static final String[] PROFILE_MIGRATIONS_0_TO_1 = {
         "home_panels",
         "home_locale"
     };
 
+    // The keys that have to be moved from the app prefs
+    // into the crash reporter's own prefs.
+    private static final String[] PROFILE_MIGRATIONS_1_TO_2 = {
+        "sendReport",
+        "includeUrl",
+        "allowContact",
+        "contactEmail"
+    };
+
     // For optimizing the migration check in subsequent get() calls
     private static volatile boolean migrationDone;
 
     public enum Flags {
         DISABLE_MIGRATIONS
     }
 
     public static SharedPreferences forApp(Context context) {
@@ -81,16 +96,32 @@ public final class GeckoSharedPrefs {
     public static SharedPreferences forApp(Context context, EnumSet<Flags> flags) {
         if (flags != null && !flags.contains(Flags.DISABLE_MIGRATIONS)) {
             migrateIfNecessary(context);
         }
 
         return context.getSharedPreferences(APP_PREFS_NAME, 0);
     }
 
+    public static SharedPreferences forCrashReporter(Context context) {
+        return forCrashReporter(context, EnumSet.noneOf(Flags.class));
+    }
+
+    /**
+     * Returns a crash-reporter-scoped SharedPreferences instance. You can disable
+     * migrations by using the DISABLE_MIGRATIONS flag.
+     */
+    public static SharedPreferences forCrashReporter(Context context, EnumSet<Flags> flags) {
+        if (flags != null && !flags.contains(Flags.DISABLE_MIGRATIONS)) {
+            migrateIfNecessary(context);
+        }
+
+        return context.getSharedPreferences(CRASH_PREFS_NAME, 0);
+    }
+
     public static SharedPreferences forProfile(Context context) {
         return forProfile(context, EnumSet.noneOf(Flags.class));
     }
 
     /**
      * Returns a SharedPreferences instance scoped to the current profile
      * in the app. You can disable migrations by using the DISABLE_MIGRATIONS
      * flag.
@@ -181,36 +212,42 @@ public final class GeckoSharedPrefs {
         final String defaultProfileName;
         try {
             defaultProfileName = GeckoProfile.getDefaultProfileName(context);
         } catch (Exception e) {
             throw new IllegalStateException("Failed to get default profile name for migration");
         }
 
         final Editor profileEditor = forProfileName(context, defaultProfileName, disableMigrations).edit();
+        final Editor crashEditor = forCrashReporter(context, disableMigrations).edit();
 
         List<String> profileKeys;
         Editor pmEditor = null;
 
         for (int v = currentVersion + 1; v <= PREFS_VERSION; v++) {
             Log.d(LOGTAG, "Migrating to version = " + v);
 
             switch (v) {
                 case 1:
                     profileKeys = Arrays.asList(PROFILE_MIGRATIONS_0_TO_1);
                     pmEditor = migrateFromPreferenceManager(context, appEditor, profileEditor, profileKeys);
                     break;
+                case 2:
+                    profileKeys = Arrays.asList(PROFILE_MIGRATIONS_1_TO_2);
+                    migrateCrashReporterSettings(appPrefs, appEditor, crashEditor, profileKeys);
+                    break;
             }
         }
 
         // Update prefs version accordingly.
         appEditor.putInt(PREFS_VERSION_KEY, PREFS_VERSION);
 
         appEditor.apply();
         profileEditor.apply();
+        crashEditor.apply();
         if (pmEditor != null) {
             pmEditor.apply();
         }
 
         Log.d(LOGTAG, "All keys have been migrated");
     }
 
     /**
@@ -238,16 +275,34 @@ public final class GeckoSharedPrefs {
             putEntry(to, key, entry.getValue());
         }
 
         // Clear PreferenceManager's prefs once we're done
         // and return the Editor to be committed.
         return pmPrefs.edit().clear();
     }
 
+    /**
+     * Moves the crash reporter's preferences from the app-wide prefs
+     * into its own shared prefs to avoid cross-process pref accesses.
+     */
+    public static void migrateCrashReporterSettings(SharedPreferences appPrefs, Editor appEditor,
+                                                    Editor crashEditor, List<String> profileKeys) {
+        Log.d(LOGTAG, "Migrating crash reporter settings");
+
+        for (Map.Entry<String, ?> entry : appPrefs.getAll().entrySet()) {
+            final String key = entry.getKey();
+
+            if (profileKeys.contains(key)) {
+                putEntry(crashEditor, key, entry.getValue());
+                appEditor.remove(key);
+            }
+        }
+    }
+
     private static void putEntry(Editor to, String key, Object value) {
         Log.d(LOGTAG, "Migrating key = " + key + " with value = " + value);
 
         if (value instanceof String) {
             to.putString(key, (String) value);
         } else if (value instanceof Boolean) {
             to.putBoolean(key, (Boolean) value);
         } else if (value instanceof Long) {