Bug 579236: Fix shutdown crash and lesser bugs with remote pref observers. r=dwitte

This commit is contained in:
Chris Jones 2010-07-21 13:42:32 -05:00
Родитель 8332fbce83
Коммит 95f3c5429a
2 изменённых файлов: 173 добавлений и 110 удалений

Просмотреть файл

@ -45,7 +45,14 @@
#include "mozilla/ipc/XPCShellEnvironment.h"
#include "mozilla/jsipc/PContextWrapperChild.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 "base/message_loop.h"
#include "base/task.h"
@ -59,9 +66,115 @@ using namespace mozilla::net;
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&);
};
ContentChild* ContentChild::sSingleton;
ContentChild::ContentChild()
: mDead(false)
{
}
@ -159,60 +272,71 @@ ContentChild::ActorDestroy(ActorDestroyReason why)
if (AbnormalShutdown == why)
NS_WARNING("shutting down because of crash!");
ClearPrefObservers();
// 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();
XRE_ShutdownChildProcess();
}
nsresult
ContentChild::AddRemotePrefObserver(const nsCString &aDomain,
const nsCString &aPrefRoot,
nsIObserver *aObserver,
ContentChild::AddRemotePrefObserver(const nsCString& aDomain,
const nsCString& aPrefRoot,
nsIObserver* aObserver,
PRBool aHoldWeak)
{
nsPrefObserverStorage* newObserver =
new nsPrefObserverStorage(aObserver, aDomain, aPrefRoot, aHoldWeak);
mPrefObserverArray.AppendElement(newObserver);
if (aObserver) {
mPrefObservers.AppendElement(
new PrefObserver(aObserver, aHoldWeak, aPrefRoot, aDomain));
}
return NS_OK;
}
nsresult
ContentChild::RemoveRemotePrefObserver(const nsCString &aDomain,
const nsCString &aPrefRoot,
nsIObserver *aObserver)
ContentChild::RemoveRemotePrefObserver(const nsCString& aDomain,
const nsCString& aPrefRoot,
nsIObserver* aObserver)
{
if (mPrefObserverArray.IsEmpty())
if (mDead) {
// Silently ignore, we're about to exit. See comment in
// ActorDestroy().
return NS_OK;
}
nsPrefObserverStorage *entry;
for (PRUint32 i = 0; i < mPrefObserverArray.Length(); ++i) {
entry = mPrefObserverArray[i];
if (entry && entry->GetObserver() == aObserver &&
entry->GetDomain().Equals(aDomain)) {
// Remove this observer from our array
mPrefObserverArray.RemoveElementAt(i);
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("No preference Observer was matched !");
NS_WARNING("RemoveRemotePrefObserver(): no observer was matched!");
return NS_ERROR_UNEXPECTED;
}
bool
ContentChild::RecvNotifyRemotePrefObserver(const nsCString& aDomain)
ContentChild::RecvNotifyRemotePrefObserver(const nsCString& aPref)
{
nsPrefObserverStorage *entry;
for (PRUint32 i = 0; i < mPrefObserverArray.Length(); ) {
entry = mPrefObserverArray[i];
nsCAutoString prefName(entry->GetPrefRoot() + entry->GetDomain());
// aDomain here is returned from our global pref observer from parent
// so it's really a full pref name (i.e. root + domain)
if (StringBeginsWith(aDomain, prefName)) {
if (!entry->NotifyObserver()) {
// remove the observer from the list
mPrefObserverArray.RemoveElementAt(i);
continue;
}
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;
}

Просмотреть файл

@ -42,85 +42,24 @@
#include "mozilla/dom/PContentChild.h"
#include "nsIObserverService.h"
#include "nsTObserverArray.h"
#include "nsIObserver.h"
#include "nsIPrefService.h"
#include "nsIPrefBranch.h"
#include "nsServiceManagerUtils.h"
#include "nsTArray.h"
#include "nsAutoPtr.h"
#include "nsWeakReference.h"
struct ChromePackage;
class nsIObserver;
struct ResourceMapping;
struct OverrideMapping;
namespace mozilla {
namespace dom {
class PrefObserver;
class ContentChild : public PContentChild
{
public:
ContentChild();
virtual ~ContentChild();
class nsPrefObserverStorage {
public:
nsPrefObserverStorage(nsIObserver *aObserver, nsCString aDomain,
nsCString aPrefRoot, bool aHoldWeak) {
mDomain = aDomain;
mPrefRoot = aPrefRoot;
mObserver = aObserver;
if (aHoldWeak) {
nsCOMPtr<nsISupportsWeakReference> weakRefFactory =
do_QueryInterface(aObserver);
if (weakRefFactory)
mWeakRef = do_GetWeakReference(aObserver);
} else {
mWeakRef = nsnull;
}
}
~nsPrefObserverStorage() {
}
bool NotifyObserver() {
nsCOMPtr<nsIObserver> observer;
if (mWeakRef) {
observer = do_QueryReferent(mWeakRef);
if (!observer) {
// this weak referenced observer went away, tell
// the caller so he can remove the observer from the list
return false;
}
} else {
observer = mObserver;
}
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;
}
nsIObserver* GetObserver() { return mObserver; }
const nsCString& GetDomain() { return mDomain; }
const nsCString& GetPrefRoot() { return mPrefRoot; }
private:
nsCOMPtr<nsIObserver> mObserver;
nsWeakPtr mWeakRef;
nsCString mPrefRoot;
nsCString mDomain;
};
bool Init(MessageLoop* aIOLoop,
base::ProcessHandle aParentHandle,
IPC::Channel* aChannel);
@ -149,28 +88,28 @@ public:
virtual bool RecvSetOffline(const PRBool& offline);
nsresult AddRemotePrefObserver(const nsCString &aDomain,
const nsCString &aPrefRoot,
nsIObserver *aObserver, PRBool aHoldWeak);
nsresult RemoveRemotePrefObserver(const nsCString &aDomain,
const nsCString &aPrefRoot,
nsIObserver *aObserver);
inline void ClearPrefObservers() {
mPrefObserverArray.Clear();
}
/**
* 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);
virtual bool RecvNotifyRemotePrefObserver(
const nsCString& aDomain);
private:
NS_OVERRIDE
virtual void ActorDestroy(ActorDestroyReason why);
static ContentChild* sSingleton;
nsTArray<nsAutoPtr<PrefObserver> > mPrefObservers;
bool mDead;
nsTArray< nsAutoPtr<nsPrefObserverStorage> > mPrefObserverArray;
static ContentChild* sSingleton;
DISALLOW_EVIL_CONSTRUCTORS(ContentChild);
};