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
--- 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) {