Bug 408123: Use nsTObserverArray rather than array copy to deal with listeners going away while firing event. r=smaug sr=jst

This commit is contained in:
jonas@sicking.cc 2007-12-13 17:41:48 -08:00
parent 2a57cdbdb4
commit 5378dfb532
7 changed files with 107 additions and 75 deletions

View File

@ -2032,7 +2032,8 @@ nsDocument::doCreateShell(nsPresContext* aContext,
NS_ENSURE_SUCCESS(rv, rv);
// Note: we don't hold a ref to the shell (it holds a ref to us)
NS_ENSURE_TRUE(mPresShells.AppendObserver(shell), NS_ERROR_OUT_OF_MEMORY);
NS_ENSURE_TRUE(mPresShells.AppendObserverUnlessExists(shell),
NS_ERROR_OUT_OF_MEMORY);
shell.swap(*aInstancePtrResult);
return NS_OK;
@ -2640,7 +2641,7 @@ void
nsDocument::AddObserver(nsIDocumentObserver* aObserver)
{
// The array makes sure the observer isn't already in the list
mObservers.AppendObserver(aObserver);
mObservers.AppendObserverUnlessExists(aObserver);
AddMutationObserver(aObserver);
}

View File

@ -284,7 +284,7 @@ nsINode::AddMutationObserver(nsIMutationObserver* aMutationObserver)
{
nsSlots* slots = GetSlots();
if (slots) {
slots->mMutationObservers.AppendObserver(aMutationObserver);
slots->mMutationObservers.AppendObserverUnlessExists(aMutationObserver);
}
}

View File

@ -346,9 +346,6 @@ PRUint32 nsEventListenerManager::mInstanceCount = 0;
nsEventListenerManager::nsEventListenerManager() :
mTarget(nsnull),
mListenersRemoved(PR_FALSE),
mListenerRemoved(PR_FALSE),
mHandlingEvent(PR_FALSE),
mMayHaveMutationListeners(PR_FALSE),
mNoListenerForEvent(NS_EVENT_TYPE_NULL)
{
@ -372,10 +369,9 @@ nsEventListenerManager::~nsEventListenerManager()
nsresult
nsEventListenerManager::RemoveAllListeners()
{
mListenersRemoved = PR_TRUE;
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; i++) {
delete static_cast<nsListenerStruct*>(mListeners.ElementAt(i));
delete mListeners.FastObserverAt(i);
}
mListeners.Clear();
return NS_OK;
@ -402,12 +398,8 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS(nsEventListenerManager, nsIEventListe
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsEventListenerManager)
PRInt32 i, count = tmp->mListeners.Count();
nsListenerStruct *ls;
for (i = 0; i < count; i++) {
ls = static_cast<nsListenerStruct*>(tmp->mListeners.ElementAt(i));
if (ls) {
cb.NoteXPCOMChild(ls->mListener.get());
}
cb.NoteXPCOMChild(tmp->mListeners.FastObserverAt(i)->mListener.get());
}
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
@ -487,7 +479,7 @@ nsEventListenerManager::AddEventListener(nsIDOMEventListener *aListener,
nsListenerStruct* ls = nsnull;
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; i++) {
ls = static_cast<nsListenerStruct*>(mListeners.ElementAt(i));
ls = mListeners.FastObserverAt(i);
if (ls->mListener == aListener && ls->mFlags == aFlags &&
ls->mGroupFlags == group &&
(EVENT_TYPE_EQUALS(ls, aType, aTypeAtom) ||
@ -509,7 +501,7 @@ nsEventListenerManager::AddEventListener(nsIDOMEventListener *aListener,
ls->mGroupFlags = group;
ls->mHandlerIsString = PR_FALSE;
ls->mTypeData = aTypeData;
mListeners.AppendElement((void*)ls);
mListeners.AppendObserver(ls);
// For mutation listeners, we need to update the global bit on the DOM window.
// Otherwise we won't actually fire the mutation event.
@ -573,18 +565,17 @@ nsEventListenerManager::RemoveEventListener(nsIDOMEventListener *aListener,
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; ++i) {
ls = static_cast<nsListenerStruct*>(mListeners.ElementAt(i));
ls = mListeners.FastObserverAt(i);
if (ls->mListener == aListener &&
ls->mGroupFlags == group &&
((ls->mFlags & ~NS_PRIV_EVENT_UNTRUSTED_PERMITTED) == aFlags) &&
(EVENT_TYPE_EQUALS(ls, aType, aUserType) ||
(!(ls->mEventType) &&
EVENT_TYPE_DATA_EQUALS(ls->mTypeData, aTypeData)))) {
mListeners.RemoveElementAt(i);
mListeners.RemoveObserverAt(i);
delete ls;
mNoListenerForEvent = NS_EVENT_TYPE_NULL;
mNoListenerForEventAtom = nsnull;
mListenerRemoved = PR_TRUE;
break;
}
}
@ -656,7 +647,7 @@ nsEventListenerManager::FindJSEventListener(PRUint32 aEventType,
nsListenerStruct *ls;
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; ++i) {
ls = static_cast<nsListenerStruct*>(mListeners.ElementAt(i));
ls = mListeners.FastObserverAt(i);
if (EVENT_TYPE_EQUALS(ls, aEventType, aTypeAtom) &&
ls->mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) {
return ls;
@ -852,11 +843,10 @@ nsEventListenerManager::RemoveScriptEventListener(nsIAtom* aName)
nsListenerStruct* ls = FindJSEventListener(eventType, aName);
if (ls) {
mListeners.RemoveElement((void*)ls);
mListeners.RemoveObserver(ls);
delete ls;
mNoListenerForEvent = NS_EVENT_TYPE_NULL;
mNoListenerForEventAtom = nsnull;
mListenerRemoved = PR_TRUE;
}
return NS_OK;
@ -1116,7 +1106,7 @@ nsEventListenerManager::HandleEvent(nsPresContext* aPresContext,
PRUint32 aFlags,
nsEventStatus* aEventStatus)
{
if (mListeners.Count() <= 0 || aEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) {
if (mListeners.Count() == 0 || aEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) {
return NS_OK;
}
@ -1167,23 +1157,11 @@ nsEventListenerManager::HandleEvent(nsPresContext* aPresContext,
found:
PRBool topMostHandleEvent = !mHandlingEvent;
if (topMostHandleEvent) {
mHandlingEvent = PR_TRUE;
mListenerRemoved = PR_FALSE;
}
PRInt32 count = mListeners.Count();
nsVoidArray originalListeners(count);
originalListeners = mListeners;
nsTObserverArray<nsListenerStruct>::EndLimitedIterator iter(mListeners);
nsAutoPopupStatePusher popupStatePusher(nsDOMEvent::GetEventPopupControlState(aEvent));
PRBool hasListener = PR_FALSE;
for (PRInt32 k = 0; !mListenersRemoved && k < count; ++k) {
nsListenerStruct* ls =
static_cast<nsListenerStruct*>(originalListeners.FastElementAt(k));
if (!ls || (mListenerRemoved && mListeners.IndexOf(ls) == -1)) {
continue;
}
nsListenerStruct* ls;
while ((ls = iter.GetNext())) {
PRBool useTypeInterface =
EVENT_TYPE_DATA_EQUALS(ls->mTypeData, typeData);
PRBool useGenericInterface =
@ -1225,10 +1203,6 @@ found:
*aEventStatus = nsEventStatus_eConsumeNoDefault;
}
if (topMostHandleEvent) {
mHandlingEvent = PR_FALSE;
}
return NS_OK;
}
@ -1710,10 +1684,8 @@ nsEventListenerManager::HasMutationListeners(PRBool* aListener)
if (mMayHaveMutationListeners) {
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; ++i) {
nsListenerStruct* ls = static_cast<nsListenerStruct*>
(mListeners.FastElementAt(i));
if (ls &&
ls->mEventType >= NS_MUTATION_START &&
nsListenerStruct* ls = mListeners.FastObserverAt(i);
if (ls->mEventType >= NS_MUTATION_START &&
ls->mEventType <= NS_MUTATION_END) {
*aListener = PR_TRUE;
break;
@ -1731,11 +1703,9 @@ nsEventListenerManager::MutationListenerBits()
if (mMayHaveMutationListeners) {
PRInt32 i, count = mListeners.Count();
for (i = 0; i < count; ++i) {
nsListenerStruct* ls = static_cast<nsListenerStruct*>
(mListeners.FastElementAt(i));
if (ls &&
(ls->mEventType >= NS_MUTATION_START &&
ls->mEventType <= NS_MUTATION_END)) {
nsListenerStruct* ls = mListeners.FastObserverAt(i);
if (ls->mEventType >= NS_MUTATION_START &&
ls->mEventType <= NS_MUTATION_END) {
if (ls->mEventType == NS_MUTATION_SUBTREEMODIFIED) {
return kAllMutationBits;
}
@ -1771,11 +1741,9 @@ found:
PRInt32 i, count = mListeners.Count();
for (i = 0; i < count; ++i) {
nsListenerStruct* ls = static_cast<nsListenerStruct*>
(mListeners.FastElementAt(i));
if (ls &&
(ls->mTypeAtom == atom ||
EVENT_TYPE_DATA_EQUALS(ls->mTypeData, typeData))) {
nsListenerStruct* ls = mListeners.FastObserverAt(i);
if (ls->mTypeAtom == atom ||
EVENT_TYPE_DATA_EQUALS(ls->mTypeData, typeData)) {
return PR_TRUE;
}
}
@ -1787,11 +1755,9 @@ nsEventListenerManager::HasUnloadListeners()
{
PRInt32 count = mListeners.Count();
for (PRInt32 i = 0; i < count; ++i) {
nsListenerStruct* ls = static_cast<nsListenerStruct*>
(mListeners.FastElementAt(i));
if (ls &&
(ls->mEventType == NS_PAGE_UNLOAD ||
ls->mEventType == NS_BEFORE_PAGE_UNLOAD) ||
nsListenerStruct* ls = mListeners.FastObserverAt(i);
if (ls->mEventType == NS_PAGE_UNLOAD ||
ls->mEventType == NS_BEFORE_PAGE_UNLOAD ||
(ls->mTypeData && ls->mTypeData->iid &&
ls->mTypeData->iid->Equals(NS_GET_IID(nsIDOMLoadListener)))) {
return PR_TRUE;

View File

@ -189,20 +189,17 @@ protected:
nsresult GetDOM2EventGroup(nsIDOMEventGroup** aGroup);
PRBool ListenerCanHandle(nsListenerStruct* aLs, nsEvent* aEvent);
nsVoidArray mListeners;
nsISupports* mTarget; //WEAK
PRPackedBool mListenersRemoved;
PRPackedBool mListenerRemoved;
PRPackedBool mHandlingEvent;
PRPackedBool mMayHaveMutationListeners;
nsTObserverArray<nsListenerStruct> mListeners;
nsISupports* mTarget; //WEAK
PRUint32 mMayHaveMutationListeners : 1;
// These two member variables are used to cache the information
// about the last event which was handled but for which event listener manager
// didn't have event listeners.
PRUint32 mNoListenerForEvent;
nsCOMPtr<nsIAtom> mNoListenerForEventAtom;
PRUint32 mNoListenerForEvent : 31;
nsCOMPtr<nsIAtom> mNoListenerForEventAtom;
static PRUint32 mInstanceCount;
static jsval sAddListenerID;
static PRUint32 mInstanceCount;
static jsval sAddListenerID;
};
#endif // nsEventListenerManager_h__

View File

@ -2325,7 +2325,7 @@ NS_IMETHODIMP
CSSLoaderImpl::AddObserver(nsICSSLoaderObserver* aObserver)
{
NS_PRECONDITION(aObserver, "Must have observer");
if (mObservers.AppendObserver(aObserver)) {
if (mObservers.AppendObserverUnlessExists(aObserver)) {
NS_ADDREF(aObserver);
return NS_OK;
}

View File

@ -130,7 +130,8 @@ nsresult imgRequest::AddProxy(imgRequestProxy *proxy)
NS_PRECONDITION(proxy, "null imgRequestProxy passed in");
LOG_SCOPE_WITH_PARAM(gImgLog, "imgRequest::AddProxy", "proxy", proxy);
return mObservers.AppendObserver(proxy) ? NS_OK : NS_ERROR_OUT_OF_MEMORY;
return mObservers.AppendObserverUnlessExists(proxy) ?
NS_OK : NS_ERROR_OUT_OF_MEMORY;
}
nsresult imgRequest::RemoveProxy(imgRequestProxy *proxy, nsresult aStatus, PRBool aNotify)

View File

@ -64,11 +64,14 @@ class NS_COM_GLUE nsTObserverArray_base {
mArray.mIterators = mNext;
}
// This function exists solely to avoid having to make the subclasses
// into friends of nsTObserverArray_base
// These functions exists solely to avoid having to make the
// subclasses into friends of nsTObserverArray_base
void* GetSafeElementAt(PRInt32 aIndex) {
return mArray.mObservers.SafeElementAt(aIndex);
}
void* FastElementAt(PRInt32 aIndex) {
return mArray.mObservers.FastElementAt(aIndex);
}
// The current position of the iterator. It's exact meaning differs
// depending on if the array is iterated forwards or backwards. See
@ -119,6 +122,10 @@ template<class T>
class nsTObserverArray : public nsTObserverArray_base {
public:
PRUint32 Count() const {
return mObservers.Count();
}
/**
* Adds an observer to the beginning of the array
* @param aObserver Observer to add
@ -140,10 +147,19 @@ class nsTObserverArray : public nsTObserverArray_base {
* @param aObserver Observer to add
* @return True on success, false otherwise
*/
PRBool AppendObserver(T* aObserver) {
PRBool AppendObserverUnlessExists(T* aObserver) {
return Contains(aObserver) || mObservers.AppendElement(aObserver);
}
/**
* Adds an observer to the end of the array.
* @param aObserver Observer to add
* @return True on success, false otherwise
*/
PRBool AppendObserver(T* aObserver) {
return mObservers.AppendElement(aObserver);
}
/**
* Removes an observer from the array
* @param aObserver Observer to remove
@ -161,6 +177,17 @@ class nsTObserverArray : public nsTObserverArray_base {
return PR_TRUE;
}
/**
* Removes an observer from the array
* @param aIndex Index of observer to remove
*/
void RemoveObserverAt(PRUint32 aIndex) {
if (aIndex < (PRUint32)mObservers.Count()) {
mObservers.RemoveElementAt(aIndex);
AdjustIterators(aIndex, -1);
}
}
PRBool Contains(T* aObserver) const {
return mObservers.IndexOf(aObserver) >= 0;
}
@ -173,6 +200,10 @@ class nsTObserverArray : public nsTObserverArray_base {
return static_cast<T*>(mObservers.SafeElementAt(aIndex));
}
T* FastObserverAt(PRInt32 aIndex) const {
return static_cast<T*>(mObservers.FastElementAt(aIndex));
}
/**
* Iterators
*/
@ -185,6 +216,15 @@ class nsTObserverArray : public nsTObserverArray_base {
ForwardIterator(const nsTObserverArray<T>& aArray)
: Iterator_base(0, aArray) {
}
ForwardIterator(const nsTObserverArray<T>& aArray, PRInt32 aPos)
: Iterator_base(aPos, aArray) {
}
PRBool operator <(const ForwardIterator& aOther) {
NS_ASSERTION(&mArray == &aOther.mArray,
"not iterating the same array");
return mPosition < aOther.mPosition;
}
/**
* Returns the next element and steps one step.
@ -196,6 +236,33 @@ class nsTObserverArray : public nsTObserverArray_base {
return static_cast<T*>(GetSafeElementAt(mPosition++));
}
};
// EndLimitedIterator works like ForwardIterator, but will not iterate new
// observers added to the array after the iterator was created.
class EndLimitedIterator : private ForwardIterator {
public:
typedef typename nsTObserverArray<T>::ForwardIterator base_type;
EndLimitedIterator(const nsTObserverArray<T>& aArray)
: ForwardIterator(aArray),
mEnd(aArray, aArray.Count()) {
}
/**
* Returns the next element and steps one step.
* Returns null if there are no more observers. Once null is returned
* the iterator becomes invalid and GetNext must not be called any more.
* @return The next observer.
*/
T* GetNext() {
return (*this < mEnd) ?
static_cast<T*>(FastElementAt(base_type::mPosition++)) :
nsnull;
}
private:
ForwardIterator mEnd;
};
};
// XXXbz I wish I didn't have to pass in the observer type, but I