Bug 933070, MessageManager should use nsTObserverArray, r=fabrice
authorOlli Pettay <Olli.Pettay@helsinki.fi>
Tue, 05 Nov 2013 13:52:04 +0200
changeset 168181 070eda49057771f25413371a71d15793216eaefa
parent 168180 cbd97422f174c7a5fcef097886ca5f9feebb9a79
child 168182 d1ea27ce8789a308fd3483413686c13ba358b968
push id3224
push userlsblakk@mozilla.com
push dateTue, 04 Feb 2014 01:06:49 +0000
treeherdermozilla-beta@60c04d0987f1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice
bugs933070
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 933070, MessageManager should use nsTObserverArray, r=fabrice
content/base/src/nsFrameMessageManager.cpp
content/base/src/nsFrameMessageManager.h
toolkit/modules/tests/browser/browser_Finder.js
--- a/content/base/src/nsFrameMessageManager.cpp
+++ b/content/base/src/nsFrameMessageManager.cpp
@@ -48,25 +48,25 @@
 #endif
 
 using namespace mozilla;
 using namespace mozilla::dom;
 using namespace mozilla::dom::ipc;
 
 static PLDHashOperator
 CycleCollectorTraverseListeners(const nsAString& aKey,
-                                nsTArray<nsMessageListenerInfo>* aListeners,
+                                nsAutoTObserverArray<nsMessageListenerInfo, 1>* aListeners,
                                 void* aCb)
 {
   nsCycleCollectionTraversalCallback* cb =
     static_cast<nsCycleCollectionTraversalCallback*> (aCb);
   uint32_t count = aListeners->Length();
   for (uint32_t i = 0; i < count; ++i) {
     NS_CYCLE_COLLECTION_NOTE_EDGE_NAME(*cb, "listeners[i] mStrongListener");
-    cb->NoteXPCOMChild((*aListeners)[i].mStrongListener.get());
+    cb->NoteXPCOMChild(aListeners->ElementAt(i).mStrongListener.get());
   }
   return PL_DHASH_NEXT;
 }
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(nsFrameMessageManager)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(nsFrameMessageManager)
   tmp->mListeners.EnumerateRead(CycleCollectorTraverseListeners,
@@ -253,78 +253,80 @@ SameProcessCpowHolder::ToObject(JSContex
 }
 
 // nsIMessageListenerManager
 
 NS_IMETHODIMP
 nsFrameMessageManager::AddMessageListener(const nsAString& aMessage,
                                           nsIMessageListener* aListener)
 {
-  nsTArray<nsMessageListenerInfo>* listeners = mListeners.Get(aMessage);
+  nsAutoTObserverArray<nsMessageListenerInfo, 1>* listeners =
+    mListeners.Get(aMessage);
   if (!listeners) {
-    listeners = new nsTArray<nsMessageListenerInfo>();
+    listeners = new nsAutoTObserverArray<nsMessageListenerInfo, 1>();
     mListeners.Put(aMessage, listeners);
   } else {
     uint32_t len = listeners->Length();
     for (uint32_t i = 0; i < len; ++i) {
-      if ((*listeners)[i].mStrongListener == aListener) {
+      if (listeners->ElementAt(i).mStrongListener == aListener) {
         return NS_OK;
       }
     }
   }
 
   nsMessageListenerInfo* entry = listeners->AppendElement();
   NS_ENSURE_TRUE(entry, NS_ERROR_OUT_OF_MEMORY);
   entry->mStrongListener = aListener;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFrameMessageManager::RemoveMessageListener(const nsAString& aMessage,
                                              nsIMessageListener* aListener)
 {
-  nsTArray<nsMessageListenerInfo>* listeners = mListeners.Get(aMessage);
+  nsAutoTObserverArray<nsMessageListenerInfo, 1>* listeners =
+    mListeners.Get(aMessage);
   if (!listeners) {
     return NS_OK;
   }
 
   uint32_t len = listeners->Length();
   for (uint32_t i = 0; i < len; ++i) {
-    if ((*listeners)[i].mStrongListener == aListener) {
+    if (listeners->ElementAt(i).mStrongListener == aListener) {
       listeners->RemoveElementAt(i);
       return NS_OK;
     }
   }
   return NS_OK;
 }
 
 #ifdef DEBUG
 typedef struct
 {
   nsCOMPtr<nsISupports> mCanonical;
   nsWeakPtr mWeak;
 } CanonicalCheckerParams;
 
 static PLDHashOperator
 CanonicalChecker(const nsAString& aKey,
-                 nsTArray<nsMessageListenerInfo>* aListeners,
+                 nsAutoTObserverArray<nsMessageListenerInfo, 1>* aListeners,
                  void* aParams)
 {
   CanonicalCheckerParams* params =
     static_cast<CanonicalCheckerParams*> (aParams);
 
   uint32_t count = aListeners->Length();
   for (uint32_t i = 0; i < count; i++) {
-    if (!(*aListeners)[i].mWeakListener) {
+    if (!aListeners->ElementAt(i).mWeakListener) {
       continue;
     }
     nsCOMPtr<nsISupports> otherCanonical =
-      do_QueryReferent((*aListeners)[i].mWeakListener);
+      do_QueryReferent(aListeners->ElementAt(i).mWeakListener);
     MOZ_ASSERT((params->mCanonical == otherCanonical) ==
-               (params->mWeak == (*aListeners)[i].mWeakListener));
+               (params->mWeak == aListeners->ElementAt(i).mWeakListener));
   }
   return PL_DHASH_NEXT;
 }
 #endif
 
 NS_IMETHODIMP
 nsFrameMessageManager::AddWeakMessageListener(const nsAString& aMessage,
                                               nsIMessageListener* aListener)
@@ -339,24 +341,25 @@ nsFrameMessageManager::AddWeakMessageLis
   // check that we're not getting ourselves into that situation.
   nsCOMPtr<nsISupports> canonical = do_QueryInterface(aListener);
   CanonicalCheckerParams params;
   params.mCanonical = canonical;
   params.mWeak = weak;
   mListeners.EnumerateRead(CanonicalChecker, (void*)&params);
 #endif
 
-  nsTArray<nsMessageListenerInfo>* listeners = mListeners.Get(aMessage);
+  nsAutoTObserverArray<nsMessageListenerInfo, 1>* listeners =
+    mListeners.Get(aMessage);
   if (!listeners) {
-    listeners = new nsTArray<nsMessageListenerInfo>();
+    listeners = new nsAutoTObserverArray<nsMessageListenerInfo, 1>();
     mListeners.Put(aMessage, listeners);
   } else {
     uint32_t len = listeners->Length();
     for (uint32_t i = 0; i < len; ++i) {
-      if ((*listeners)[i].mWeakListener == weak) {
+      if (listeners->ElementAt(i).mWeakListener == weak) {
         return NS_OK;
       }
     }
   }
 
   nsMessageListenerInfo* entry = listeners->AppendElement();
   entry->mWeakListener = weak;
   return NS_OK;
@@ -364,24 +367,25 @@ nsFrameMessageManager::AddWeakMessageLis
 
 NS_IMETHODIMP
 nsFrameMessageManager::RemoveWeakMessageListener(const nsAString& aMessage,
                                                  nsIMessageListener* aListener)
 {
   nsWeakPtr weak = do_GetWeakReference(aListener);
   NS_ENSURE_TRUE(weak, NS_OK);
 
-  nsTArray<nsMessageListenerInfo>* listeners = mListeners.Get(aMessage);
+  nsAutoTObserverArray<nsMessageListenerInfo, 1>* listeners =
+    mListeners.Get(aMessage);
   if (!listeners) {
     return NS_OK;
   }
 
   uint32_t len = listeners->Length();
   for (uint32_t i = 0; i < len; ++i) {
-    if ((*listeners)[i].mWeakListener == weak) {
+    if (listeners->ElementAt(i).mWeakListener == weak) {
       listeners->RemoveElementAt(i);
       return NS_OK;
     }
   }
 
   return NS_OK;
 }
 
@@ -835,37 +839,41 @@ nsresult
 nsFrameMessageManager::ReceiveMessage(nsISupports* aTarget,
                                       const nsAString& aMessage,
                                       bool aIsSync,
                                       const StructuredCloneData* aCloneData,
                                       CpowHolder* aCpows,
                                       InfallibleTArray<nsString>* aJSONRetVal)
 {
   AutoSafeJSContext ctx;
-  nsTArray<nsMessageListenerInfo>* listeners = mListeners.Get(aMessage);
+  nsAutoTObserverArray<nsMessageListenerInfo, 1>* listeners =
+    mListeners.Get(aMessage);
   if (listeners) {
 
     MMListenerRemover lr(this);
 
-    for (uint32_t i = 0; i < listeners->Length(); ++i) {
+    nsAutoTObserverArray<nsMessageListenerInfo, 1>::EndLimitedIterator
+      iter(*listeners);
+    while(iter.HasMore()) {
+      nsMessageListenerInfo& listener = iter.GetNext();
       // Remove mListeners[i] if it's an expired weak listener.
       nsCOMPtr<nsISupports> weakListener;
-      if ((*listeners)[i].mWeakListener) {
-        weakListener = do_QueryReferent((*listeners)[i].mWeakListener);
+      if (listener.mWeakListener) {
+        weakListener = do_QueryReferent(listener.mWeakListener);
         if (!weakListener) {
-          listeners->RemoveElementAt(i--);
+          listeners->RemoveElement(listener);
           continue;
         }
       }
 
       nsCOMPtr<nsIXPConnectWrappedJS> wrappedJS;
       if (weakListener) {
         wrappedJS = do_QueryInterface(weakListener);
       } else {
-        wrappedJS = do_QueryInterface((*listeners)[i].mStrongListener);
+        wrappedJS = do_QueryInterface(listener.mStrongListener);
       }
 
       if (!wrappedJS) {
         continue;
       }
       JS::Rooted<JSObject*> object(ctx, wrappedJS->GetJSObject());
       if (!object) {
         continue;
@@ -1083,17 +1091,17 @@ protected:
   void CountReferents(nsFrameMessageManager* aMessageManager,
                       MessageManagerReferentCount* aReferentCount);
 };
 
 NS_IMPL_ISUPPORTS1(MessageManagerReporter, nsIMemoryReporter)
 
 static PLDHashOperator
 CollectMessageListenerData(const nsAString& aKey,
-                           nsTArray<nsMessageListenerInfo>* aListeners,
+                           nsAutoTObserverArray<nsMessageListenerInfo, 1>* aListeners,
                            void* aData)
 {
   MessageManagerReferentCount* referentCount =
     static_cast<MessageManagerReferentCount*>(aData);
 
   uint32_t listenerCount = aListeners->Length();
   if (!listenerCount) {
     return PL_DHASH_NEXT;
@@ -1107,17 +1115,18 @@ CollectMessageListenerData(const nsAStri
 
   // Keep track of messages that have a suspiciously large
   // number of referents (symptom of leak).
   if (currentCount == MessageManagerReporter::kSuspectReferentCount) {
     referentCount->mSuspectMessages.AppendElement(key);
   }
 
   for (uint32_t i = 0; i < listenerCount; ++i) {
-    const nsMessageListenerInfo& listenerInfo = (*aListeners)[i];
+    const nsMessageListenerInfo& listenerInfo =
+      aListeners->ElementAt(i);
     if (listenerInfo.mWeakListener) {
       nsCOMPtr<nsISupports> referent =
         do_QueryReferent(listenerInfo.mWeakListener);
       if (referent) {
         referentCount->mWeakAlive++;
       } else {
         referentCount->mWeakDead++;
       }
@@ -1774,23 +1783,23 @@ NS_NewChildProcessMessageManager(nsISync
                                                         nullptr,
                                                         MM_PROCESSMANAGER | MM_OWNSCALLBACK);
   nsFrameMessageManager::sChildProcessManager = mm;
   return CallQueryInterface(mm, aResult);
 }
 
 static PLDHashOperator
 CycleCollectorMarkListeners(const nsAString& aKey,
-                            nsTArray<nsMessageListenerInfo>* aListeners,
+                            nsAutoTObserverArray<nsMessageListenerInfo, 1>* aListeners,
                             void* aData)
 {
   uint32_t count = aListeners->Length();
   for (uint32_t i = 0; i < count; i++) {
-    if ((*aListeners)[i].mStrongListener) {
-      xpc_TryUnmarkWrappedGrayObject((*aListeners)[i].mStrongListener);
+    if (aListeners->ElementAt(i).mStrongListener) {
+      xpc_TryUnmarkWrappedGrayObject(aListeners->ElementAt(i).mStrongListener);
     }
   }
   return PL_DHASH_NEXT;
 }
 
 bool
 nsFrameMessageManager::MarkForCC()
 {
--- a/content/base/src/nsFrameMessageManager.h
+++ b/content/base/src/nsFrameMessageManager.h
@@ -20,16 +20,17 @@
 #include "nsDataHashtable.h"
 #include "nsClassHashtable.h"
 #include "mozilla/Services.h"
 #include "nsIObserverService.h"
 #include "nsThreadUtils.h"
 #include "nsWeakPtr.h"
 #include "mozilla/Attributes.h"
 #include "js/RootingAPI.h"
+#include "nsTObserverArray.h"
 
 namespace mozilla {
 namespace dom {
 
 class ContentParent;
 class ContentChild;
 struct StructuredCloneData;
 class ClonedMessageData;
@@ -109,16 +110,21 @@ StructuredCloneData UnpackClonedMessageD
 } // namespace ipc
 } // namespace dom
 } // namespace mozilla
 
 class nsAXPCNativeCallContext;
 
 struct nsMessageListenerInfo
 {
+  bool operator==(const nsMessageListenerInfo& aOther) const
+  {
+    return &aOther == this;
+  }
+
   // Exactly one of mStrongListener and mWeakListener must be non-null.
   nsCOMPtr<nsIMessageListener> mStrongListener;
   nsWeakPtr mWeakListener;
 };
 
 class CpowHolder
 {
   public:
@@ -265,17 +271,18 @@ private:
                        JSContext* aCx,
                        uint8_t aArgc,
                        JS::Value* aRetval,
                        bool aIsSync);
 protected:
   friend class MMListenerRemover;
   // We keep the message listeners as arrays in a hastable indexed by the
   // message name. That gives us fast lookups in ReceiveMessage().
-  nsClassHashtable<nsStringHashKey, nsTArray<nsMessageListenerInfo>> mListeners;
+  nsClassHashtable<nsStringHashKey,
+                   nsAutoTObserverArray<nsMessageListenerInfo, 1>> mListeners;
   nsCOMArray<nsIContentFrameMessageManager> mChildManagers;
   bool mChrome;     // true if we're in the chrome process
   bool mGlobal;     // true if we're the global frame message manager
   bool mIsProcessManager; // true if the message manager belongs to the process realm
   bool mIsBroadcaster; // true if the message manager is a broadcaster
   bool mOwnsCallback;
   bool mHandlingMessage;
   bool mDisconnected;
--- a/toolkit/modules/tests/browser/browser_Finder.js
+++ b/toolkit/modules/tests/browser/browser_Finder.js
@@ -46,27 +46,28 @@ function startTests () {
 
         browser.messageManager.addMessageListener("OutlineTest", function f(aMessage) {
           browser.messageManager.removeMessageListener("OutlineTest", f);
 
 
           if (first) {
             ok(aMessage.data.ok, "content script should send okay");
             first = false;
+
+            // Just a simple search for "test link".
+            finder.fastFind("test link", false, false);
           } else {
             ok(!aMessage.data.ok, "content script should not send okay");
             cleanup();
           }
         })
         browser.messageManager.loadFrameScript("data:," + outlineTest, false)
       }
       // Search only for links and draw outlines.
       finder.fastFind("test link", true, true);
-      // Just a simple search for "test link".
-      finder.fastFind("test link", false, false);
     }
     finder.highlight(true, "Bla");
   }
   finder.highlight(true, "content");
 }
 
 function cleanup() {
   gBrowser.removeTab(tab);