Bug 493701 - part2: Use TObserverArray for DocLoader Listeners; r=bzbarsky
authorArpad Borsos <arpad.borsos@googlemail.com>
Sat, 19 Apr 2014 09:43:34 +0200
changeset 180935 30f8577510eac44e29f94b7c0ae1b78c3bd38274
parent 180934 bff4e97827d2c28f3acb6b149afc4dc78b4cbfe1
child 180936 4797c2555c1b9dbcccdbae033393e1d38fa43ebb
push id272
push userpvanderbeken@mozilla.com
push dateMon, 05 May 2014 16:31:18 +0000
reviewersbzbarsky
bugs493701
milestone31.0a1
Bug 493701 - part2: Use TObserverArray for DocLoader Listeners; r=bzbarsky * * * Bug 493701 - part 3: add and use a BackwardIterator::Remove method; r=bzbarsky
uriloader/base/nsDocLoader.cpp
uriloader/base/nsDocLoader.h
--- a/uriloader/base/nsDocLoader.cpp
+++ b/uriloader/base/nsDocLoader.cpp
@@ -77,34 +77,30 @@ nsDocLoader::RequestInfoHashInitEntry(PL
 void
 nsDocLoader::RequestInfoHashClearEntry(PLDHashTable* table,
                                        PLDHashEntryHdr* entry)
 {
   nsRequestInfo* info = static_cast<nsRequestInfo *>(entry);
   info->~nsRequestInfo();
 }
 
-struct nsListenerInfo {
-  nsListenerInfo(nsIWeakReference *aListener, unsigned long aNotifyMask) 
-    : mWeakListener(aListener),
-      mNotifyMask(aNotifyMask)
-  {
-  }
-
-  // Weak pointer for the nsIWebProgressListener...
-  nsWeakPtr mWeakListener;
-
-  // Mask indicating which notifications the listener wants to receive.
-  unsigned long mNotifyMask;
+// this is used for mListenerInfoList.Contains()
+template <>
+class nsDefaultComparator <nsDocLoader::nsListenerInfo, nsIWebProgressListener*> {
+  public:
+    bool Equals(const nsDocLoader::nsListenerInfo& aInfo,
+                nsIWebProgressListener* const& aListener) const {
+      nsCOMPtr<nsIWebProgressListener> listener =
+                                       do_QueryReferent(aInfo.mWeakListener);
+      return aListener == listener;
+    }
 };
 
-
 nsDocLoader::nsDocLoader()
   : mParent(nullptr),
-    mListenerInfoList(8),
     mCurrentSelfProgress(0),
     mMaxSelfProgress(0),
     mCurrentTotalProgress(0),
     mMaxTotalProgress(0),
     mCompletedTotalProgress(0),
     mIsLoadingDocument(false),
     mIsRestoringDocument(false),
     mDontFlushLayout(false),
@@ -370,25 +366,16 @@ nsDocLoader::Destroy()
   if (mParent) 
   {
     mParent->RemoveChildLoader(this);
   }
 
   // Release all the information about network requests...
   ClearRequestInfoHash();
 
-  // Release all the information about registered listeners...
-  int32_t count = mListenerInfoList.Count();
-  for(int32_t i = 0; i < count; i++) {
-    nsListenerInfo *info =
-      static_cast<nsListenerInfo*>(mListenerInfoList.ElementAt(i));
-
-    delete info;
-  }
-
   mListenerInfoList.Clear();
   mListenerInfoList.Compact();
 
   mDocumentRequest = 0;
 
   if (mLoadGroup)
     mLoadGroup->SetGroupObserver(nullptr);
 
@@ -876,54 +863,36 @@ void nsDocLoader::doStopDocumentLoad(nsI
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
 // The following section contains support for nsIWebProgress and related stuff
 ////////////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP
 nsDocLoader::AddProgressListener(nsIWebProgressListener *aListener,
-                                     uint32_t aNotifyMask)
+                                 uint32_t aNotifyMask)
 {
-  nsresult rv;
-
-  nsListenerInfo* info = GetListenerInfo(aListener);
-  if (info) {
+  if (mListenerInfoList.Contains(aListener)) {
     // The listener is already registered!
     return NS_ERROR_FAILURE;
   }
 
   nsWeakPtr listener = do_GetWeakReference(aListener);
   if (!listener) {
     return NS_ERROR_INVALID_ARG;
   }
 
-  info = new nsListenerInfo(listener, aNotifyMask);
-  if (!info) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
-  rv = mListenerInfoList.AppendElement(info) ? NS_OK : NS_ERROR_FAILURE;
-  return rv;
+  return mListenerInfoList.AppendElement(nsListenerInfo(listener, aNotifyMask)) ?
+         NS_OK : NS_ERROR_OUT_OF_MEMORY;
 }
 
 NS_IMETHODIMP
 nsDocLoader::RemoveProgressListener(nsIWebProgressListener *aListener)
 {
-  nsresult rv;
-
-  nsListenerInfo* info = GetListenerInfo(aListener);
-  if (info) {
-    rv = mListenerInfoList.RemoveElement(info) ? NS_OK : NS_ERROR_FAILURE;
-    delete info;
-  } else {
-    // The listener is not registered!
-    rv = NS_ERROR_FAILURE;
-  }
-  return rv;
+  return mListenerInfoList.RemoveElement(aListener) ? NS_OK : NS_ERROR_FAILURE;
 }
 
 NS_IMETHODIMP
 nsDocLoader::GetDOMWindow(nsIDOMWindow **aResult)
 {
   return CallGetInterface(this, aResult);
 }
 
@@ -1175,16 +1144,38 @@ void nsDocLoader::ClearInternalProgress(
 
   mCurrentSelfProgress  = mMaxSelfProgress  = 0;
   mCurrentTotalProgress = mMaxTotalProgress = 0;
   mCompletedTotalProgress = 0;
 
   mProgressStateFlags = nsIWebProgressListener::STATE_STOP;
 }
 
+/**
+ * |_code| is executed for every listener matching |_flag|
+ * |listener| should be used inside |_code| as the nsIWebProgressListener var.
+ */
+#define NOTIFY_LISTENERS(_flag, _code)                     \
+PR_BEGIN_MACRO                                             \
+  nsCOMPtr<nsIWebProgressListener> listener;               \
+  ListenerArray::BackwardIterator iter(mListenerInfoList); \
+  while (iter.HasMore()) {                                 \
+    nsListenerInfo &info = iter.GetNext();                 \
+    if (!(info.mNotifyMask & (_flag))) {                   \
+      continue;                                            \
+    }                                                      \
+    listener = do_QueryReferent(info.mWeakListener);       \
+    if (!listener) {                                       \
+      iter.Remove();                                       \
+      continue;                                            \
+    }                                                      \
+    _code                                                  \
+  }                                                        \
+  mListenerInfoList.Compact();                             \
+PR_END_MACRO
 
 void nsDocLoader::FireOnProgressChange(nsDocLoader *aLoadInitiator,
                                        nsIRequest *request,
                                        int64_t aProgress,
                                        int64_t aProgressMax,
                                        int64_t aProgressDelta,
                                        int64_t aTotalProgress,
                                        int64_t aMaxTotalProgress)
@@ -1201,48 +1192,22 @@ void nsDocLoader::FireOnProgressChange(n
   nsAutoCString buffer;
 
   GetURIStringFromRequest(request, buffer);
   PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, 
          ("DocLoader:%p: Progress (%s): curSelf: %d maxSelf: %d curTotal: %d maxTotal %d\n",
           this, buffer.get(), aProgress, aProgressMax, aTotalProgress, aMaxTotalProgress));
 #endif /* DEBUG */
 
-  /*
-   * First notify any listeners of the new progress info...
-   *
-   * Operate the elements from back to front so that if items get
-   * get removed from the list it won't affect our iteration
-   */
-  nsCOMPtr<nsIWebProgressListener> listener;
-  int32_t count = mListenerInfoList.Count();
-
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_PROGRESS)) {
-      continue;
-    }
-
-    listener = do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_PROGRESS,
     // XXX truncates 64-bit to 32-bit
     listener->OnProgressChange(aLoadInitiator,request,
                                int32_t(aProgress), int32_t(aProgressMax),
                                int32_t(aTotalProgress), int32_t(aMaxTotalProgress));
-  }
-
-  mListenerInfoList.Compact();
+  );
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnProgressChange(aLoadInitiator, request,
                                   aProgress, aProgressMax,
                                   aProgressDelta,
                                   aTotalProgress, aMaxTotalProgress);
   }
@@ -1295,215 +1260,92 @@ void nsDocLoader::DoFireOnStateChange(ns
   GetURIStringFromRequest(aRequest, buffer);
   PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, 
          ("DocLoader:%p: Status (%s): code: %x\n",
          this, buffer.get(), aStateFlags));
 #endif /* DEBUG */
 
   NS_ASSERTION(aRequest, "Firing OnStateChange(...) notification with a NULL request!");
 
-  /*                                                                           
-   * First notify any listeners of the new state info...
-   *
-   * Operate the elements from back to front so that if items get
-   * get removed from the list it won't affect our iteration
-   */
-  nsCOMPtr<nsIWebProgressListener> listener;
-  int32_t count = mListenerInfoList.Count();
-  int32_t notifyMask = (aStateFlags >> 16) & nsIWebProgress::NOTIFY_STATE_ALL;
-
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & notifyMask)) {
-      continue;
-    }
-
-    listener = do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(((aStateFlags >> 16) & nsIWebProgress::NOTIFY_STATE_ALL),
     listener->OnStateChange(aProgress, aRequest, aStateFlags, aStatus);
-  }
-
-  mListenerInfoList.Compact();
+  );
 }
 
 
 
 void
 nsDocLoader::FireOnLocationChange(nsIWebProgress* aWebProgress,
                                   nsIRequest* aRequest,
                                   nsIURI *aUri,
                                   uint32_t aFlags)
 {
-  /*                                                                           
-   * First notify any listeners of the new state info...
-   *
-   * Operate the elements from back to front so that if items get
-   * get removed from the list it won't affect our iteration
-   */
-  nsCOMPtr<nsIWebProgressListener> listener;
-  int32_t count = mListenerInfoList.Count();
-
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_LOCATION)) {
-      continue;
-    }
-
-    listener = do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_LOCATION,
     PR_LOG(gDocLoaderLog, PR_LOG_DEBUG, ("DocLoader [%p] calling %p->OnLocationChange", this, listener.get()));
     listener->OnLocationChange(aWebProgress, aRequest, aUri, aFlags);
-  }
-
-  mListenerInfoList.Compact();
+  );
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnLocationChange(aWebProgress, aRequest, aUri, aFlags);
   }
 }
 
 void
 nsDocLoader::FireOnStatusChange(nsIWebProgress* aWebProgress,
                                 nsIRequest* aRequest,
                                 nsresult aStatus,
                                 const char16_t* aMessage)
 {
-  /*                                                                           
-   * First notify any listeners of the new state info...
-   *
-   * Operate the elements from back to front so that if items get
-   * get removed from the list it won't affect our iteration
-   */
-  nsCOMPtr<nsIWebProgressListener> listener;
-  int32_t count = mListenerInfoList.Count();
-
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_STATUS)) {
-      continue;
-    }
-
-    listener = do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_STATUS,
     listener->OnStatusChange(aWebProgress, aRequest, aStatus, aMessage);
-  }
-  mListenerInfoList.Compact();
+  );
   
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->FireOnStatusChange(aWebProgress, aRequest, aStatus, aMessage);
   }
 }
 
 bool
 nsDocLoader::RefreshAttempted(nsIWebProgress* aWebProgress,
                               nsIURI *aURI,
                               int32_t aDelay,
                               bool aSameURI)
 {
   /*
    * Returns true if the refresh may proceed,
    * false if the refresh should be blocked.
-   *
-   * First notify any listeners of the refresh attempt...
-   *
-   * Iterate the elements from back to front so that if items
-   * get removed from the list it won't affect our iteration
    */
   bool allowRefresh = true;
-  int32_t count = mListenerInfoList.Count();
 
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_REFRESH)) {
-      continue;
-    }
-
-    nsCOMPtr<nsIWebProgressListener> listener =
-      do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_REFRESH,
     nsCOMPtr<nsIWebProgressListener2> listener2 =
-      do_QueryReferent(info->mWeakListener);
+      do_QueryReferent(info.mWeakListener);
     if (!listener2)
       continue;
 
     bool listenerAllowedRefresh;
     nsresult listenerRV = listener2->OnRefreshAttempted(
         aWebProgress, aURI, aDelay, aSameURI, &listenerAllowedRefresh);
     if (NS_FAILED(listenerRV))
       continue;
 
     allowRefresh = allowRefresh && listenerAllowedRefresh;
-  }
-
-  mListenerInfoList.Compact();
+  );
 
   // Pass the notification up to the parent...
   if (mParent) {
     allowRefresh = allowRefresh &&
       mParent->RefreshAttempted(aWebProgress, aURI, aDelay, aSameURI);
   }
 
   return allowRefresh;
 }
 
-nsListenerInfo * 
-nsDocLoader::GetListenerInfo(nsIWebProgressListener *aListener)
-{
-  int32_t i, count;
-  nsListenerInfo *info;
-
-  nsCOMPtr<nsISupports> listener1 = do_QueryInterface(aListener);
-  count = mListenerInfoList.Count();
-  for (i=0; i<count; i++) {
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(i));
-
-    NS_ASSERTION(info, "There should NEVER be a null listener in the list");
-    if (info) {
-      nsCOMPtr<nsISupports> listener2 = do_QueryReferent(info->mWeakListener);
-      if (listener1 == listener2)
-        return info;
-    }
-  }
-  return nullptr;
-}
-
 nsresult nsDocLoader::AddRequestInfo(nsIRequest *aRequest)
 {
   if (!PL_DHashTableOperate(&mRequestInfoHash, aRequest, PL_DHASH_ADD)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   return NS_OK;
 }
@@ -1618,45 +1460,19 @@ NS_IMETHODIMP nsDocLoader::OnSecurityCha
 {
   //
   // Fire progress notifications out to any registered nsIWebProgressListeners.  
   //
   
   nsCOMPtr<nsIRequest> request = do_QueryInterface(aContext);
   nsIWebProgress* webProgress = static_cast<nsIWebProgress*>(this);
 
-  /*                                                                           
-   * First notify any listeners of the new state info...
-   *
-   * Operate the elements from back to front so that if items get
-   * get removed from the list it won't affect our iteration
-   */
-  nsCOMPtr<nsIWebProgressListener> listener;
-  int32_t count = mListenerInfoList.Count();
-
-  while (--count >= 0) {
-    nsListenerInfo *info;
-
-    info = static_cast<nsListenerInfo*>(mListenerInfoList.SafeElementAt(count));
-    if (!info || !(info->mNotifyMask & nsIWebProgress::NOTIFY_SECURITY)) {
-      continue;
-    }
-
-    listener = do_QueryReferent(info->mWeakListener);
-    if (!listener) {
-      // the listener went away. gracefully pull it out of the list.
-      mListenerInfoList.RemoveElementAt(count);
-      delete info;
-      continue;
-    }
-
+  NOTIFY_LISTENERS(nsIWebProgress::NOTIFY_SECURITY,
     listener->OnSecurityChange(webProgress, request, aState);
-  }
-
-  mListenerInfoList.Compact();
+  );
 
   // Pass the notification up to the parent...
   if (mParent) {
     mParent->OnSecurityChange(aContext, aState);
   }
   return NS_OK;
 }
 
--- a/uriloader/base/nsDocLoader.h
+++ b/uriloader/base/nsDocLoader.h
@@ -12,33 +12,30 @@
 #include "nsIDocumentLoader.h"
 #include "nsIWebProgress.h"
 #include "nsIWebProgressListener.h"
 #include "nsIRequestObserver.h"
 #include "nsWeakReference.h"
 #include "nsILoadGroup.h"
 #include "nsCOMArray.h"
 #include "nsTObserverArray.h"
-#include "nsVoidArray.h"
 #include "nsString.h"
 #include "nsIChannel.h"
 #include "nsIProgressEventSink.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsIChannelEventSink.h"
 #include "nsISecurityEventSink.h"
 #include "nsISupportsPriority.h"
 #include "nsCOMPtr.h"
 #include "pldhash.h"
 #include "nsAutoPtr.h"
 
 #include "mozilla/LinkedList.h"
 
-struct nsListenerInfo;
-
 /****************************************************************************
  * nsDocLoader implementation...
  ****************************************************************************/
 
 #define NS_THIS_DOCLOADER_IMPL_CID                    \
  { /* b4ec8387-98aa-4c08-93b6-6d23069c06f2 */         \
      0xb4ec8387,                                      \
      0x98aa,                                          \
@@ -93,16 +90,30 @@ public:
     // Remove aChild from our childlist.  This nulls out the child's mParent
     // pointer.
     nsresult RemoveChildLoader(nsDocLoader *aChild);
     // Add aChild to our child list.  This will set aChild's mParent pointer to
     // |this|.
     nsresult AddChildLoader(nsDocLoader* aChild);
     nsDocLoader* GetParent() const { return mParent; }
 
+    struct nsListenerInfo {
+      nsListenerInfo(nsIWeakReference *aListener, unsigned long aNotifyMask) 
+        : mWeakListener(aListener),
+          mNotifyMask(aNotifyMask)
+      {
+      }
+
+      // Weak pointer for the nsIWebProgressListener...
+      nsWeakPtr mWeakListener;
+
+      // Mask indicating which notifications the listener wants to receive.
+      unsigned long mNotifyMask;
+    };
+
 protected:
     virtual ~nsDocLoader();
 
     virtual nsresult SetDocLoaderParent(nsDocLoader * aLoader);
 
     bool IsBusy();
 
     void Destroy();
@@ -244,17 +255,18 @@ protected:
     // for owning pointers and raw COM interface pointers for weak
     // (ie, non owning) references. If you add any members to this
     // class, please make the ownership explicit (pinkerton, scc).
   
     nsCOMPtr<nsIRequest>       mDocumentRequest;       // [OWNER] ???compare with document
 
     nsDocLoader*               mParent;                // [WEAK]
 
-    nsVoidArray                mListenerInfoList;
+    typedef nsAutoTObserverArray<nsListenerInfo, 8> ListenerArray;
+    ListenerArray              mListenerInfoList;
 
     nsCOMPtr<nsILoadGroup>        mLoadGroup;
     // We hold weak refs to all our kids
     nsTObserverArray<nsDocLoader*> mChildList;
 
     // The following member variables are related to the new nsIWebProgress 
     // feedback interfaces that travis cooked up.
     int32_t mProgressStateFlags;
@@ -299,18 +311,16 @@ private:
     
     // DocLoaderIsEmpty should be called whenever the docloader may be empty.
     // This method is idempotent and does nothing if the docloader is not in
     // fact empty.  This method _does_ make sure that layout is flushed if our
     // loadgroup has no active requests before checking for "real" emptiness if
     // aFlushLayout is true.
     void DocLoaderIsEmpty(bool aFlushLayout);
 
-    nsListenerInfo *GetListenerInfo(nsIWebProgressListener* aListener);
-
     int64_t GetMaxTotalProgress();
 
     nsresult AddRequestInfo(nsIRequest* aRequest);
     void RemoveRequestInfo(nsIRequest* aRequest);
     nsRequestInfo *GetRequestInfo(nsIRequest* aRequest);
     void ClearRequestInfoHash();
     int64_t CalculateMaxProgress();
     static PLDHashOperator CalcMaxProgressCallback(PLDHashTable* table,