implement nsTObserverArray using nsTArray, clean up the interface, provide an nsAutoTObserverArray, and use it to reduce allocations in nsEventListenerManager. b=407442, r+sr=sicking, a=beltzner

content/base/public/nsIDocument.h

@@ -996,7 +996,7 @@ protected:
   // won't be collected
   PRUint32 mMarkedCCGeneration;
 
-  nsTObserverArray<nsIPresShell> mPresShells;
+  nsTObserverArray<nsIPresShell*> mPresShells;
 
   nsCOMArray<nsINode> mSubtreeModifiedTargets;
   PRUint32            mSubtreeModifiedDepth;

content/base/public/nsINode.h

@@ -564,7 +564,7 @@ public:
     /**
      * A list of mutation observers
      */
-    nsTObserverArray<nsIMutationObserver> mMutationObservers;
+    nsTObserverArray<nsIMutationObserver*> mMutationObservers;
 
     /**
      * An object implementing nsIDOMNodeList for this content (childNodes)

content/base/public/nsPresShellIterator.h

@@ -42,17 +42,17 @@
 #include "nsIDocument.h"
 
 class nsPresShellIterator :
-  private nsTObserverArray<nsIPresShell>::ForwardIterator
+  private nsTObserverArray<nsIPresShell*>::ForwardIterator
 {
 public:
   nsPresShellIterator(nsIDocument* aDoc)
-  : nsTObserverArray<nsIPresShell>::ForwardIterator(aDoc->mPresShells),
+  : nsTObserverArray<nsIPresShell*>::ForwardIterator(aDoc->mPresShells),
     mDoc(aDoc) {}
 
   already_AddRefed<nsIPresShell> GetNextShell()
   {
     nsIPresShell* shell = nsnull;
-    if (!mDoc->ShellsAreHidden()) {
+    if (!mDoc->ShellsAreHidden() && HasMore()) {
       shell = GetNext();
       NS_IF_ADDREF(shell);
     }

content/base/src/nsDocument.cpp

@@ -1126,7 +1126,7 @@ nsDocument::Init()
   // subclasses currently do, other don't). This is because the code in
   // nsNodeUtils always notifies the first observer first, expecting the
   // first observer to be the document.
-  NS_ENSURE_TRUE(slots->mMutationObservers.PrependObserver(this),
+  NS_ENSURE_TRUE(slots->mMutationObservers.PrependElementUnlessExists(this),
                  NS_ERROR_OUT_OF_MEMORY);
 
 
@@ -2045,7 +2045,7 @@ 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.AppendObserverUnlessExists(shell),
+  NS_ENSURE_TRUE(mPresShells.AppendElementUnlessExists(shell),
                  NS_ERROR_OUT_OF_MEMORY);
   shell.swap(*aInstancePtrResult);
 
@@ -2055,14 +2055,14 @@ nsDocument::doCreateShell(nsPresContext* aContext,
 PRBool
 nsDocument::DeleteShell(nsIPresShell* aShell)
 {
-  return mPresShells.RemoveObserver(aShell);
+  return mPresShells.RemoveElement(aShell);
 }
 
 
 nsIPresShell *
 nsDocument::GetPrimaryShell() const
 {
-  return mShellsAreHidden ? nsnull : mPresShells.SafeObserverAt(0);
+  return mShellsAreHidden ? nsnull : mPresShells.SafeElementAt(0, nsnull);
 }
 
 PR_STATIC_CALLBACK(void)
@@ -2654,7 +2654,7 @@ void
 nsDocument::AddObserver(nsIDocumentObserver* aObserver)
 {
   // The array makes sure the observer isn't already in the list
-  mObservers.AppendObserverUnlessExists(aObserver);
+  mObservers.AppendElementUnlessExists(aObserver);
   AddMutationObserver(aObserver);
 }
 
@@ -2667,7 +2667,7 @@ nsDocument::RemoveObserver(nsIDocumentObserver* aObserver)
   // don't hold a live reference to the observers.
   if (!mInDestructor) {
     RemoveMutationObserver(aObserver);
-    return mObservers.RemoveObserver(aObserver);
+    return mObservers.RemoveElement(aObserver);
   }
 
   return mObservers.Contains(aObserver);

content/base/src/nsDocument.h

@@ -741,7 +741,7 @@ protected:
   nsCOMArray<nsIStyleSheet> mCatalogSheets;
 
   // Array of observers
-  nsTObserverArray<nsIDocumentObserver> mObservers;
+  nsTObserverArray<nsIDocumentObserver*> mObservers;
 
   // The document's script global object, the object from which the
   // document can get its script context and scope. This is the

content/base/src/nsGenericElement.cpp

@@ -320,7 +320,7 @@ nsINode::AddMutationObserver(nsIMutationObserver* aMutationObserver)
 {
   nsSlots* slots = GetSlots();
   if (slots) {
-    slots->mMutationObservers.AppendObserverUnlessExists(aMutationObserver);
+    slots->mMutationObservers.AppendElementUnlessExists(aMutationObserver);
   }
 }
 
@@ -329,7 +329,7 @@ nsINode::RemoveMutationObserver(nsIMutationObserver* aMutationObserver)
 {
   nsSlots* slots = GetExistingSlots();
   if (slots) {
-    slots->mMutationObservers.RemoveObserver(aMutationObserver);
+    slots->mMutationObservers.RemoveElement(aMutationObserver);
   }
 }
 

content/events/src/nsEventListenerManager.cpp

@@ -369,10 +369,6 @@ nsEventListenerManager::~nsEventListenerManager()
 nsresult
 nsEventListenerManager::RemoveAllListeners()
 {
-  PRInt32 count = mListeners.Count();
-  for (PRInt32 i = 0; i < count; i++) {
-    delete mListeners.FastObserverAt(i);
-  }
   mListeners.Clear();
   return NS_OK;
 }
@@ -397,9 +393,9 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF_AMBIGUOUS(nsEventListenerManager, nsIEventListen
 NS_IMPL_CYCLE_COLLECTING_RELEASE_AMBIGUOUS(nsEventListenerManager, nsIEventListenerManager)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsEventListenerManager)
-  PRInt32 i, count = tmp->mListeners.Count();
+  PRInt32 i, count = tmp->mListeners.Length();
   for (i = 0; i < count; i++) {
-    cb.NoteXPCOMChild(tmp->mListeners.FastObserverAt(i)->mListener.get());
+    cb.NoteXPCOMChild(tmp->mListeners.ElementAt(i)->mListener.get());
   }  
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
@@ -477,9 +473,9 @@ nsEventListenerManager::AddEventListener(nsIDOMEventListener *aListener,
   }
 
   nsListenerStruct* ls = nsnull;
-  PRInt32 count = mListeners.Count();
+  PRInt32 count = mListeners.Length();
   for (PRInt32 i = 0; i < count; i++) {
-    ls = mListeners.FastObserverAt(i);
+    ls = mListeners.ElementAt(i);
     if (ls->mListener == aListener && ls->mFlags == aFlags &&
         ls->mGroupFlags == group &&
         (EVENT_TYPE_EQUALS(ls, aType, aTypeAtom) ||
@@ -501,7 +497,7 @@ nsEventListenerManager::AddEventListener(nsIDOMEventListener *aListener,
   ls->mGroupFlags = group;
   ls->mHandlerIsString = PR_FALSE;
   ls->mTypeData = aTypeData;
-  mListeners.AppendObserver(ls);
+  mListeners.AppendElement(ls);
 
   // For mutation listeners, we need to update the global bit on the DOM window.
   // Otherwise we won't actually fire the mutation event.
@@ -563,17 +559,16 @@ nsEventListenerManager::RemoveEventListener(nsIDOMEventListener *aListener,
   nsListenerStruct* ls = nsnull;
   aFlags &= ~NS_PRIV_EVENT_UNTRUSTED_PERMITTED;
 
-  PRInt32 count = mListeners.Count();
+  PRInt32 count = mListeners.Length();
   for (PRInt32 i = 0; i < count; ++i) {
-    ls = mListeners.FastObserverAt(i);
+    ls = mListeners.ElementAt(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.RemoveObserverAt(i);
+      mListeners.RemoveElementAt(i);
-      delete ls;
       mNoListenerForEvent = NS_EVENT_TYPE_NULL;
       mNoListenerForEventAtom = nsnull;
       break;
@@ -645,9 +640,9 @@ nsEventListenerManager::FindJSEventListener(PRUint32 aEventType,
   // Run through the listeners for this type and see if a script
   // listener is registered
   nsListenerStruct *ls;
-  PRInt32 count = mListeners.Count();
+  PRInt32 count = mListeners.Length();
   for (PRInt32 i = 0; i < count; ++i) {
-    ls = mListeners.FastObserverAt(i);
+    ls = mListeners.ElementAt(i);
     if (EVENT_TYPE_EQUALS(ls, aEventType, aTypeAtom) &&
         ls->mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) {
       return ls;
@@ -843,8 +838,7 @@ nsEventListenerManager::RemoveScriptEventListener(nsIAtom* aName)
   nsListenerStruct* ls = FindJSEventListener(eventType, aName);
 
   if (ls) {
-    mListeners.RemoveObserver(ls);
+    mListeners.RemoveElement(ls);
-    delete ls;
     mNoListenerForEvent = NS_EVENT_TYPE_NULL;
     mNoListenerForEventAtom = nsnull;
   }
@@ -1106,7 +1100,7 @@ nsEventListenerManager::HandleEvent(nsPresContext* aPresContext,
                                     PRUint32 aFlags,
                                     nsEventStatus* aEventStatus)
 {
-  if (mListeners.Count() == 0 || aEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) {
+  if (mListeners.IsEmpty() || aEvent->flags & NS_EVENT_FLAG_STOP_DISPATCH) {
     return NS_OK;
   }
 
@@ -1157,11 +1151,11 @@ nsEventListenerManager::HandleEvent(nsPresContext* aPresContext,
 
 found:
 
-  nsTObserverArray<nsListenerStruct>::EndLimitedIterator iter(mListeners);
+  nsAutoTObserverArray<nsAutoPtr<nsListenerStruct>, 2>::EndLimitedIterator iter(mListeners);
   nsAutoPopupStatePusher popupStatePusher(nsDOMEvent::GetEventPopupControlState(aEvent));
   PRBool hasListener = PR_FALSE;
-  nsListenerStruct* ls;
+  while (iter.HasMore()) {
-  while ((ls = iter.GetNext())) {
+    nsListenerStruct* ls = iter.GetNext();
     PRBool useTypeInterface =
       EVENT_TYPE_DATA_EQUALS(ls->mTypeData, typeData);
     PRBool useGenericInterface =
@@ -1682,9 +1676,9 @@ nsEventListenerManager::HasMutationListeners(PRBool* aListener)
 {
   *aListener = PR_FALSE;
   if (mMayHaveMutationListeners) {
-    PRInt32 count = mListeners.Count();
+    PRInt32 count = mListeners.Length();
     for (PRInt32 i = 0; i < count; ++i) {
-      nsListenerStruct* ls = mListeners.FastObserverAt(i);
+      nsListenerStruct* ls = mListeners.ElementAt(i);
       if (ls->mEventType >= NS_MUTATION_START &&
           ls->mEventType <= NS_MUTATION_END) {
         *aListener = PR_TRUE;
@@ -1701,9 +1695,9 @@ nsEventListenerManager::MutationListenerBits()
 {
   PRUint32 bits = 0;
   if (mMayHaveMutationListeners) {
-    PRInt32 i, count = mListeners.Count();
+    PRInt32 i, count = mListeners.Length();
     for (i = 0; i < count; ++i) {
-      nsListenerStruct* ls = mListeners.FastObserverAt(i);
+      nsListenerStruct* ls = mListeners.ElementAt(i);
       if (ls->mEventType >= NS_MUTATION_START &&
           ls->mEventType <= NS_MUTATION_END) {
         if (ls->mEventType == NS_MUTATION_SUBTREEMODIFIED) {
@@ -1739,9 +1733,9 @@ nsEventListenerManager::HasListenersFor(const nsAString& aEventName)
   }
 found:
 
-  PRInt32 i, count = mListeners.Count();
+  PRInt32 i, count = mListeners.Length();
   for (i = 0; i < count; ++i) {
-    nsListenerStruct* ls = mListeners.FastObserverAt(i);
+    nsListenerStruct* ls = mListeners.ElementAt(i);
     if (ls->mTypeAtom == atom ||
         EVENT_TYPE_DATA_EQUALS(ls->mTypeData, typeData)) {
       return PR_TRUE;
@@ -1753,9 +1747,9 @@ found:
 PRBool
 nsEventListenerManager::HasUnloadListeners()
 {
-  PRInt32 count = mListeners.Count();
+  PRInt32 count = mListeners.Length();
   for (PRInt32 i = 0; i < count; ++i) {
-    nsListenerStruct* ls = mListeners.FastObserverAt(i);
+    nsListenerStruct* ls = mListeners.ElementAt(i);
     if (ls->mEventType == NS_PAGE_UNLOAD ||
         ls->mEventType == NS_BEFORE_PAGE_UNLOAD ||
         (ls->mTypeData && ls->mTypeData->iid &&

content/events/src/nsEventListenerManager.h

@@ -49,7 +49,6 @@
 #include "nsCycleCollectionParticipant.h"
 
 class nsIDOMEvent;
-class nsVoidArray;
 class nsIAtom;
 struct EventTypeData;
 
@@ -189,17 +188,17 @@ protected:
   nsresult GetDOM2EventGroup(nsIDOMEventGroup** aGroup);
   PRBool ListenerCanHandle(nsListenerStruct* aLs, nsEvent* aEvent);
 
-  nsTObserverArray<nsListenerStruct> mListeners;
+  nsAutoTObserverArray<nsAutoPtr<nsListenerStruct>, 2> mListeners;
-  nsISupports*                       mTarget;  //WEAK
+  nsISupports*                                         mTarget;  //WEAK
-  PRUint32                           mMayHaveMutationListeners : 1;
+  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 : 31;
+  PRUint32                                             mNoListenerForEvent : 31;
-  nsCOMPtr<nsIAtom>                  mNoListenerForEventAtom;
+  nsCOMPtr<nsIAtom>                                    mNoListenerForEventAtom;
 
-  static PRUint32                    mInstanceCount;
+  static PRUint32                                      mInstanceCount;
-  static jsval                       sAddListenerID;
+  static jsval                                         sAddListenerID;
 };
 
 #endif // nsEventListenerManager_h__

layout/style/nsCSSLoader.cpp

@@ -1575,9 +1575,10 @@ CSSLoaderImpl::SheetComplete(SheetLoadData* aLoadData, nsresult aStatus)
                                         aStatus);
     }
 
-    nsTObserverArray<nsICSSLoaderObserver>::ForwardIterator iter(mObservers);
+    nsTObserverArray< nsCOMPtr<nsICSSLoaderObserver> >::ForwardIterator iter(mObservers);
     nsCOMPtr<nsICSSLoaderObserver> obs;
-    while ((obs = iter.GetNext())) {
+    while (iter.HasMore()) {
+      obs = iter.GetNext();
       LOG(("  Notifying global observer 0x%x for data 0x%s.  wasAlternate: %d",
            obs.get(), data, data->mWasAlternate));
       obs->StyleSheetLoaded(data->mSheet, data->mWasAlternate, aStatus);
@@ -2325,8 +2326,7 @@ NS_IMETHODIMP
 CSSLoaderImpl::AddObserver(nsICSSLoaderObserver* aObserver)
 {
   NS_PRECONDITION(aObserver, "Must have observer");
-  if (mObservers.AppendObserverUnlessExists(aObserver)) {
+  if (mObservers.AppendElementUnlessExists(aObserver)) {
-    NS_ADDREF(aObserver);
     return NS_OK;
   }
 
@@ -2336,9 +2336,7 @@ CSSLoaderImpl::AddObserver(nsICSSLoaderObserver* aObserver)
 NS_IMETHODIMP_(void)
 CSSLoaderImpl::RemoveObserver(nsICSSLoaderObserver* aObserver)
 {
-  if (mObservers.RemoveObserver(aObserver)) {
+  mObservers.RemoveElement(aObserver);
-    NS_RELEASE(aObserver);
-  }
 }
 
 PR_STATIC_CALLBACK(PLDHashOperator)

layout/style/nsCSSLoader.h

@@ -505,7 +505,7 @@ private:
   PRUint32 mDatasToNotifyOn;
 
   // Our array of "global" observers
-  nsTObserverArray<nsICSSLoaderObserver> mObservers;
+  nsTObserverArray< nsCOMPtr<nsICSSLoaderObserver> > mObservers;
 };
 
 #endif // nsCSSLoader_h__

modules/libpr0n/src/imgRequest.cpp

@@ -130,7 +130,7 @@ nsresult imgRequest::AddProxy(imgRequestProxy *proxy)
   NS_PRECONDITION(proxy, "null imgRequestProxy passed in");
   LOG_SCOPE_WITH_PARAM(gImgLog, "imgRequest::AddProxy", "proxy", proxy);
 
-  return mObservers.AppendObserverUnlessExists(proxy) ?
+  return mObservers.AppendElementUnlessExists(proxy) ?
     NS_OK : NS_ERROR_OUT_OF_MEMORY;
 }
 
@@ -138,7 +138,7 @@ nsresult imgRequest::RemoveProxy(imgRequestProxy *proxy, nsresult aStatus, PRBoo
 {
   LOG_SCOPE_WITH_PARAM(gImgLog, "imgRequest::RemoveProxy", "proxy", proxy);
 
-  mObservers.RemoveObserver(proxy);
+  mObservers.RemoveElement(proxy);
 
   /* Check mState below before we potentially call Cancel() below. Since
      Cancel() may result in OnStopRequest being called back before Cancel()
@@ -333,9 +333,10 @@ void imgRequest::RemoveFromCache()
 
 PRBool imgRequest::HaveProxyWithObserver(imgRequestProxy* aProxyToIgnore) const
 {
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
   imgRequestProxy* proxy;
-  while ((proxy = iter.GetNext())) {
+  while (iter.HasMore()) {
+    proxy = iter.GetNext();
     if (proxy == aProxyToIgnore) {
       continue;
     }
@@ -366,7 +367,7 @@ void imgRequest::AdjustPriority(imgRequestProxy *proxy, PRInt32 delta)
   // concern though is that image loads remain lower priority than other pieces
   // of content such as link clicks, CSS, and JS.
   //
-  if (mObservers.SafeObserverAt(0) != proxy)
+  if (mObservers.SafeElementAt(0, nsnull) != proxy)
     return;
 
   nsCOMPtr<nsISupportsPriority> p = do_QueryInterface(mRequest);
@@ -412,10 +413,9 @@ NS_IMETHODIMP imgRequest::FrameChanged(imgIContainer *container,
 {
   LOG_SCOPE(gImgLog, "imgRequest::FrameChanged");
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->FrameChanged(container, newframe, dirtyRect);
-    proxy->FrameChanged(container, newframe, dirtyRect);
   }
 
   return NS_OK;
@@ -430,10 +430,9 @@ NS_IMETHODIMP imgRequest::OnStartDecode(imgIRequest *request)
 
   mState |= onStartDecode;
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStartDecode();
-    proxy->OnStartDecode();
   }
 
   /* In the case of streaming jpegs, it is possible to get multiple OnStartDecodes which
@@ -465,10 +464,9 @@ NS_IMETHODIMP imgRequest::OnStartContainer(imgIRequest *request, imgIContainer *
 
   mImageStatus |= imgIRequest::STATUS_SIZE_AVAILABLE;
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStartContainer(image);
-    proxy->OnStartContainer(image);
   }
 
   return NS_OK;
@@ -480,10 +478,9 @@ NS_IMETHODIMP imgRequest::OnStartFrame(imgIRequest *request,
 {
   LOG_SCOPE(gImgLog, "imgRequest::OnStartFrame");
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStartFrame(frame);
-    proxy->OnStartFrame(frame);
   }
 
   return NS_OK;
@@ -496,10 +493,9 @@ NS_IMETHODIMP imgRequest::OnDataAvailable(imgIRequest *request,
 {
   LOG_SCOPE(gImgLog, "imgRequest::OnDataAvailable");
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnDataAvailable(frame, rect);
-    proxy->OnDataAvailable(frame, rect);
   }
 
   return NS_OK;
@@ -526,10 +522,9 @@ NS_IMETHODIMP imgRequest::OnStopFrame(imgIRequest *request,
     mCacheEntry->SetDataSize(cacheSize + imageSize);
   }
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStopFrame(frame);
-    proxy->OnStopFrame(frame);
   }
 
   return NS_OK;
@@ -543,10 +538,9 @@ NS_IMETHODIMP imgRequest::OnStopContainer(imgIRequest *request,
 
   mState |= onStopContainer;
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStopContainer(image);
-    proxy->OnStopContainer(image);
   }
 
   return NS_OK;
@@ -567,10 +561,9 @@ NS_IMETHODIMP imgRequest::OnStopDecode(imgIRequest *aRequest,
     mImageStatus |= imgIRequest::STATUS_ERROR;
   }
 
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStopDecode(GetResultFromImageStatus(mImageStatus), aStatusArg);
-    proxy->OnStopDecode(GetResultFromImageStatus(mImageStatus), aStatusArg);
   }
 
   return NS_OK;
@@ -607,10 +600,9 @@ NS_IMETHODIMP imgRequest::OnStartRequest(nsIRequest *aRequest, nsISupports *ctxt
   mLoading = PR_TRUE;
 
   /* notify our kids */
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStartRequest(aRequest, ctxt);
-    proxy->OnStartRequest(aRequest, ctxt);
   }
 
   /* Get our principal */
@@ -739,13 +731,9 @@ NS_IMETHODIMP imgRequest::OnStopRequest(nsIRequest *aRequest, nsISupports *ctxt,
   }
 
   /* notify the kids */
-  nsTObserverArray<imgRequestProxy>::ForwardIterator iter(mObservers);
+  nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mObservers);
-  imgRequestProxy* proxy;
+  while (iter.HasMore()) {
-  while ((proxy = iter.GetNext())) {
+    iter.GetNext()->OnStopRequest(aRequest, ctxt, status, mHadLastPart);
-    /* calling OnStopRequest may result in the death of |proxy| so don't use the
-       pointer after this call.
-     */
-    proxy->OnStopRequest(aRequest, ctxt, status, mHadLastPart);
   }
 
   return NS_OK;

modules/libpr0n/src/imgRequest.h

@@ -158,7 +158,7 @@ private:
   nsCOMPtr<imgIDecoder> mDecoder;
   nsCOMPtr<nsIProperties> mProperties;
 
-  nsTObserverArray<imgRequestProxy> mObservers;
+  nsTObserverArray<imgRequestProxy*> mObservers;
 
   PRPackedBool mLoading;
   PRPackedBool mProcessing;

xpcom/build/dlldeps.cpp

@@ -127,8 +127,8 @@ void XXXNeverCalled()
     {
       nsTObserverArray<PRBool> dummyObserverArray;
       PRBool a = PR_FALSE;
-      dummyObserverArray.AppendObserver(&a);
+      dummyObserverArray.AppendElement(a);
-      dummyObserverArray.RemoveObserver(&a);
+      dummyObserverArray.RemoveElement(a);
       dummyObserverArray.Clear();
     }
     nsStringHashSet();

xpcom/glue/nsTArray.h

@@ -359,6 +359,26 @@ class nsTArray : public nsTArray_base {
     // Search methods
     //
 
+    // This method searches for the first element in this array that is equal
+    // to the given element.
+    // @param item   The item to search for.
+    // @param comp   The Comparator used to determine element equality.
+    // @return       PR_TRUE if the element was found.
+    template<class Item, class Comparator>
+    PRBool Contains(const Item& item, const Comparator& comp) const {
+      return IndexOf(item, 0, comp) != NoIndex;
+    }
+
+    // This method searches for the first element in this array that is equal
+    // to the given element.  This method assumes that 'operator==' is defined
+    // for elem_type.
+    // @param item   The item to search for.
+    // @return       PR_TRUE if the element was found.
+    template<class Item>
+    PRBool Contains(const Item& item) const {
+      return IndexOf(item) != NoIndex;
+    }
+
     // This method searches for the offset of the first element in this
     // array that is equal to the given element.
     // @param item   The item to search for.
@@ -735,4 +755,12 @@ class nsAutoTArray : public nsTArray<E> {
     char mAutoBuf[sizeof(Header) + N * sizeof(elem_type)];
 };
 
+// specialization for N = 0. this makes the inheritance model easier for
+// templated users of nsAutoTArray.
+template<class E>
+class nsAutoTArray<E, 0> : public nsTArray<E> {
+  public:
+    nsAutoTArray() {}
+};
+
 #endif  // nsTArray_h__

xpcom/glue/nsTObserverArray.cpp

@@ -20,6 +20,7 @@
  *
  * Contributor(s):
  *   Jonas Sicking <jonas@sicking.cc> (Original Author)
+ *   Daniel Witte <dwitte@stanford.edu>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -38,15 +39,13 @@
 #include "nsTObserverArray.h"
 
 void
-nsTObserverArray_base::AdjustIterators(PRInt32 aModPos,
+nsTObserverArray_base::AdjustIterators(index_type aModPos,
-                                       PRInt32 aAdjustment)
+                                       diff_type  aAdjustment)
 {
   NS_PRECONDITION(aAdjustment == -1 || aAdjustment == 1,
                   "invalid adjustment");
   Iterator_base* iter = mIterators;
   while (iter) {
-    NS_ASSERTION(&(iter->mArray) == this, "wrong array");
-
     if (iter->mPosition > aModPos) {
       iter->mPosition += aAdjustment;
     }
@@ -55,14 +54,10 @@ nsTObserverArray_base::AdjustIterators(PRInt32 aModPos,
 }
 
 void
-nsTObserverArray_base::Clear()
+nsTObserverArray_base::ClearIterators()
 {
-  mObservers.Clear();
-
   Iterator_base* iter = mIterators;
   while (iter) {
-    NS_ASSERTION(&(iter->mArray) == this, "wrong array");
-
     iter->mPosition = 0;
     iter = iter->mNext;
   }

xpcom/glue/nsTObserverArray.h

@@ -20,6 +20,7 @@
  *
  * Contributor(s):
  *   Jonas Sicking <jonas@sicking.cc> (Original Author)
+ *   Daniel Witte <dwitte@stanford.edu>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either the GNU General Public License Version 2 or later (the "GPL"), or
@@ -38,62 +39,31 @@
 #ifndef nsTObserverArray_h___
 #define nsTObserverArray_h___
 
-#include "nsVoidArray.h"
+#include "nsTArray.h"
 
 class NS_COM_GLUE nsTObserverArray_base {
-  public:
+  protected:
-    class Iterator_base;
+    typedef PRUint32 index_type;
-    friend class Iterator_base;
+    typedef PRUint32 size_type;
+    typedef PRInt32  diff_type;
 
     class Iterator_base {
       protected:
         friend class nsTObserverArray_base;
 
-        Iterator_base(PRInt32 aPosition, const nsTObserverArray_base& aArray)
+        Iterator_base(index_type aPosition, Iterator_base* aNext)
           : mPosition(aPosition),
-            mNext(aArray.mIterators),
+            mNext(aNext) {
-            mArray(aArray) {
-          aArray.mIterators = this;
         }
 
-        ~Iterator_base() {
+        // The current position of the iterator. Its exact meaning differs
-          NS_ASSERTION(mArray.mIterators == this,
+        // depending on iterator. See nsTObserverArray<T>::ForwardIterator.
-                       "Iterators must currently be destroyed in opposite order "
+        index_type mPosition;
-                       "from the construction order. It is suggested that you "
-                       "simply put them on the stack");
-          mArray.mIterators = mNext;
-        }
-
-        // 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
-        // nsTObserverArray<T>::ForwardIterator and
-        // nsTObserverArray<T>::ReverseIterator
-        PRInt32 mPosition;
 
         // The next iterator currently iterating the same array
         Iterator_base* mNext;
-
-        // The array we're iterating
-        const nsTObserverArray_base& mArray;
     };
 
-    /**
-     * Removes all observers and collapses all iterators to the beginning of
-     * the array. The result is that forward iterators will see all elements
-     * in the array, and backward iterators will not see any more elements.
-     */
-    void Clear();
-
-  protected:
     nsTObserverArray_base()
       : mIterators(nsnull) {
     }
@@ -105,173 +75,301 @@ class NS_COM_GLUE nsTObserverArray_base {
      * @param adjustment -1 if an element was removed, 1 if an element was
      *                   added.
      */
-    void AdjustIterators(PRInt32 aModPos, PRInt32 aAdjustment);
+    void AdjustIterators(index_type aModPos, diff_type aAdjustment);
+
+    /**
+     * Clears iterators when the array is destroyed.
+     */
+    void ClearIterators();
 
     mutable Iterator_base* mIterators;
-    nsVoidArray mObservers;
 };
 
 /**
  * An array of observers. Like a normal array, but supports iterators that are
  * stable even if the array is modified during iteration.
- * The template parameter is the type of observer the array will hold pointers
+ * The template parameter T is the observer type the array will hold;
- * to.
+ * N is the number of built-in storage slots that come with the array.
+ * NOTE: You probably want to use nsTObserverArray, unless you specifically
+ * want built-in storage. See below.
+ * @see nsTObserverArray, nsTArray
  */
 
-template<class T>
+template<class T, PRUint32 N>
-class nsTObserverArray : public nsTObserverArray_base {
+class nsAutoTObserverArray : protected nsTObserverArray_base {
   public:
+    typedef T           elem_type;
+    typedef nsTArray<T> array_type;
 
-    PRUint32 Count() const {
+    nsAutoTObserverArray() {
-      return mObservers.Count();
     }
 
-    /**
+    //
-     * Adds an observer to the beginning of the array
+    // Accessor methods
-     * @param aObserver Observer to add
+    //
-     */
-    PRBool PrependObserver(T* aObserver) {
-      NS_PRECONDITION(!Contains(aObserver),
-                      "Don't prepend if the observer is already in the list");
 
-      PRBool res = mObservers.InsertElementAt(aObserver, 0);
+    // @return The number of elements in the array.
-      if (res) {
+    size_type Length() const {
-        AdjustIterators(0, 1);
+      return mArray.Length();
-      }
-      return res;
     }
 
-    /**
+    // @return True if the array is empty or false otherwise.
-     * Adds an observer to the end of the array unless it already exists in
+    PRBool IsEmpty() const {
-     * the array.
+      return mArray.IsEmpty();
-     * @param aObserver Observer to add
-     * @return True on success, false otherwise
-     */
-    PRBool AppendObserverUnlessExists(T* aObserver) {
-      return Contains(aObserver) || mObservers.AppendElement(aObserver);
     }
 
-    /**
+    // This method provides direct access to the i'th element of the array.
-     * Adds an observer to the end of the array.
+    // The given index must be within the array bounds.
-     * @param aObserver Observer to add
+    // @param i  The index of an element in the array.
-     * @return True on success, false otherwise
+    // @return   A reference to the i'th element of the array.
-     */
+    elem_type& ElementAt(index_type i) {
-    PRBool AppendObserver(T* aObserver) {
+      return mArray.ElementAt(i);
-      return mObservers.AppendElement(aObserver);
     }
 
-    /**
+    // Same as above, but readonly.
-     * Removes an observer from the array
+    const elem_type& ElementAt(index_type i) const {
-     * @param aObserver Observer to remove
+      return mArray.ElementAt(i);
-     * @return True if observer was found and removed, false otherwise
+    }
-     */
-    PRBool RemoveObserver(T* aObserver) {
-      PRInt32 index = mObservers.IndexOf(aObserver);
-      if (index < 0) {
-        return PR_FALSE;
-      }
 
-      mObservers.RemoveElementAt(index);
+    // This method provides direct access to the i'th element of the array in
+    // a bounds safe manner. If the requested index is out of bounds the
+    // provided default value is returned.
+    // @param i  The index of an element in the array.
+    // @param def The value to return if the index is out of bounds.
+    elem_type& SafeElementAt(index_type i, elem_type& def) {
+      return mArray.SafeElementAt(i, def);
+    }
+
+    // Same as above, but readonly.
+    const elem_type& SafeElementAt(index_type i, const elem_type& def) const {
+      return mArray.SafeElementAt(i, def);
+    }
+
+    //
+    // Search methods
+    //
+
+    // This method searches, starting from the beginning of the array,
+    // for the first element in this array that is equal to the given element.
+    // 'operator==' must be defined for elem_type.
+    // @param item   The item to search for.
+    // @return       PR_TRUE if the element was found.
+    template<class Item>
+    PRBool Contains(const Item& item) const {
+      return IndexOf(item) != array_type::NoIndex;
+    }
+
+    // This method searches for the offset of the first element in this
+    // array that is equal to the given element.
+    // 'operator==' must be defined for elem_type.
+    // @param item   The item to search for.
+    // @param start  The index to start from.
+    // @return       The index of the found element or NoIndex if not found.
+    template<class Item>
+    index_type IndexOf(const Item& item, index_type start = 0) const {
+      return mArray.IndexOf(item, start);
+    }
+
+    //
+    // Mutation methods
+    //
+
+    // Prepend an element to the array unless it already exists in the array.
+    // 'operator==' must be defined for elem_type.
+    // @param item   The item to prepend.
+    // @return       PR_TRUE if the element was found, or inserted successfully.
+    template<class Item>
+    PRBool PrependElementUnlessExists(const Item& item) {
+      return Contains(item) || mArray.InsertElementAt(0, item) != nsnull;
+    }
+
+    // Append an element to the array.
+    // @param item   The item to append.
+    // @return A pointer to the newly appended element, or null on OOM.
+    template<class Item>
+    elem_type* AppendElement(const Item& item) {
+      return mArray.AppendElement(item);
+    }
+
+    // Same as above, but without copy-constructing. This is useful to avoid
+    // temporaries.
+    elem_type* AppendElement() {
+      return mArray.AppendElement();
+    }
+
+    // Append an element to the array unless it already exists in the array.
+    // 'operator==' must be defined for elem_type.
+    // @param item   The item to append.
+    // @return       PR_TRUE if the element was found, or inserted successfully.
+    template<class Item>
+    PRBool AppendElementUnlessExists(const Item& item) {
+      return Contains(item) || AppendElement(item) != nsnull;
+    }
+
+    // Remove an element from the array.
+    // @param index  The index of the item to remove.
+    void RemoveElementAt(index_type index) {
+      NS_ASSERTION(index < mArray.Length(), "invalid index");
+      mArray.RemoveElementAt(index);
       AdjustIterators(index, -1);
+    }
 
+    // This helper function combines IndexOf with RemoveElementAt to "search
+    // and destroy" the first element that is equal to the given element.
+    // 'operator==' must be defined for elem_type.
+    // @param item  The item to search for.
+    // @return PR_TRUE if the element was found and removed.
+    template<class Item>
+    PRBool RemoveElement(const Item& item) {
+      index_type index = mArray.IndexOf(item, 0);
+      if (index == array_type::NoIndex)
+        return PR_FALSE;
+
+      mArray.RemoveElementAt(index);
+      AdjustIterators(index, -1);
       return PR_TRUE;
     }
 
-    /**
+    // Removes all observers and collapses all iterators to the beginning of
-     * Removes an observer from the array
+    // the array. The result is that forward iterators will see all elements
-     * @param aIndex Index of observer to remove
+    // in the array.
-     */
+    void Clear() {
-    void RemoveObserverAt(PRUint32 aIndex) {
+      mArray.Clear();
-      if (aIndex < (PRUint32)mObservers.Count()) {
+      ClearIterators();
-        mObservers.RemoveElementAt(aIndex);
-        AdjustIterators(aIndex, -1);
-      }
     }
 
-    PRBool Contains(T* aObserver) const {
+    //
-      return mObservers.IndexOf(aObserver) >= 0;
+    // Iterators
-    }
+    //
 
-    PRBool IsEmpty() const {
+    // Base class for iterators. Do not use this directly.
-      return mObservers.Count() == 0;
+    class Iterator : public Iterator_base {
-    }
+      protected:
+        friend class nsAutoTObserverArray;
+        typedef nsAutoTObserverArray<T, N> array_type;
 
-    T* SafeObserverAt(PRInt32 aIndex) const {
+        Iterator(index_type aPosition, const array_type& aArray)
-      return static_cast<T*>(mObservers.SafeElementAt(aIndex));
+          : Iterator_base(aPosition, aArray.mIterators),
-    }
+            mArray(const_cast<array_type&>(aArray)) {
+          aArray.mIterators = this;
+        }
 
-    T* FastObserverAt(PRInt32 aIndex) const {
+        ~Iterator() {
-      return static_cast<T*>(mObservers.FastElementAt(aIndex));
+          NS_ASSERTION(mArray.mIterators == this,
-    }
+                       "Iterators must currently be destroyed in opposite order "
+                       "from the construction order. It is suggested that you "
+                       "simply put them on the stack");
+          mArray.mIterators = mNext;
+        }
 
-    /**
+        // The array we're iterating
-     * Iterators
+        array_type& mArray;
-     */
+    };
 
-    // Iterates the array forward from beginning to end.
+    // Iterates the array forward from beginning to end. mPosition points
-    // mPosition points to the element that will be returned on next call
+    // to the element that will be returned on next call to GetNext.
-    // to GetNext
+    // Elements:
-    class ForwardIterator : public nsTObserverArray_base::Iterator_base {
+    // - prepended to the array during iteration *will not* be traversed
+    // - appended during iteration *will* be traversed
+    // - removed during iteration *will not* be traversed.
+    // @see EndLimitedIterator
+    class ForwardIterator : protected Iterator {
       public:
-        ForwardIterator(const nsTObserverArray<T>& aArray)
+        typedef nsAutoTObserverArray<T, N> array_type;
-          : Iterator_base(0, aArray) {
+        typedef Iterator                   base_type;
-        }
+
-        ForwardIterator(const nsTObserverArray<T>& aArray, PRInt32 aPos)
+        ForwardIterator(const array_type& aArray)
-          : Iterator_base(aPos, aArray) {
+          : Iterator(0, aArray) {
         }
 
-        PRBool operator <(const ForwardIterator& aOther) {
+        ForwardIterator(const array_type& aArray, index_type aPos)
-          NS_ASSERTION(&mArray == &aOther.mArray,
+          : Iterator(aPos, aArray) {
+        }
+
+        PRBool operator <(const ForwardIterator& aOther) const {
+          NS_ASSERTION(&this->mArray == &aOther.mArray,
                        "not iterating the same array");
-          return mPosition < aOther.mPosition;
+          return base_type::mPosition < aOther.mPosition;
         }
 
-        /**
+        // Returns PR_TRUE if there are more elements to iterate.
-         * Returns the next element and steps one step.
+        // This must precede a call to GetNext(). If PR_FALSE is
-         * Returns null if there are no more observers. Once null is returned
+        // returned, GetNext() must not be called.
-         * the iterator becomes invalid and GetNext must not be called any more.
+        PRBool HasMore() const {
-         * @return The next observer.
+          return base_type::mPosition < base_type::mArray.Length();
-         */
+        }
-        T* GetNext() {
+
-          return static_cast<T*>(GetSafeElementAt(mPosition++));
+        // Returns the next element and steps one step. This must
+        // be preceded by a call to HasMore().
+        // @return The next observer.
+        elem_type& GetNext() {
+          NS_ASSERTION(HasMore(), "iterating beyond end of array");
+          return base_type::mArray.ElementAt(base_type::mPosition++);
         }
     };
 
     // EndLimitedIterator works like ForwardIterator, but will not iterate new
-    // observers added to the array after the iterator was created.
+    // observers appended to the array after the iterator was created.
-    class EndLimitedIterator : private ForwardIterator {
+    class EndLimitedIterator : protected ForwardIterator {
       public:
-        typedef typename nsTObserverArray<T>::ForwardIterator base_type;
+        typedef nsAutoTObserverArray<T, N> array_type;
+        typedef Iterator                   base_type;
 
-        EndLimitedIterator(const nsTObserverArray<T>& aArray)
+        EndLimitedIterator(const array_type& aArray)
           : ForwardIterator(aArray),
-            mEnd(aArray, aArray.Count()) {
+            mEnd(aArray, aArray.Length()) {
         }
 
-        /**
+        // Returns PR_TRUE if there are more elements to iterate.
-         * Returns the next element and steps one step.
+        // This must precede a call to GetNext(). If PR_FALSE is
-         * Returns null if there are no more observers. Once null is returned
+        // returned, GetNext() must not be called.
-         * the iterator becomes invalid and GetNext must not be called any more.
+        PRBool HasMore() const {
-         * @return The next observer.
+          return *this < mEnd;
-         */
+        }
-        T* GetNext() {
+
-          return (*this < mEnd) ?
+        // Returns the next element and steps one step. This must
-                 static_cast<T*>(FastElementAt(base_type::mPosition++)) :
+        // be preceded by a call to HasMore().
-                 nsnull;
+        // @return The next observer.
+        elem_type& GetNext() {
+          NS_ASSERTION(HasMore(), "iterating beyond end of array");
+          return base_type::mArray.ElementAt(base_type::mPosition++);
         }
 
       private:
         ForwardIterator mEnd;
     };
+
+  protected:
+    nsAutoTArray<T, N> mArray;
+};
+
+template<class T>
+class nsTObserverArray : public nsAutoTObserverArray<T, 0> {
+  public:
+    typedef nsAutoTObserverArray<T, 0>       base_type;
+    typedef nsTObserverArray_base::size_type size_type;
+
+    //
+    // Initialization methods
+    //
+
+    nsTObserverArray() {}
+
+    // Initialize this array and pre-allocate some number of elements.
+    explicit nsTObserverArray(size_type capacity) {
+      base_type::mArray.SetCapacity(capacity);
+    }
 };
 
 // XXXbz I wish I didn't have to pass in the observer type, but I
 // don't see a way to get it out of array_.
+// Note that this macro only works if the array holds pointers to XPCOM objects.
 #define NS_OBSERVER_ARRAY_NOTIFY_OBSERVERS(array_, obstype_, func_, params_) \
   PR_BEGIN_MACRO                                                             \
-    nsTObserverArray<obstype_>::ForwardIterator iter_(array_);               \
+    nsTObserverArray<obstype_ *>::ForwardIterator iter_(array_);             \
     nsCOMPtr<obstype_> obs_;                                                 \
-    while ((obs_ = iter_.GetNext())) {                                       \
+    while (iter_.HasMore()) {                                                 \
+      obs_ = iter_.GetNext();                                                \
       obs_ -> func_ params_ ;                                                \
     }                                                                        \
   PR_END_MACRO