Bug 1118818 - Flush Gecko preferences when they change in Settings. r=bnicholson, a=lizzard
authorRichard Newman <rnewman@mozilla.com>
Mon, 06 Apr 2015 10:23:04 -0700
changeset 267757 b15795eeab4a75667d1b8fae49532adc38a13333
parent 267756 572747c7b17ffe2fc178ea26d6f57ff0529e8769
child 267758 44d2a7b1ca964397e0abc42d6f20c8854ddc07bb
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbnicholson, lizzard
bugs1118818
milestone39.0
Bug 1118818 - Flush Gecko preferences when they change in Settings. r=bnicholson, a=lizzard This is the simplest and most durable option: when Settings (GeckoPreferences) sends a Preferences:Set message to Gecko, typically via PrefsHelper and a SharedPreferences change observer, we include a "flush": true field. When browser.js handles Preferences:Set, it checks for this and flushes. The scope of this change, then, is preferences changes within Settings; nothing else (e.g., turning on search suggestions) is affected. I decided to do this rather than sending a flush message when leaving Settings for three reasons: * One less message. * No flushes if nothing changed. * No data loss if we crash before the user completes the departure from Settings (e.g., they change something then hit the Android 'home' button).
mobile/android/base/PrefsHelper.java
mobile/android/base/preferences/GeckoPreferences.java
mobile/android/chrome/content/browser.js
--- a/mobile/android/base/PrefsHelper.java
+++ b/mobile/android/base/PrefsHelper.java
@@ -110,23 +110,29 @@ public final class PrefsHelper {
                 }
             }
         };
         EventDispatcher.getInstance().registerGeckoThreadListener(listener, "Preferences:Data");
         sRegistered = true;
     }
 
     public static void setPref(String pref, Object value) {
+        setPref(pref, value, false);
+    }
+
+    public static void setPref(String pref, Object value, boolean flush) {
         if (pref == null || pref.length() == 0) {
             throw new IllegalArgumentException("Pref name must be non-empty");
         }
 
         try {
             JSONObject jsonPref = new JSONObject();
             jsonPref.put("name", pref);
+            jsonPref.put("flush", flush);
+
             if (value instanceof Boolean) {
                 jsonPref.put("type", "bool");
                 jsonPref.put("value", ((Boolean)value).booleanValue());
             } else if (value instanceof Integer) {
                 jsonPref.put("type", "int");
                 jsonPref.put("value", ((Integer)value).intValue());
             } else {
                 jsonPref.put("type", "string");
--- a/mobile/android/base/preferences/GeckoPreferences.java
+++ b/mobile/android/base/preferences/GeckoPreferences.java
@@ -1105,17 +1105,17 @@ OnSharedPreferenceChangeListener
             newValue = (Boolean) newValue ? 1 : 0;
         } else if (handlers.containsKey(prefName)) {
             PrefHandler handler = handlers.get(prefName);
             handler.onChange(this, preference, newValue);
         }
 
         // Send Gecko-side pref changes to Gecko
         if (isGeckoPref(prefName)) {
-            PrefsHelper.setPref(prefName, newValue);
+            PrefsHelper.setPref(prefName, newValue, true /* flush */);
         }
 
         if (preference instanceof ListPreference) {
             // We need to find the entry for the new value
             int newIndex = ((ListPreference) preference).findIndexOfValue((String) newValue);
             CharSequence newEntry = ((ListPreference) preference).getEntries()[newIndex];
             ((ListPreference) preference).setSummary(newEntry);
         } else if (preference instanceof LinkPreference) {
@@ -1208,16 +1208,17 @@ OnSharedPreferenceChangeListener
                 builder.setTitle(R.string.masterpassword_create_title)
                        .setView((View) linearLayout)
                        .setPositiveButton(R.string.button_ok, new DialogInterface.OnClickListener() {
                             @Override
                             public void onClick(DialogInterface dialog, int which) {
                                 JSONObject jsonPref = new JSONObject();
                                 try {
                                     jsonPref.put("name", PREFS_MP_ENABLED);
+                                    jsonPref.put("flush", true);
                                     jsonPref.put("type", "string");
                                     jsonPref.put("value", input1.getText().toString());
 
                                     GeckoEvent event = GeckoEvent.createBroadcastEvent("Preferences:Set", jsonPref.toString());
                                     GeckoAppShell.sendEventToGecko(event);
                                 } catch(Exception ex) {
                                     Log.e(LOGTAG, "Error setting master password", ex);
                                 }
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -1410,17 +1410,17 @@ var BrowserApp = {
 
     Messaging.sendRequest({
       type: "Preferences:Data",
       requestId: aRequestId,    // opaque request identifier, can be any string/int/whatever
       preferences: prefs
     });
   },
 
-  setPreferences: function setPreferences(aPref) {
+  setPreferences: function (aPref) {
     let json = JSON.parse(aPref);
 
     switch (json.name) {
       // The plugin pref is actually two separate prefs, so
       // we need to handle it differently
       case "plugin.enable":
         PluginHelper.setPluginPreference(json.value);
         return;
@@ -1467,16 +1467,23 @@ var BrowserApp = {
         break;
       default: {
         let pref = Cc["@mozilla.org/pref-localizedstring;1"].createInstance(Ci.nsIPrefLocalizedString);
         pref.data = json.value;
         Services.prefs.setComplexValue(json.name, Ci.nsISupportsString, pref);
         break;
       }
     }
+
+    // Finally, if we were asked to flush, flush prefs to disk right now.
+    // This allows us to be confident that prefs set in Settings are persisted,
+    // even if we crash very soon after.
+    if (json.flush) {
+      Services.prefs.savePrefFile(null);
+    }
   },
 
   sanitize: function (aItems, callback) {
     let success = true;
 
     for (let key in aItems) {
       if (!aItems[key])
         continue;