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 203492 4a4e0c618649904bf572156453b83f7c59bf96c7
parent 203491 0d8aa5c075e984b1758573d922f2eef3e843b091
child 203493 f5134820671ac0eecc952364abeafb1be86967db
push id27428
push usercbook@mozilla.com
push dateThu, 04 Sep 2014 13:00:04 +0000
treeherdermozilla-central@7bfd030e8fc8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsemdberg
bugs1056170
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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]