Bug 1426245 - Allow partial removal of places oberver listeners r=mak
authorDoug Thayer <dothayer@mozilla.com>
Tue, 09 Oct 2018 14:45:58 +0000
changeset 496012 01f7327625291933b8886befa8fb561ced401b3d
parent 496011 b8c1b55829137acd018c4ff0ca763d5e1e1a89e9
child 496013 50ca67245a715b0e37014f2066102ad9feec9f1f
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1426245
milestone64.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 1426245 - Allow partial removal of places oberver listeners r=mak This was a bug in the original design that just hadn't shown up yet. If I were to listen to, say "notification-a", and then try to remove my listener for "notification-a" and "notification-b", we would end up with a dead listener staying in the list. This fixes that. MozReview-Commit-ID: KhYQSJaBDF9 Differential Revision: https://phabricator.services.mozilla.com/D4605
dom/base/PlacesObservers.cpp
--- a/dom/base/PlacesObservers.cpp
+++ b/dom/base/PlacesObservers.cpp
@@ -34,28 +34,28 @@ template <class T>
 using FlaggedArray = nsTArray<Flagged<T>>;
 
 template <class T>
 struct ListenerCollection
 {
   static StaticAutoPtr<FlaggedArray<T>> gListeners;
   static StaticAutoPtr<FlaggedArray<T>> gListenersToRemove;
 
-  static FlaggedArray<T>* GetListeners() {
+  static FlaggedArray<T>* GetListeners(bool aDoNotInit = false) {
     MOZ_ASSERT(NS_IsMainThread());
-    if (!gListeners) {
+    if (!gListeners && !aDoNotInit) {
       gListeners = new FlaggedArray<T>();
       ClearOnShutdown(&gListeners);
     }
     return gListeners;
   }
 
-  static FlaggedArray<T>* GetListenersToRemove() {
+  static FlaggedArray<T>* GetListenersToRemove(bool aDoNotInit = false) {
     MOZ_ASSERT(NS_IsMainThread());
-    if (!gListenersToRemove) {
+    if (!gListenersToRemove && !aDoNotInit) {
       gListenersToRemove = new FlaggedArray<T>();
       ClearOnShutdown(&gListenersToRemove);
     }
     return gListenersToRemove;
   }
 };
 
 template <class T>
@@ -221,67 +221,76 @@ PlacesObservers::RemoveListener(const ns
     RemoveListener(flags, aCallback);
   }
 }
 
 void
 PlacesObservers::RemoveListener(uint32_t aFlags,
                                 PlacesEventCallback& aCallback)
 {
-  FlaggedArray<RefPtr<PlacesEventCallback>>& listeners =
-    *JSListeners::GetListeners();
-  for (uint32_t i = 0; i < listeners.Length(); i++) {
-    Flagged<RefPtr<PlacesEventCallback>>& l = listeners[i];
+  FlaggedArray<RefPtr<PlacesEventCallback>>* listeners =
+    JSListeners::GetListeners(/* aDoNotInit: */ true);
+  if (!listeners) {
+    return;
+  }
+  for (uint32_t i = 0; i < listeners->Length(); i++) {
+    Flagged<RefPtr<PlacesEventCallback>>& l = listeners->ElementAt(i);
     if (!(*l.value == aCallback)) {
       continue;
     }
-    if (l.flags == aFlags) {
-      listeners.RemoveElementAt(i);
+    if (l.flags == (aFlags & l.flags)) {
+      listeners->RemoveElementAt(i);
       i--;
     } else {
       l.flags &= ~aFlags;
     }
   }
 }
 
 void
 PlacesObservers::RemoveListener(uint32_t aFlags,
                                 PlacesWeakCallbackWrapper& aCallback)
 {
-  FlaggedArray<WeakPtr<PlacesWeakCallbackWrapper>>& listeners =
-    *WeakJSListeners::GetListeners();
-  for (uint32_t i = 0; i < listeners.Length(); i++) {
-    Flagged<WeakPtr<PlacesWeakCallbackWrapper>>& l = listeners[i];
+  FlaggedArray<WeakPtr<PlacesWeakCallbackWrapper>>* listeners =
+    WeakJSListeners::GetListeners(/* aDoNotInit: */ true);
+  if (!listeners) {
+    return;
+  }
+  for (uint32_t i = 0; i < listeners->Length(); i++) {
+    Flagged<WeakPtr<PlacesWeakCallbackWrapper>>& l = listeners->ElementAt(i);
     RefPtr<PlacesWeakCallbackWrapper> unwrapped = l.value.get();
     if (unwrapped != &aCallback) {
       continue;
     }
-    if (l.flags == aFlags) {
-      listeners.RemoveElementAt(i);
+    if (l.flags == (aFlags & l.flags)) {
+      listeners->RemoveElementAt(i);
       i--;
     } else {
       l.flags &= ~aFlags;
     }
   }
 }
 
 void
 PlacesObservers::RemoveListener(uint32_t aFlags,
                                 places::INativePlacesEventCallback* aCallback)
 {
-  FlaggedArray<WeakPtr<places::INativePlacesEventCallback>>& listeners =
-    *WeakNativeListeners::GetListeners();
-  for (uint32_t i = 0; i < listeners.Length(); i++) {
-    Flagged<WeakPtr<places::INativePlacesEventCallback>>& l = listeners[i];
+  FlaggedArray<WeakPtr<places::INativePlacesEventCallback>>* listeners =
+    WeakNativeListeners::GetListeners(/* aDoNotInit: */ true);
+  if (!listeners) {
+    return;
+  }
+  for (uint32_t i = 0; i < listeners->Length(); i++) {
+    Flagged<WeakPtr<places::INativePlacesEventCallback>>& l = listeners->ElementAt(i);
     RefPtr<places::INativePlacesEventCallback> unwrapped = l.value.get();
     if (unwrapped != aCallback) {
       continue;
     }
-    if (l.flags == aFlags) {
-      listeners.RemoveElementAt(i);
+    if (l.flags == (aFlags & l.flags)) {
+      listeners->RemoveElementAt(i);
       i--;
     } else {
       l.flags &= ~aFlags;
     }
   }
 }
 
 void