Bug 1394578 - Add PrefHashEntry::ValueMatches(). r=glandium draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 22 Nov 2017 09:53:23 +1100
changeset 702904 d3c9b81dd78c01ccb0d91e728584c9bc8a574b14
parent 702903 4c24fa3a323b29d2a64188f83412eaa89a26a231
child 702905 760e6eb36238a19c4c27b942d59c2c38bce2fb8f
push id90624
push usernnethercote@mozilla.com
push dateThu, 23 Nov 2017 23:13:32 +0000
reviewersglandium
bugs1394578
milestone59.0a1
Bug 1394578 - Add PrefHashEntry::ValueMatches(). r=glandium This factors out some common code from SetValue(), making it easier to read. The pach also improves the comments in SetValue(). MozReview-Commit-ID: 60JnBlIS1q6
modules/libpref/Preferences.cpp
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -464,16 +464,25 @@ private:
 
     if (aKind == PrefValueKind::Default) {
       mHasDefaultValue = true;
     } else {
       mHasUserValue = true;
     }
   }
 
+  bool ValueMatches(PrefValueKind aKind, PrefType aType, PrefValue aValue)
+  {
+    return (aKind == PrefValueKind::Default)
+             ? IsType(aType) && mHasDefaultValue &&
+                 mDefaultValue.Equals(aType, aValue)
+             : IsType(aType) && mHasUserValue &&
+                 mUserValue.Equals(aType, aValue);
+  }
+
 public:
   void ClearUserValue()
   {
     if (Type() == PrefType::String) {
       free(const_cast<char*>(mUserValue.mStringVal));
       mUserValue.mStringVal = nullptr;
     }
 
@@ -487,66 +496,70 @@ public:
                     bool* aDirty)
   {
     if (aFlags & kPrefSetDefault) {
       // Types must always match when setting the default value.
       if (!IsType(aType)) {
         return NS_ERROR_UNEXPECTED;
       }
 
-      if (!IsLocked()) {
-        // ?? change of semantics?
-        if (!mHasDefaultValue || !mDefaultValue.Equals(aType, aValue)) {
-          ReplaceValue(PrefValueKind::Default, aType, aValue);
-          if (aFlags & kPrefSticky) {
-            mIsSticky = true;
-          }
-          if (!mHasUserValue) {
-            *aValueChanged = true;
-          }
+      // Should we set the default value? Only if the pref is not locked, and
+      // doing so would change the default value.
+      if (!IsLocked() && !ValueMatches(PrefValueKind::Default, aType, aValue)) {
+        ReplaceValue(PrefValueKind::Default, aType, aValue);
+        if (aFlags & kPrefSticky) {
+          mIsSticky = true;
+        }
+        if (!mHasUserValue) {
+          *aValueChanged = true;
         }
         // What if we change the default to be the same as the user value?
-        // Should we clear the user value?
+        // Should we clear the user value? Currently we don't.
       }
     } else {
       // If we have a default value, types must match when setting the user
       // value.
       if (mHasDefaultValue && !IsType(aType)) {
         return NS_ERROR_UNEXPECTED;
       }
 
-      // If new value is same as the default value and it's not a "sticky"
-      // pref, then un-set the user value. Otherwise, set the user value only
-      // if it has changed.
-      if (mHasDefaultValue && !mIsSticky &&
-          mDefaultValue.Equals(aType, aValue) && !(aFlags & kPrefForceSet)) {
+      // Should we clear the user value, if present? Only if the new user value
+      // matches the default value, and the pref isn't sticky, and we aren't
+      // force-setting it.
+      if (ValueMatches(PrefValueKind::Default, aType, aValue) && !mIsSticky &&
+          !(aFlags & kPrefForceSet)) {
         if (mHasUserValue) {
           ClearUserValue();
           if (!IsLocked()) {
             *aDirty = true;
             *aValueChanged = true;
           }
         }
-      } else if (!mHasUserValue || !IsType(aType) ||
-                 !mUserValue.Equals(aType, aValue)) {
+
+        // Otherwise, should we set the user value? Only if doing so would
+        // change the user value.
+      } else if (!ValueMatches(PrefValueKind::User, aType, aValue)) {
         ReplaceValue(PrefValueKind::User, aType, aValue);
         if (!IsLocked()) {
           *aDirty = true;
           *aValueChanged = true;
         }
       }
     }
     return NS_OK;
   }
 
   // Returns false if this pref doesn't have a user value worth saving.
   bool UserValueToStringForSaving(nsCString& aStr)
   {
-    if (mHasUserValue && (!mHasDefaultValue || mIsSticky ||
-                          !mDefaultValue.Equals(Type(), mUserValue))) {
+    // Should we save the user value, if present? Only if it does not match the
+    // default value, or it is sticky.
+    if (mHasUserValue &&
+        (!ValueMatches(PrefValueKind::Default, Type(), mUserValue) ||
+         mIsSticky)) {
       if (IsTypeString()) {
         StrEscape(mUserValue.mStringVal, aStr);
 
       } else if (IsTypeInt()) {
         aStr.AppendInt(mUserValue.mIntVal);
 
       } else if (IsTypeBool()) {
         aStr = mUserValue.mBoolVal ? "true" : "false";