Bug 1469914 - Prevent the HAL from registering duplicate observers; r…

…=froydnj This also replaces the custom logic in ObserverList with an nsTObserverArray which has all the necessary logic for stable iteration over a potentially changing list of items. Unused dependencies were also removed. --HG-- extra : source : 303478f7f248470a1c747f42dad9cb85c3129f0a
dothq · Jun 21, 2018 · 2d99f56 · 2d99f56
1 parent 9bf6837
commit 2d99f56
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 29 deletions.
diff --git a/hal/Hal.cpp b/hal/Hal.cpp
@@ -20,10 +20,7 @@
 #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"
 

diff --git a/hal/Hal.h b/hal/Hal.h
@@ -17,7 +17,6 @@
 #include "mozilla/hal_sandbox/PHal.h"
 #include "mozilla/HalScreenConfiguration.h"
 #include "mozilla/HalTypes.h"
-#include "mozilla/Observer.h"
 #include "mozilla/Types.h"
 
 /*

diff --git a/xpcom/ds/Observer.h b/xpcom/ds/Observer.h
@@ -7,7 +7,7 @@
 #ifndef mozilla_Observer_h
 #define mozilla_Observer_h
 
-#include "nsTArray.h"
+#include "nsTObserverArray.h"
 
 namespace mozilla {
 
@@ -48,7 +48,7 @@ class ObserverList
    */
   void AddObserver(Observer<T>* aObserver)
   {
-    mObservers.AppendElement(aObserver);
+    mObservers.AppendElementUnlessExists(aObserver);
   }
 
   /**
@@ -57,17 +57,7 @@ class ObserverList
    */
   bool RemoveObserver(Observer<T>* 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;
+    return mObservers.RemoveElement(aObserver);
   }
 
   uint32_t Length()
@@ -77,25 +67,18 @@ class ObserverList
 
   /**
    * Call Notify() on each item in the list.
-   * Handles the case of Notify() calling RemoveObserver()
    */
   void Broadcast(const T& 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);
-      }
+    typename nsTObserverArray<Observer<T>*>::ForwardIterator iter(mObservers);
+    while (iter.HasMore()) {
+      Observer<T>* obs = iter.GetNext();
+      obs->Notify(aParam);
     }
-    mBroadcastCopy.Clear();
   }
 
 protected:
-  nsTArray<Observer<T>*> mObservers;
-  nsTArray<Observer<T>*> mBroadcastCopy;
+  nsTObserverArray<Observer<T>*> mObservers;
 };
 
 } // namespace mozilla