Bug 1469914 - Prevent the HAL from registering duplicate observers; r=froydnj a=lizzard
authorGabriele Svelto <gsvelto@mozilla.com>
Fri, 22 Jun 2018 00:35:08 +0200
changeset 477828 3da147dac512da9368fbe9179c5ad72dc8c759e4
parent 477827 eb0b435eba5b60040f7273d55acd29d7ee1fc3f5
child 477829 0915e733540199880a1529f7c0cb0b9a4fef09ae
push id9441
push userarchaeopteryx@coole-files.de
push dateFri, 06 Jul 2018 15:09:28 +0000
treeherdermozilla-beta@3a7a791ef4ff [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, lizzard
bugs1469914
milestone62.0
Bug 1469914 - Prevent the HAL from registering duplicate observers; r=froydnj a=lizzard 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.
hal/Hal.cpp
hal/Hal.h
xpcom/ds/Observer.h
--- a/hal/Hal.cpp
+++ b/hal/Hal.cpp
@@ -15,20 +15,17 @@
 #include "nsITabChild.h"
 #include "nsIWebNavigation.h"
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "nsPIDOMWindow.h"
 #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"
 
 #ifdef XP_WIN
 #include <process.h>
 #define getpid _getpid
 #endif
 
--- a/hal/Hal.h
+++ b/hal/Hal.h
@@ -12,17 +12,16 @@
 #include "nsTArray.h"
 #include "mozilla/dom/battery/Types.h"
 #include "mozilla/dom/network/Types.h"
 #include "mozilla/dom/power/Types.h"
 #include "mozilla/dom/ScreenOrientation.h"
 #include "mozilla/hal_sandbox/PHal.h"
 #include "mozilla/HalScreenConfiguration.h"
 #include "mozilla/HalTypes.h"
-#include "mozilla/Observer.h"
 #include "mozilla/Types.h"
 
 /*
  * Hal.h contains the public Hal API.
  *
  * By default, this file defines its functions in the hal namespace, but if
  * MOZ_HAL_NAMESPACE is defined, we'll define our functions in that namespace.
  *
--- a/xpcom/ds/Observer.h
+++ b/xpcom/ds/Observer.h
@@ -2,17 +2,17 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_Observer_h
 #define mozilla_Observer_h
 
-#include "nsTArray.h"
+#include "nsTObserverArray.h"
 
 namespace mozilla {
 
 /**
  * Observer<T> provides a way for a class to observe something.
  * When an event has to be broadcasted to all Observer<T>, Notify() method
  * is called.
  * T represents the type of the object passed in argument to Notify().
@@ -43,61 +43,44 @@ public:
    * Note: When calling AddObserver, it's up to the caller to make sure the
    * object isn't going to be release as long as RemoveObserver hasn't been
    * called.
    *
    * @see RemoveObserver()
    */
   void AddObserver(Observer<T>* aObserver)
   {
-    mObservers.AppendElement(aObserver);
+    mObservers.AppendElementUnlessExists(aObserver);
   }
 
   /**
    * Remove the observer from the observer list.
    * @return Whether the observer has been found in the list.
    */
   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()
   {
     return mObservers.Length();
   }
 
   /**
    * 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
 
 #endif // mozilla_Observer_h