Bug 600110 - Pref follow up patch. Remove dead code, fix observers. r=dwitte. a=blocking-fennec
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -49,17 +49,16 @@
#include "mozilla/ipc/XPCShellEnvironment.h"
#include "mozilla/jsipc/PContextWrapperChild.h"
#include "mozilla/dom/ExternalHelperAppChild.h"
#include "nsIObserverService.h"
#include "nsTObserverArray.h"
#include "nsIObserver.h"
#include "nsIPrefService.h"
-#include "nsIPrefBranch.h"
#include "nsServiceManagerUtils.h"
#include "nsXULAppAPI.h"
#include "nsWeakReference.h"
#include "History.h"
#include "nsDocShellCID.h"
#include "nsNetUtil.h"
@@ -73,121 +72,16 @@
#include "nsIGeolocationProvider.h"
using namespace mozilla::ipc;
using namespace mozilla::net;
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:
AlertObserver(nsIObserver *aObserver, const nsString& aData)
: mData(aData)
, mObserver(aObserver)
{
@@ -217,17 +111,16 @@ private:
nsCOMPtr<nsIObserver> mObserver;
nsString mData;
};
ContentChild* ContentChild::sSingleton;
ContentChild::ContentChild()
- : mDead(false)
{
}
ContentChild::~ContentChild()
{
}
bool
@@ -354,24 +247,16 @@ ContentChild::ActorDestroy(ActorDestroyR
#ifndef DEBUG
// In release builds, there's no point in the content process
// going through the full XPCOM shutdown path, because it doesn't
// keep persistent state.
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();
}
void
ContentChild::ProcessingError(Result what)
{
switch (what) {
@@ -394,88 +279,35 @@ ContentChild::ProcessingError(Result wha
void
ContentChild::QuickExit()
{
NS_WARNING("content process _exit()ing");
_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)
{
NS_ASSERTION(aObserver, "Adding a null observer?");
mAlertObservers.AppendElement(new AlertObserver(aObserver, aData));
return NS_OK;
}
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];
if (observer->Observes(aData) && observer->Notify(aType)) {
// if aType == alertfinished, this alert is done. we can
// remove the observer.
if (aType.Equals(nsDependentCString("alertfinished"))) {
mAlertObservers.RemoveElementAt(i);
--- a/dom/ipc/ContentChild.h
+++ b/dom/ipc/ContentChild.h
@@ -93,32 +93,20 @@ public:
virtual bool RecvRegisterChrome(const nsTArray<ChromePackage>& packages,
const nsTArray<ResourceMapping>& resources,
const nsTArray<OverrideMapping>& overrides);
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);
virtual bool RecvAsyncMessage(const nsString& aMsg, const nsString& aJSON);
virtual bool RecvGeolocationUpdate(const GeoPosition& somewhere);
private:
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -40,16 +40,17 @@
#include "ContentParent.h"
#include "TabParent.h"
#include "History.h"
#include "mozilla/ipc/TestShellParent.h"
#include "mozilla/net/NeckoParent.h"
#include "nsIPrefBranch.h"
#include "nsIPrefBranch2.h"
+#include "nsIPrefService.h"
#include "nsIPrefLocalizedString.h"
#include "nsIObserverService.h"
#include "nsContentUtils.h"
#include "nsAutoPtr.h"
#include "nsCOMPtr.h"
#include "nsServiceManagerUtils.h"
#include "nsThreadUtils.h"
#include "nsChromeRegistryChrome.h"
@@ -266,17 +267,20 @@ ContentParent::Observe(nsISupports* aSub
if (!mIsAlive || !mSubprocess)
return NS_OK;
// listening for remotePrefs...
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)) {
NS_ConvertUTF16toUTF8 dataStr(aData);
const char *offline = dataStr.get();
if (!SendSetOffline(!strcmp(offline, "true") ? true : false))
return NS_ERROR_NOT_AVAILABLE;
}
--- a/dom/ipc/PContent.ipdl
+++ b/dom/ipc/PContent.ipdl
@@ -70,17 +70,17 @@ child:
RegisterChrome(ChromePackage[] packages, ResourceMapping[] resources,
OverrideMapping[] overrides);
async SetOffline(PRBool offline);
async NotifyVisited(URI uri);
- NotifyRemotePrefObserver(nsCString aDomain);
+ PreferenceUpdate(nsCString pref);
NotifyAlertsObserver(nsCString topic, nsString data);
GeolocationUpdate(GeoPosition somewhere);
parent:
PNecko();
--- a/modules/libpref/public/nsIPrefService.idl
+++ b/modules/libpref/public/nsIPrefService.idl
@@ -165,16 +165,18 @@ interface nsIPrefServiceInternal : nsISu
* @return NS_OK The file was read and processed.
* @return Other The file failed to read or contained invalid data.
*
* @see readUserPrefs
*/
void readExtensionPrefs(in nsILocalFile aFile);
ACString serializePreferences();
+ ACString serializePreference(in ACString aPrefName);
+ void readPrefBuffer(in ACString aBuffer);
};
%{C++
#define NS_PREFSERVICE_CID \
{ /* {1cd91b88-1dd2-11b2-92e1-ed22ed298000} */ \
0x1cd91b88, \
0x1dd2, \
--- a/modules/libpref/src/nsPrefService.cpp
+++ b/modules/libpref/src/nsPrefService.cpp
@@ -143,22 +143,17 @@ nsresult nsPrefService::Init()
#ifdef MOZ_IPC
using mozilla::dom::ContentChild;
if (XRE_GetProcessType() == GeckoProcessType_Content) {
ContentChild* cpc = ContentChild::GetSingleton();
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
nsXPIDLCString lockFileName;
/*
* The following is a small hack which will allow us to only load the library
* which supports the netscape.cfg file if the preference is defined. We
* test for the existence of the pref, set in the all.js (mozilla) or
@@ -354,16 +349,44 @@ NS_IMETHODIMP nsPrefService::SerializePr
NS_Free(*walker);
}
}
PR_Free(valueArray);
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;
if ((nsnull != aPrefRoot) && (*aPrefRoot != '\0')) {
// TODO: - cache this stuff and allow consumers to share branches (hold weak references I think)
nsPrefBranch* prefBranch = new nsPrefBranch(aPrefRoot, PR_FALSE);
if (!prefBranch)
--- a/modules/libpref/src/prefapi.cpp
+++ b/modules/libpref/src/prefapi.cpp
@@ -179,17 +179,16 @@ struct CallbackNode {
struct CallbackNode* next;
};
/* -- Prototypes */
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
nsresult PREF_Init()
{
if (!gHashTable.ops) {
if (!PL_DHashTableInit(&gHashTable, &pref_HashTableOps, nsnull,
sizeof(PrefHashEntry),
@@ -382,17 +381,16 @@ pref_CompareStrings(const void *v1, cons
return -1;
}
else if (!s2)
return 1;
else
return strcmp(s1, s2);
}
-
PRBool PREF_HasUserPref(const char *pref_name)
{
if (!gHashTable.ops)
return PR_FALSE;
PrefHashEntry *pref = pref_HashTableLookup(pref_name);
if (!pref) return PR_FALSE;
@@ -673,17 +671,17 @@ static void pref_SetValue(PrefValue* old
break;
default:
*oldValue = newValue;
}
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));
if (PL_DHASH_ENTRY_IS_FREE(result))
return nsnull;
return result;
--- a/modules/libpref/src/prefapi_private_data.h
+++ b/modules/libpref/src/prefapi_private_data.h
@@ -47,8 +47,9 @@ struct pref_saveArgs {
char **prefArray;
pref_SaveTypes saveTypes;
};
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);