Bug 1056170 - Expose pref service 'dirty' flag and test it. r=bsmedberg
authorIrving Reid <irving@mozilla.com>
Thu, 04 Sep 2014 09:52:28 -0400
changeset 228188 344a4be1b051a5d96d5cede8a979a8651c232856
parent 228187 9deef72c48d6fc061d242b4856a30fe3ad75b249
child 228189 42428115fe7081e7418da251e43fc91314ec4548
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs1056170
milestone35.0a1
Bug 1056170 - Expose pref service 'dirty' flag and test it. r=bsmedberg
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
@@ -22,17 +22,17 @@ interface nsIFile;
  * preferences management library. The preference service is directly
  * responsible for the management of the preferences files and also facilitates
  * access to the preference branch object which allows the direct manipulation
  * of the preferences themselves.
  *
  * @see nsIPrefBranch
  */
 
-[scriptable, uuid(decb9cc7-c08f-4ea5-be91-a8fc637ce2d2)]
+[scriptable, uuid(1f84fd56-3956-40df-b86a-1ea01402ee96)]
 interface nsIPrefService : nsISupports
 {
   /**
    * Called to read in the preferences specified in a user preference file.
    *
    * @param aFile The file to be read.
    *
    * @note
@@ -115,16 +115,21 @@ 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,                                            \
--- 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]