Bug 600110 - Pref follow up patch. Remove dead code, fix observers. r=dwitte. a=blocking-fennec

--HG--
extra : rebase_source : dd0861639f346b0c0f6103c25586f6136aa125f3

dom/ipc/ContentChild.cpp

@@ -54,7 +54,6 @@
 #include "nsTObserverArray.h"
 #include "nsIObserver.h"
 #include "nsIPrefService.h"
-#include "nsIPrefBranch.h"
 #include "nsServiceManagerUtils.h"
 #include "nsXULAppAPI.h"
 #include "nsWeakReference.h"
@@ -78,111 +77,6 @@ using namespace mozilla::places;
 
 namespace mozilla {
 namespace dom {
-
-class PrefObserver
-{
-public:
-    /**
-     * Pass |aHoldWeak=true| to force this to hold a weak ref to
-     * |aObserver|.  Otherwise, this holds a strong ref.
-     *
-     * XXX/cjones: what do domain and prefRoot mean?
-     */
-    PrefObserver(nsIObserver *aObserver, bool aHoldWeak,
-                 const nsCString& aPrefRoot, const nsCString& aDomain)
-        : mPrefRoot(aPrefRoot)
-        , mDomain(aDomain)
-    {
-        if (aHoldWeak) {
-            nsCOMPtr<nsISupportsWeakReference> supportsWeakRef = 
-                do_QueryInterface(aObserver);
-            if (supportsWeakRef)
-                mWeakObserver = do_GetWeakReference(aObserver);
-        } else {
-            mObserver = aObserver;
-        }
-    }
-
-    ~PrefObserver() {}
-
-    /**
-     * Return true if this observer can no longer receive
-     * notifications.
-     */
-    bool IsDead() const
-    {
-        nsCOMPtr<nsIObserver> observer = GetObserver();
-        return !observer;
-    }
-
-    /**
-     * Return true iff a request to remove observers matching
-     * <aObserver, aDomain, aPrefRoot> entails removal of this.
-     */
-    bool ShouldRemoveFrom(nsIObserver* aObserver,
-                          const nsCString& aPrefRoot,
-                          const nsCString& aDomain) const
-    {
-        nsCOMPtr<nsIObserver> observer = GetObserver();
-        return (observer == aObserver &&
-                mDomain == aDomain && mPrefRoot == aPrefRoot);
-    }
-
-    /**
-     * Return true iff this should be notified of changes to |aPref|.
-     */
-    bool Observes(const nsCString& aPref) const
-    {
-        nsCAutoString myPref(mPrefRoot);
-        myPref += mDomain;
-        return StringBeginsWith(aPref, myPref);
-    }
-
-    /**
-     * Notify this of a pref change that's relevant to our interests
-     * (see Observes() above).  Return false iff this no longer cares
-     * to observe any more pref changes.
-     */
-    bool Notify() const
-    {
-        nsCOMPtr<nsIObserver> observer = GetObserver();
-        if (!observer) {
-            return false;
-        }
-
-        nsCOMPtr<nsIPrefBranch> prefBranch;
-        nsCOMPtr<nsIPrefService> prefService =
-            do_GetService(NS_PREFSERVICE_CONTRACTID);
-        if (prefService) {
-            prefService->GetBranch(mPrefRoot.get(), 
-                                   getter_AddRefs(prefBranch));
-            observer->Observe(prefBranch, "nsPref:changed",
-                              NS_ConvertASCIItoUTF16(mDomain).get());
-        }
-        return true;
-    }
-
-private:
-    already_AddRefed<nsIObserver> GetObserver() const
-    {
-        nsCOMPtr<nsIObserver> observer =
-            mObserver ? mObserver : do_QueryReferent(mWeakObserver);
-        return observer.forget();
-    }
-
-    // We only either hold a strong or a weak reference to the
-    // observer, so only either mObserver or
-    // GetReferent(mWeakObserver) is ever non-null.
-    nsCOMPtr<nsIObserver> mObserver;
-    nsWeakPtr mWeakObserver;
-    nsCString mPrefRoot;
-    nsCString mDomain;
-
-    // disable these
-    PrefObserver(const PrefObserver&);
-    PrefObserver& operator=(const PrefObserver&);
-};
-
 class AlertObserver
 {
 public:
@@ -222,7 +116,6 @@ private:
 ContentChild* ContentChild::sSingleton;
 
 ContentChild::ContentChild()
-    : mDead(false)
 {
 }
 
@@ -359,14 +252,6 @@ ContentChild::ActorDestroy(ActorDestroyReason why)
     QuickExit();
 #endif
 
-    // We might be holding the last ref to some of the observers in
-    // mPrefObserverArray.  Some of them try to unregister themselves
-    // in their dtors (sketchy).  To side-step uaf problems and so
-    // forth, we set this mDead flag.  Then, if during a Clear() a
-    // being-deleted observer tries to unregister itself, it hits the
-    // |if (mDead)| special case below and we're safe.
-    mDead = true;
-    mPrefObservers.Clear();
     mAlertObservers.Clear();
     XRE_ShutdownChildProcess();
 }
@@ -398,47 +283,6 @@ ContentChild::QuickExit()
     _exit(0);
 }
 
-nsresult
-ContentChild::AddRemotePrefObserver(const nsCString& aDomain, 
-                                    const nsCString& aPrefRoot, 
-                                    nsIObserver* aObserver, 
-                                    PRBool aHoldWeak)
-{
-    if (aObserver) {
-        mPrefObservers.AppendElement(
-            new PrefObserver(aObserver, aHoldWeak, aPrefRoot, aDomain));
-    }
-    return NS_OK;
-}
-
-nsresult
-ContentChild::RemoveRemotePrefObserver(const nsCString& aDomain, 
-                                       const nsCString& aPrefRoot, 
-                                       nsIObserver* aObserver)
-{
-    if (mDead) {
-        // Silently ignore, we're about to exit.  See comment in
-        // ActorDestroy().
-        return NS_OK;
-    }
-
-    for (PRUint32 i = 0; i < mPrefObservers.Length();
-         /*we mutate the array during the loop; ++i iff no mutation*/) {
-        PrefObserver* observer = mPrefObservers[i];
-        if (observer->IsDead()) {
-            mPrefObservers.RemoveElementAt(i);
-            continue;
-        } else if (observer->ShouldRemoveFrom(aObserver, aPrefRoot, aDomain)) {
-            mPrefObservers.RemoveElementAt(i);
-            return NS_OK;
-        }
-        ++i;
-    }
-
-    NS_WARNING("RemoveRemotePrefObserver(): no observer was matched!");
-    return NS_ERROR_UNEXPECTED;
-}
-
 nsresult
 ContentChild::AddRemoteAlertObserver(const nsString& aData,
                                      nsIObserver* aObserver)
@@ -449,28 +293,16 @@ ContentChild::AddRemoteAlertObserver(const nsString& aData,
 }
 
 bool
-ContentChild::RecvNotifyRemotePrefObserver(const nsCString& aPref)
+ContentChild::RecvPreferenceUpdate(const nsCString& aPref)
 {
-    for (PRUint32 i = 0; i < mPrefObservers.Length();
-         /*we mutate the array during the loop; ++i iff no mutation*/) {
-        PrefObserver* observer = mPrefObservers[i];
-        if (observer->Observes(aPref) &&
-            !observer->Notify()) {
-            // |observer| had a weak ref that went away, so it no
-            // longer cares about pref changes
-            mPrefObservers.RemoveElementAt(i);
-            continue;
-        }
-        ++i;
-    }
+    nsCOMPtr<nsIPrefServiceInternal> prefs = do_GetService("@mozilla.org/preferences-service;1");
+    prefs->ReadPrefBuffer(aPref);
     return true;
 }
 
 bool
 ContentChild::RecvNotifyAlertsObserver(const nsCString& aType, const nsString& aData)
 {
-    printf("ContentChild::RecvNotifyAlertsObserver %s\n", aType.get() );
-
     for (PRUint32 i = 0; i < mAlertObservers.Length();
          /*we mutate the array during the loop; ++i iff no mutation*/) {
         AlertObserver* observer = mAlertObservers[i];

dom/ipc/ContentChild.h

@@ -98,22 +98,10 @@ public:
     virtual bool RecvSetOffline(const PRBool& offline);
 
     virtual bool RecvNotifyVisited(const IPC::URI& aURI);
-
-    /**
-     * Notify |aObserver| of changes to |aPrefRoot|.|aDomain|.  If
-     * |aHoldWeak|, only a weak reference to |aObserver| is held.
-     */
-    nsresult AddRemotePrefObserver(const nsCString& aDomain, 
-                                   const nsCString& aPrefRoot, 
-                                   nsIObserver* aObserver, PRBool aHoldWeak);
-    nsresult RemoveRemotePrefObserver(const nsCString& aDomain, 
-                                      const nsCString& aPrefRoot, 
-                                      nsIObserver* aObserver);
-
     // auto remove when alertfinished is received.
     nsresult AddRemoteAlertObserver(const nsString& aData, nsIObserver* aObserver);
 
-    virtual bool RecvNotifyRemotePrefObserver(const nsCString& aDomain);
+    virtual bool RecvPreferenceUpdate(const nsCString& aDomain);
     
     virtual bool RecvNotifyAlertsObserver(const nsCString& aType, const nsString& aData);
 

dom/ipc/ContentParent.cpp

@@ -45,6 +45,7 @@
 #include "mozilla/net/NeckoParent.h"
 #include "nsIPrefBranch.h"
 #include "nsIPrefBranch2.h"
+#include "nsIPrefService.h"
 #include "nsIPrefLocalizedString.h"
 #include "nsIObserverService.h"
 #include "nsContentUtils.h"
@@ -271,7 +272,10 @@ ContentParent::Observe(nsISupports* aSubject,
     if (!strcmp(aTopic, "nsPref:changed")) {
         // We know prefs are ASCII here.
         NS_LossyConvertUTF16toASCII strData(aData);
-        if (!SendNotifyRemotePrefObserver(strData))
+        nsCString prefBuffer;
+        nsCOMPtr<nsIPrefServiceInternal> prefs = do_GetService("@mozilla.org/preferences-service;1");
+        prefs->SerializePreference(strData, prefBuffer);
+        if (!SendPreferenceUpdate(prefBuffer))
             return NS_ERROR_NOT_AVAILABLE;
     }
     else if (!strcmp(aTopic, NS_IPC_IOSERVICE_SET_OFFLINE_TOPIC)) {

dom/ipc/PContent.ipdl

@@ -75,7 +75,7 @@ child:
 
     async NotifyVisited(URI uri);
 
-    NotifyRemotePrefObserver(nsCString aDomain);
+    PreferenceUpdate(nsCString pref);
 
     NotifyAlertsObserver(nsCString topic, nsString data);
 

modules/libpref/public/nsIPrefService.idl

@@ -170,6 +170,8 @@ interface nsIPrefServiceInternal : nsISupports
   void readExtensionPrefs(in nsILocalFile aFile);
 
   ACString serializePreferences();
+  ACString serializePreference(in ACString aPrefName);
+  void readPrefBuffer(in ACString aBuffer);
 };
 
 %{C++

modules/libpref/src/nsPrefService.cpp

@@ -148,12 +148,7 @@ nsresult nsPrefService::Init()
     nsCAutoString prefs;
     cpc->SendReadPrefs(&prefs);
 
-    PrefParseState ps;
-    PREF_InitParseState(&ps, PREF_ReaderCallback, NULL);
-    nsresult rv = PREF_ParseBuf(&ps, prefs.get(), prefs.Length());
-    PREF_FinalizeParseState(&ps);
-
-    return rv;
+    return ReadPrefBuffer(prefs);
   }
 #endif
 
@@ -359,6 +354,34 @@ NS_IMETHODIMP nsPrefService::SerializePreferences(nsACString& prefs)
   return NS_OK;
 }
 
+NS_IMETHODIMP nsPrefService::SerializePreference(const nsACString& aPrefName, nsACString& aBuffer)
+{
+  PrefHashEntry *pref = pref_HashTableLookup(nsDependentCString(aPrefName).get());
+  if (!pref)
+    return NS_ERROR_NOT_AVAILABLE;
+
+  char* prefArray = nsnull;
+
+  pref_saveArgs saveArgs;
+  saveArgs.prefArray = &prefArray;
+  saveArgs.saveTypes = SAVE_ALL_AND_DEFAULTS;
+
+  pref_savePref(&gHashTable, pref, 0, &saveArgs);
+  aBuffer = prefArray;
+  PR_Free(prefArray);
+
+  return NS_OK;
+}
+
+NS_IMETHODIMP nsPrefService::ReadPrefBuffer(const nsACString& aBuffer)
+{
+  PrefParseState ps;
+  PREF_InitParseState(&ps, PREF_ReaderCallback, NULL);
+  nsresult rv = PREF_ParseBuf(&ps, nsDependentCString(aBuffer).get(), aBuffer.Length());
+  PREF_FinalizeParseState(&ps);
+  return rv;
+}
+
 NS_IMETHODIMP nsPrefService::GetBranch(const char *aPrefRoot, nsIPrefBranch **_retval)
 {
   nsresult rv;

modules/libpref/src/prefapi.cpp

@@ -184,7 +184,6 @@ static nsresult pref_DoCallback(const char* changed_pref);
 
 
 static nsresult pref_HashPref(const char *key, PrefValue value, PrefType type, PRBool defaultPref);
-static inline PrefHashEntry* pref_HashTableLookup(const void *key);
 
 #define PREF_HASHTABLE_INITIAL_SIZE	2048
 
@@ -387,7 +386,6 @@ pref_CompareStrings(const void *v1, const void *v2, void *unused)
         return strcmp(s1, s2);
 }
 
-
 PRBool PREF_HasUserPref(const char *pref_name)
 {
     if (!gHashTable.ops)
@@ -678,7 +676,7 @@ static void pref_SetValue(PrefValue* oldValue, PrefValue newValue, PrefType type
     gDirty = PR_TRUE;
 }
 
-static inline PrefHashEntry* pref_HashTableLookup(const void *key)
+PrefHashEntry* pref_HashTableLookup(const void *key)
 {
     PrefHashEntry* result =
         static_cast<PrefHashEntry*>(PL_DHashTableOperate(&gHashTable, key, PL_DHASH_LOOKUP));

modules/libpref/src/prefapi_private_data.h

@@ -52,3 +52,4 @@ PLDHashOperator
 pref_savePref(PLDHashTable *table, PLDHashEntryHdr *heh, PRUint32 i, void *arg);
 
 int pref_CompareStrings(const void *v1, const void *v2, void* unused);
+PrefHashEntry* pref_HashTableLookup(const void *key);