Backed out changeset cd857f86810c (bug 1469914) for causing leaks on multiple tests. CLOSED TREE

hal/Hal.cpp

@@ -20,7 +20,10 @@
 #include "nsJSUtils.h"
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/Observer.h"
+#include "mozilla/Services.h"
+#include "mozilla/StaticPtr.h"
 #include "mozilla/dom/ContentChild.h"
+#include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/ScreenOrientation.h"
 #include "WindowIdentifier.h"
 
@@ -180,28 +183,43 @@ class ObserversManager
 {
 public:
   void AddObserver(Observer<InfoType>* aObserver) {
-    mObservers.AddObserver(aObserver);
+    if (!mObservers) {
+      mObservers = new mozilla::ObserverList<InfoType>();
+    }
 
-    if (mObservers.Length() == 1) {
+    mObservers->AddObserver(aObserver);
+
+    if (mObservers->Length() == 1) {
       EnableNotifications();
     }
   }
 
   void RemoveObserver(Observer<InfoType>* aObserver) {
-    bool removed = mObservers.RemoveObserver(aObserver);
+    bool removed = mObservers && mObservers->RemoveObserver(aObserver);
     if (!removed) {
       return;
     }
 
-    if (mObservers.Length() == 0) {
+    if (mObservers->Length() == 0) {
       DisableNotifications();
 
       OnNotificationsDisabled();
+
+      delete mObservers;
+      mObservers = nullptr;
     }
   }
 
   void BroadcastInformation(const InfoType& aInfo) {
-    mObservers.Broadcast(aInfo);
+    // It is possible for mObservers to be nullptr here on some platforms,
+    // because a call to BroadcastInformation gets queued up asynchronously
+    // while RemoveObserver is running (and before the notifications are
+    // disabled). The queued call can then get run after mObservers has
+    // been nulled out. See bug 757025.
+    if (!mObservers) {
+      return;
+    }
+    mObservers->Broadcast(aInfo);
   }
 
 protected:
@@ -210,7 +228,7 @@ protected:
   virtual void OnNotificationsDisabled() {}
 
 private:
-  mozilla::ObserverList<InfoType> mObservers;
+  mozilla::ObserverList<InfoType>* mObservers;
 };
 
 template <class InfoType>

hal/Hal.h

@@ -17,6 +17,7 @@
 #include "mozilla/hal_sandbox/PHal.h"
 #include "mozilla/HalScreenConfiguration.h"
 #include "mozilla/HalTypes.h"
+#include "mozilla/Observer.h"
 #include "mozilla/Types.h"
 
 /*

xpcom/ds/Observer.h

@@ -7,7 +7,7 @@
 #ifndef mozilla_Observer_h
 #define mozilla_Observer_h
 
-#include "nsTObserverArray.h"
+#include "nsTArray.h"
 
 namespace mozilla {
 
@@ -48,7 +48,7 @@ public:
    */
   void AddObserver(Observer<T>* aObserver)
   {
-    mObservers.AppendElementUnlessExists(aObserver);
+    mObservers.AppendElement(aObserver);
   }
 
   /**
@@ -57,7 +57,17 @@ public:
    */
   bool RemoveObserver(Observer<T>* aObserver)
   {
-    return mObservers.RemoveElement(aObserver);
+    if (mObservers.RemoveElement(aObserver)) {
+      if (!mBroadcastCopy.IsEmpty()) {
+        // Annoyingly, someone could RemoveObserver() an item on the list
+        // while we're in a Broadcast()'s Notify() call.
+        auto i = mBroadcastCopy.IndexOf(aObserver);
+        MOZ_ASSERT(i != mBroadcastCopy.NoIndex);
+        mBroadcastCopy[i] = nullptr;
+      }
+      return true;
+    }
+    return false;
   }
 
   uint32_t Length()
@@ -67,18 +77,25 @@ public:
 
   /**
    * Call Notify() on each item in the list.
+   * Handles the case of Notify() calling RemoveObserver()
    */
   void Broadcast(const T& aParam)
   {
-    typename nsTObserverArray<Observer<T>*>::ForwardIterator iter(mObservers);
-    while (iter.HasMore()) {
-      Observer<T>* obs = iter.GetNext();
-      obs->Notify(aParam);
+    MOZ_ASSERT(mBroadcastCopy.IsEmpty());
+    mBroadcastCopy = mObservers;
+    uint32_t size = mBroadcastCopy.Length();
+    for (uint32_t i = 0; i < size; ++i) {
+      // nulled if Removed during Broadcast
+      if (mBroadcastCopy[i]) {
+        mBroadcastCopy[i]->Notify(aParam);
+      }
     }
+    mBroadcastCopy.Clear();
   }
 
 protected:
-  nsTObserverArray<Observer<T>*> mObservers;
+  nsTArray<Observer<T>*> mObservers;
+  nsTArray<Observer<T>*> mBroadcastCopy;
 };
 
 } // namespace mozilla