Bug 1056170 - Expose pref service 'dirty' flag and test it. r=bsemdberg
☠☠ backed out by 4f30572ae7d7 ☠ ☠
authorIrving Reid <irving@mozilla.com>
Wed, 27 Aug 2014 22:21:07 -0400
changeset 228027 4a4e0c618649904bf572156453b83f7c59bf96c7
parent 228026 0d8aa5c075e984b1758573d922f2eef3e843b091
child 228028 f5134820671ac0eecc952364abeafb1be86967db
push idunknown
push userunknown
push dateunknown
reviewersbsemdberg
bugs1056170
milestone35.0a1
Bug 1056170 - Expose pref service 'dirty' flag and test it. r=bsemdberg
modules/libpref/Preferences.cpp
modules/libpref/nsIPrefService.idl
modules/libpref/prefapi.cpp
modules/libpref/test/unit/head_libPrefs.js
modules/libpref/test/unit/test_dirtyPrefs.js
modules/libpref/test/unit/xpcshell.ini
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -783,16 +783,21 @@ Preferences::GetDefaultBranch(const char
   nsRefPtr<nsPrefBranch> prefBranch = new nsPrefBranch(aPrefRoot, true);
   if (!prefBranch)
     return NS_ERROR_OUT_OF_MEMORY;
 
   prefBranch.forget(_retval);
   return NS_OK;
 }
 
+NS_IMETHODIMP
+Preferences::GetDirty(bool *_retval) {
+  *_retval = gDirty;
+  return NS_OK;
+}
 
 nsresult
 Preferences::NotifyServiceObservers(const char *aTopic)
 {
   nsCOMPtr<nsIObserverService> observerService = 
     mozilla::services::GetObserverService();  
   if (!observerService)
     return NS_ERROR_FAILURE;
--- a/modules/libpref/nsIPrefService.idl
+++ b/modules/libpref/nsIPrefService.idl
@@ -115,26 +115,29 @@ interface nsIPrefService : nsISupports
    * make sense when applied to user set preferences.
    *
    * @return nsIPrefBranch The object representing the requested default branch.
    *
    * @see getBranch
    */
   nsIPrefBranch getDefaultBranch(in string aPrefRoot);
 
+  /**
+   * The preference service is 'dirty' if there are changes to user preferences
+   * that have not been written to disk
+   */
+  readonly attribute boolean dirty;
 };
 
 %{C++
 
 #define NS_PREFSERVICE_CID                             \
-  { /* {1cd91b88-1dd2-11b2-92e1-ed22ed298000} */       \
-    0x91ca2441,                                        \
-    0x050f,                                            \
-    0x4f7c,                                            \
-    { 0x9d, 0xf8, 0x75, 0xb4, 0x0e, 0xa4, 0x01, 0x56 } \
+  { /* {410c269f-3f7a-4396-88ce-781071733182} */       \
+    0x410c269f, 0x3f7a, 0x4396,                        \
+    { 0x88, 0xce, 0x78, 0x10, 0x71, 0x73, 0x31, 0x82 } \
   }
 
 #define NS_PREFSERVICE_CONTRACTID "@mozilla.org/preferences-service;1"
 
 /**
  * Notification sent before reading the default user preferences files.
  */
 #define NS_PREFSERVICE_READ_TOPIC_ID "prefservice:before-read-userprefs"
--- a/modules/libpref/prefapi.cpp
+++ b/modules/libpref/prefapi.cpp
@@ -307,17 +307,17 @@ pref_SetPref(const dom::PrefSetting& aPr
         if (NS_FAILED(rv)) {
             return rv;
         }
     }
 
     if (userValue.type() == dom::MaybePrefValue::TPrefValue) {
         rv = SetPrefValue(prefName, userValue.get_PrefValue(), USER_VALUE);
     } else {
-        rv = PREF_ClearUserPref(prefName);      
+        rv = PREF_ClearUserPref(prefName);
     }
 
     // NB: we should never try to clear a default value, that doesn't
     // make sense
 
     return rv;
 }
 
@@ -720,17 +720,16 @@ static void pref_SetValue(PrefValue* exi
     *existingFlags = (*existingFlags & ~PREF_VALUETYPE_MASK) | newType;
     if (newType & PREF_STRING) {
         PR_ASSERT(newValue.stringVal);
         existingValue->stringVal = newValue.stringVal ? PL_strdup(newValue.stringVal) : nullptr;
     }
     else {
         *existingValue = newValue;
     }
-    gDirty = true;
 }
 
 PrefHashEntry* pref_HashTableLookup(const void *key)
 {
 #ifndef MOZ_B2G
     MOZ_ASSERT(NS_IsMainThread());
 #endif
 
@@ -780,54 +779,55 @@ nsresult pref_HashPref(const char *key, 
             if (pref_ValueChanged(pref->defaultPref, value, type) ||
                 !(pref->flags & PREF_HAS_DEFAULT))
             {
                 pref_SetValue(&pref->defaultPref, &pref->flags, value, type);
                 pref->flags |= PREF_HAS_DEFAULT;
                 if (!PREF_HAS_USER_VALUE(pref))
                     valueChanged = true;
             }
+            // What if we change the default to be the same as the user value?
+            // Should we clear the user value?
         }
     }
     else
     {
         /* If new value is same as the default value, then un-set the user value.
            Otherwise, set the user value only if it has changed */
         if ((pref->flags & PREF_HAS_DEFAULT) &&
             !pref_ValueChanged(pref->defaultPref, value, type) &&
             !(flags & kPrefForceSet))
         {
             if (PREF_HAS_USER_VALUE(pref))
             {
                 /* XXX should we free a user-set string value if there is one? */
                 pref->flags &= ~PREF_USERSET;
-                if (!PREF_IS_LOCKED(pref))
+                if (!PREF_IS_LOCKED(pref)) {
+                    gDirty = true;
                     valueChanged = true;
+                }
             }
         }
         else if (!PREF_HAS_USER_VALUE(pref) ||
                  PREF_TYPE(pref) != type ||
                  pref_ValueChanged(pref->userPref, value, type) )
         {
             pref_SetValue(&pref->userPref, &pref->flags, value, type);
             pref->flags |= PREF_USERSET;
-            if (!PREF_IS_LOCKED(pref))
+            if (!PREF_IS_LOCKED(pref)) {
+                gDirty = true;
                 valueChanged = true;
+            }
         }
     }
 
-    nsresult rv = NS_OK;
     if (valueChanged) {
-        gDirty = true;
-
-        nsresult rv2 = pref_DoCallback(key);
-        if (NS_FAILED(rv2))
-            rv = rv2;
+        return pref_DoCallback(key);
     }
-    return rv;
+    return NS_OK;
 }
 
 size_t
 pref_SizeOfPrivateData(MallocSizeOf aMallocSizeOf)
 {
     size_t n = PL_SizeOfArenaPoolExcludingPool(&gPrefNameArena, aMallocSizeOf);
     for (struct CallbackNode* node = gCallbacks; node; node = node->next) {
         n += aMallocSizeOf(node);
--- a/modules/libpref/test/unit/head_libPrefs.js
+++ b/modules/libpref/test/unit/head_libPrefs.js
@@ -27,17 +27,17 @@ function do_check_throws(f, result, stac
 var dirSvc = Cc["@mozilla.org/file/directory_service;1"].getService(Ci.nsIProperties);
 
 // Register current test directory as provider for the profile directory.
 var provider = {
   getFile: function(prop, persistent) {
     persistent.value = true;
     if (prop == NS_APP_USER_PROFILE_50_DIR)
       return dirSvc.get("CurProcD", Ci.nsIFile);
-    throw Cr.NS_ERROR_FAILURE;
+    throw Components.Exception("Tried to get test directory '" + prop + "'", Cr.NS_ERROR_FAILURE);
   },
   QueryInterface: function(iid) {
     if (iid.equals(Ci.nsIDirectoryServiceProvider) ||
         iid.equals(Ci.nsISupports)) {
       return this;
     }
     throw Cr.NS_ERROR_NO_INTERFACE;
   }
new file mode 100644
--- /dev/null
+++ b/modules/libpref/test/unit/test_dirtyPrefs.js
@@ -0,0 +1,75 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* Tests for handling of the preferences 'dirty' flag (bug 985998) */
+
+const PREF_INVALID = 0;
+const PREF_BOOL    = 128;
+const PREF_INT     = 64;
+const PREF_STRING  = 32;
+
+function run_test() {
+
+  var ps = Cc["@mozilla.org/preferences-service;1"].
+            getService(Ci.nsIPrefService);
+
+  let defaultBranch = ps.getDefaultBranch("");
+  let userBranch = ps.getBranch("");
+
+  let prefFile = do_get_profile();
+  prefFile.append("prefs.js");
+
+  //**************************************************************************//
+  // prefs are not dirty after a write
+  ps.savePrefFile(prefFile);
+  do_check_false(ps.dirty);
+
+  // set a new a user value, we should become dirty
+  userBranch.setBoolPref("DirtyTest.new.bool", true);
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+  // Overwrite a pref with the same value => not dirty
+  userBranch.setBoolPref("DirtyTest.new.bool", true);
+  do_check_false(ps.dirty);
+
+  // Repeat for the other two types
+  userBranch.setIntPref("DirtyTest.new.int", 1);
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+  // Overwrite a pref with the same value => not dirty
+  userBranch.setIntPref("DirtyTest.new.int", 1);
+  do_check_false(ps.dirty);
+
+  userBranch.setCharPref("DirtyTest.new.char", "oop");
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+  // Overwrite a pref with the same value => not dirty
+  userBranch.setCharPref("DirtyTest.new.char", "oop");
+  do_check_false(ps.dirty);
+
+  // change *type* of a user value -> dirty
+  userBranch.setBoolPref("DirtyTest.new.char", false);
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+
+  // Set a default pref => not dirty (defaults don't go into prefs.js)
+  defaultBranch.setBoolPref("DirtyTest.existing.bool", true);
+  do_check_false(ps.dirty);
+  // Fail to change type of a pref with default value -> not dirty
+  do_check_throws(function() {
+    userBranch.setCharPref("DirtyTest.existing.bool", "boo"); }, Cr.NS_ERROR_UNEXPECTED);
+  do_check_false(ps.dirty);
+
+  // Set user value same as default, not dirty
+  userBranch.setBoolPref("DirtyTest.existing.bool", true);
+  do_check_false(ps.dirty);
+  // User value different from default, dirty
+  userBranch.setBoolPref("DirtyTest.existing.bool", false);
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+  // Back to default value, dirty again
+  userBranch.setBoolPref("DirtyTest.existing.bool", true);
+  do_check_true(ps.dirty);
+  ps.savePrefFile(prefFile);
+}
--- a/modules/libpref/test/unit/xpcshell.ini
+++ b/modules/libpref/test/unit/xpcshell.ini
@@ -6,10 +6,11 @@ support-files =
   extdata/testExt.js
 
 [test_warnings.js]
 [test_bug345529.js]
 [test_bug506224.js]
 [test_bug577950.js]
 [test_bug790374.js]
 [test_changeType.js]
+[test_dirtyPrefs.js]
 [test_extprefs.js]
 [test_libPrefs.js]