Bug 671190 - Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse, r=bsmedberg
authorMichael Wu <mwu@mozilla.com>
Mon, 22 Aug 2011 19:15:52 -0700
changeset 75694 0fba4d8f69c522d6578b710bc1e77fc3a3907cb8
parent 75693 f385c77d177ef1e3dcdb30a180e0ce92e1b48204
child 75695 09a0af24837d0c3f571ec450b4432a2b92061fd5
push id1409
push usermwu@mozilla.com
push dateTue, 23 Aug 2011 02:21:55 +0000
treeherdermozilla-inbound@09a0af24837d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs671190
milestone9.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 671190 - Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse, r=bsmedberg
extensions/pref/system-pref/src/gconf/nsSystemPrefService.cpp
modules/libpref/public/nsIPrefBranch.idl
modules/libpref/public/nsIPrefBranch2.idl
modules/libpref/public/nsIPrefBranchInternal.idl
modules/libpref/src/nsPrefBranch.cpp
modules/libpref/src/prefapi.cpp
modules/libpref/src/prefapi.h
--- a/extensions/pref/system-pref/src/gconf/nsSystemPrefService.cpp
+++ b/extensions/pref/system-pref/src/gconf/nsSystemPrefService.cpp
@@ -280,17 +280,17 @@ NS_IMETHODIMP nsSystemPrefService::GetPr
 /* boolean getBoolPref (in string aPrefName); */
 NS_IMETHODIMP nsSystemPrefService::GetBoolPref(const char *aPrefName, PRBool *_retval)
 {
     return mInitialized ?
         mGConf->GetBoolPref(aPrefName, _retval) : NS_ERROR_FAILURE;
 }
 
 /* void setBoolPref (in string aPrefName, in long aValue); */
-NS_IMETHODIMP nsSystemPrefService::SetBoolPref(const char *aPrefName, PRInt32 aValue)
+NS_IMETHODIMP nsSystemPrefService::SetBoolPref(const char *aPrefName, PRBool aValue)
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 /* string getCharPref (in string aPrefName); */
 NS_IMETHODIMP nsSystemPrefService::GetCharPref(const char *aPrefName, char **_retval)
 {
     return mInitialized ?
--- a/modules/libpref/public/nsIPrefBranch.idl
+++ b/modules/libpref/public/nsIPrefBranch.idl
@@ -50,17 +50,17 @@
  * For example, if this object is created with the root "browser.startup.",
  * the preferences "browser.startup.page", "browser.startup.homepage",
  * and "browser.startup.homepage_override" can be accessed by simply passing
  * "page", "homepage", or "homepage_override" to the various Get/Set methods.
  *
  * @see nsIPrefService
  */
 
-[scriptable, uuid(e0b6e170-691b-11e0-ae3e-0800200c9a66)]
+[scriptable, uuid(e162bfa0-01bd-4e9f-9843-8fb2efcd6d1f)]
 interface nsIPrefBranch : nsISupports
 {
 
   /**
    * Values describing the basic preference types.
    *
    * @see getPrefType
    */
@@ -102,17 +102,17 @@ interface nsIPrefBranch : nsISupports
    * @param aPrefName The boolean preference to set the state of.
    * @param aValue    The boolean value to set the preference to.
    *
    * @return NS_OK The value was successfully set.
    * @return Other The value was not set or is the wrong type.
    *
    * @see getBoolPref
    */
-  void setBoolPref(in string aPrefName, in long aValue);
+  void setBoolPref(in string aPrefName, in boolean aValue);
 
   /**
    * Called to get the state of an individual string preference.
    *
    * @param aPrefName The string preference to retrieve.
    *
    * @return string   The value of the requested string preference.
    *
--- a/modules/libpref/public/nsIPrefBranch2.idl
+++ b/modules/libpref/public/nsIPrefBranch2.idl
@@ -42,17 +42,17 @@
 
 interface nsIObserver;
 
 /**
  * nsIPrefBranch2 allows clients to observe changes to pref values.
  *
  * @see nsIPrefBranch
  */
-[scriptable, uuid(784de8e2-e72f-441a-ae74-9d5fdfe13be3)]
+[scriptable, uuid(d9bb54df-daac-4ce6-a70c-95d87b770cd8)]
 interface nsIPrefBranch2 : nsIPrefBranch
 {
   /**
    * Add a preference change observer. On preference changes, the following
    * arguments will be passed to the nsIObserver.observe() method:
    *   aSubject - The nsIPrefBranch object (this)
    *   aTopic   - The string defined by NS_PREFBRANCH_PREFCHANGE_TOPIC_ID
    *   aData    - The name of the preference which has changed, relative to
--- a/modules/libpref/public/nsIPrefBranchInternal.idl
+++ b/modules/libpref/public/nsIPrefBranchInternal.idl
@@ -39,12 +39,12 @@
 
 /**
  * An empty interface to provide backwards compatibility for existing code that
  * bsmedberg didn't want to break all at once. Don't use me!
  *
  * @status NON-FROZEN interface WHICH WILL PROBABLY GO AWAY.
  */
 
-[scriptable, uuid(d1d412d9-15d6-4a6a-9533-b949dc175ff5)]
+[scriptable, uuid(355bd1e9-248a-438b-809d-e0db1b287882)]
 interface nsIPrefBranchInternal : nsIPrefBranch2
 {
 };
--- a/modules/libpref/src/nsPrefBranch.cpp
+++ b/modules/libpref/src/nsPrefBranch.cpp
@@ -159,17 +159,17 @@ NS_IMETHODIMP nsPrefBranch::GetPrefType(
 
 NS_IMETHODIMP nsPrefBranch::GetBoolPref(const char *aPrefName, PRBool *_retval)
 {
   NS_ENSURE_ARG(aPrefName);
   const char *pref = getPrefName(aPrefName);
   return PREF_GetBoolPref(pref, _retval, mIsDefault);
 }
 
-NS_IMETHODIMP nsPrefBranch::SetBoolPref(const char *aPrefName, PRInt32 aValue)
+NS_IMETHODIMP nsPrefBranch::SetBoolPref(const char *aPrefName, PRBool aValue)
 {
   if (GetContentChild()) {
     NS_ERROR("cannot set pref from content process");
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   NS_ENSURE_ARG(aPrefName);
   const char *pref = getPrefName(aPrefName);
--- a/modules/libpref/src/prefapi.cpp
+++ b/modules/libpref/src/prefapi.cpp
@@ -63,19 +63,16 @@
 #include "nsPrintfCString.h"
 #include "prlink.h"
 
 #ifdef XP_OS2
 #define INCL_DOS
 #include <os2.h>
 #endif
 
-#define BOGUS_DEFAULT_INT_PREF_VALUE (-5632)
-#define BOGUS_DEFAULT_BOOL_PREF_VALUE (-2)
-
 static void
 clearPrefEntry(PLDHashTable *table, PLDHashEntryHdr *entry)
 {
     PrefHashEntry *pref = static_cast<PrefHashEntry *>(entry);
     if (pref->flags & PREF_STRING)
     {
         if (pref->defaultPref.stringVal)
             PL_strfree(pref->defaultPref.stringVal);
@@ -293,17 +290,17 @@ PREF_SetIntPref(const char *pref_name, P
 
     return pref_HashPref(pref_name, pref, PREF_INT, set_default);
 }
 
 nsresult
 PREF_SetBoolPref(const char *pref_name, PRBool value, PRBool set_default)
 {
     PrefValue pref;
-    pref.boolVal = value ? PR_TRUE : PR_FALSE;
+    pref.boolVal = value;
 
     return pref_HashPref(pref_name, pref, PREF_BOOL, set_default);
 }
 
 nsresult
 pref_SetPrefTuple(const PrefTuple &aPref, PRBool set_default)
 {
     switch (aPref.type) {
@@ -334,19 +331,20 @@ pref_savePref(PLDHashTable *table, PLDHa
     nsCAutoString prefValue;
     nsCAutoString prefPrefix;
     prefPrefix.Assign(NS_LITERAL_CSTRING("user_pref(\""));
 
     // where we're getting our pref from
     PrefValue* sourcePref;
 
     if (PREF_HAS_USER_VALUE(pref) &&
-        pref_ValueChanged(pref->defaultPref,
-                          pref->userPref,
-                          (PrefType) PREF_TYPE(pref))) {
+        (pref_ValueChanged(pref->defaultPref,
+                           pref->userPref,
+                           (PrefType) PREF_TYPE(pref)) ||
+         !(pref->flags & PREF_HAS_DEFAULT))) {
         sourcePref = &pref->userPref;
     } else {
         if (argData->saveTypes == SAVE_ALL_AND_DEFAULTS) {
             prefPrefix.Assign(NS_LITERAL_CSTRING("pref(\""));
             sourcePref = &pref->defaultPref;
         }
         else
             // do not save default prefs that haven't changed
@@ -518,17 +516,17 @@ nsresult PREF_GetIntPref(const char *pre
     nsresult rv = NS_ERROR_UNEXPECTED;
     PrefHashEntry* pref = pref_HashTableLookup(pref_name);
     if (pref && (pref->flags & PREF_INT))
     {
         if (get_default || PREF_IS_LOCKED(pref) || !PREF_HAS_USER_VALUE(pref))
         {
             PRInt32 tempInt = pref->defaultPref.intVal;
             /* check to see if we even had a default */
-            if (tempInt == ((PRInt32) BOGUS_DEFAULT_INT_PREF_VALUE))
+            if (!(pref->flags & PREF_HAS_DEFAULT))
                 return NS_ERROR_UNEXPECTED;
             *return_int = tempInt;
         }
         else
             *return_int = pref->userPref.intVal;
         rv = NS_OK;
     }
     return rv;
@@ -543,17 +541,17 @@ nsresult PREF_GetBoolPref(const char *pr
     PrefHashEntry* pref = pref_HashTableLookup(pref_name);
     //NS_ASSERTION(pref, pref_name);
     if (pref && (pref->flags & PREF_BOOL))
     {
         if (get_default || PREF_IS_LOCKED(pref) || !PREF_HAS_USER_VALUE(pref))
         {
             PRBool tempBool = pref->defaultPref.boolVal;
             /* check to see if we even had a default */
-            if (tempBool != ((PRBool) BOGUS_DEFAULT_BOOL_PREF_VALUE)) {
+            if (pref->flags & PREF_HAS_DEFAULT) {
                 *return_value = tempBool;
                 rv = NS_OK;
             }
         }
         else {
             *return_value = pref->userPref.boolVal;
             rv = NS_OK;
         }
@@ -609,21 +607,17 @@ PREF_ClearUserPref(const char *pref_name
     if (!gHashTable.ops)
         return NS_ERROR_NOT_INITIALIZED;
 
     PrefHashEntry* pref = pref_HashTableLookup(pref_name);
     if (pref && PREF_HAS_USER_VALUE(pref))
     {
         pref->flags &= ~PREF_USERSET;
 
-        if ((pref->flags & PREF_INT && 
-             pref->defaultPref.intVal == ((PRInt32) BOGUS_DEFAULT_INT_PREF_VALUE)) ||
-            (pref->flags & PREF_BOOL && 
-             pref->defaultPref.boolVal == ((PRBool) BOGUS_DEFAULT_BOOL_PREF_VALUE)) ||
-            (pref->flags & PREF_STRING && !pref->defaultPref.stringVal)) {
+        if (!(pref->flags & PREF_HAS_DEFAULT)) {
             PL_DHashTableOperate(&gHashTable, pref_name, PL_DHASH_REMOVE);
         }
 
         pref_DoCallback(pref_name);
         gDirty = PR_TRUE;
     }
     return NS_OK;
 }
@@ -635,21 +629,17 @@ pref_ClearUserPref(PLDHashTable *table, 
     PrefHashEntry *pref = static_cast<PrefHashEntry*>(he);
 
     PLDHashOperator nextOp = PL_DHASH_NEXT;
 
     if (PREF_HAS_USER_VALUE(pref))
     {
         pref->flags &= ~PREF_USERSET;
 
-        if ((pref->flags & PREF_INT && 
-             pref->defaultPref.intVal == ((PRInt32) BOGUS_DEFAULT_INT_PREF_VALUE)) ||
-            (pref->flags & PREF_BOOL && 
-             pref->defaultPref.boolVal == ((PRBool) BOGUS_DEFAULT_BOOL_PREF_VALUE)) ||
-            (pref->flags & PREF_STRING && !pref->defaultPref.stringVal)) {
+        if (!(pref->flags & PREF_HAS_DEFAULT)) {
             nextOp = PL_DHASH_REMOVE;
         }
 
         pref_DoCallback(pref->key);
     }
     return nextOp;
 }
 
@@ -752,51 +742,45 @@ nsresult pref_HashPref(const char *key, 
     // new entry, better intialize
     if (!pref->key) {
 
         // initialize the pref entry
         pref->flags = type;
         pref->key = ArenaStrDup(key, &gPrefNameArena);
         memset(&pref->defaultPref, 0, sizeof(pref->defaultPref));
         memset(&pref->userPref, 0, sizeof(pref->userPref));
-
-        /* ugly hack -- define it to a default that no pref will ever
-           default to this should really get fixed right by some out
-           of band data
-        */
-        if (pref->flags & PREF_BOOL)
-            pref->defaultPref.boolVal = (PRBool) BOGUS_DEFAULT_BOOL_PREF_VALUE;
-        if (pref->flags & PREF_INT)
-            pref->defaultPref.intVal = (PRInt32) BOGUS_DEFAULT_INT_PREF_VALUE;
     }
     else if ((((PrefType)(pref->flags)) & PREF_VALUETYPE_MASK) !=
                  (type & PREF_VALUETYPE_MASK))
     {
         NS_WARNING(nsPrintfCString(192, "Trying to set pref %s to with the wrong type!", key).get());
         return NS_ERROR_UNEXPECTED;
     }
 
     PRBool valueChanged = PR_FALSE;
     if (set_default)
     {
         if (!PREF_IS_LOCKED(pref))
         {       /* ?? change of semantics? */
-            if (pref_ValueChanged(pref->defaultPref, value, type))
+            if (pref_ValueChanged(pref->defaultPref, value, type) ||
+                !(pref->flags & PREF_HAS_DEFAULT))
             {
                 pref_SetValue(&pref->defaultPref, value, type);
+                pref->flags |= PREF_HAS_DEFAULT;
                 if (!PREF_HAS_USER_VALUE(pref))
                     valueChanged = PR_TRUE;
             }
         }
     }
     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_ValueChanged(pref->defaultPref, value, type) )
+        if (!pref_ValueChanged(pref->defaultPref, value, type) &&
+            pref->flags & PREF_HAS_DEFAULT)
         {
             if (PREF_HAS_USER_VALUE(pref))
             {
                 pref->flags &= ~PREF_USERSET;
                 if (!PREF_IS_LOCKED(pref))
                     valueChanged = PR_TRUE;
             }
         }
--- a/modules/libpref/src/prefapi.h
+++ b/modules/libpref/src/prefapi.h
@@ -53,17 +53,17 @@ typedef union
     PRBool      boolVal;
 } PrefValue;
 
 struct PrefHashEntry : PLDHashEntryHdr
 {
     const char *key;
     PrefValue defaultPref;
     PrefValue userPref;
-    PRUint8   flags;
+    PRUint16  flags;
 };
 
 /*
 // <font color=blue>
 // The Init function initializes the preference context and creates
 // the preference hashtable.
 // </font>
 */
@@ -79,19 +79,20 @@ void        PREF_CleanupPrefs();
 /*
 // <font color=blue>
 // Preference flags, including the native type of the preference
 // </font>
 */
 
 typedef enum { PREF_INVALID = 0,
                PREF_LOCKED = 1, PREF_USERSET = 2, PREF_CONFIG = 4, PREF_REMOTE = 8,
-			   PREF_LILOCAL = 16, PREF_STRING = 32, PREF_INT = 64, PREF_BOOL = 128,
-			   PREF_VALUETYPE_MASK = (PREF_STRING | PREF_INT | PREF_BOOL)
-			  } PrefType;
+               PREF_LILOCAL = 16, PREF_STRING = 32, PREF_INT = 64, PREF_BOOL = 128,
+               PREF_HAS_DEFAULT = 256,
+               PREF_VALUETYPE_MASK = (PREF_STRING | PREF_INT | PREF_BOOL)
+             } PrefType;
 
 /*
 // <font color=blue>
 // Set the various types of preferences.  These functions take a dotted
 // notation of the preference name (e.g. "browser.startup.homepage").  
 // Note that this will cause the preference to be saved to the file if
 // it is different from the default.  In other words, these are used
 // to set the _user_ preferences.