Bug 1649228 - Fix ContentChild::RecvNotifyAlertsObserver to notify after, not while, removing observers from the vector. r=froydnj, a=tritter

This probably had logic issues before bug 1642991, but not security
issues (at worst, an array out of bounds which is a release assertion
that would crash the process in a safe way).

Differential Revision: https://phabricator.services.mozilla.com/D81596

dom/ipc/ContentChild.cpp

@@ -415,18 +415,6 @@ class AlertObserver {
 
   ~AlertObserver() = default;
 
-  bool ShouldRemoveFrom(nsIObserver* aObserver, const nsString& aData) const {
-    return (mObserver == aObserver && mData == aData);
-  }
-
-  bool Observes(const nsString& aData) const { return mData.Equals(aData); }
-
-  bool Notify(const nsCString& aType) const {
-    mObserver->Observe(nullptr, aType.get(), mData.get());
-    return true;
-  }
-
- private:
   nsCOMPtr<nsIObserver> mObserver;
   nsString mData;
 };
@@ -2270,13 +2258,23 @@ mozilla::ipc::IPCResult ContentChild::RecvDataStorageClear(
 
 mozilla::ipc::IPCResult ContentChild::RecvNotifyAlertsObserver(
     const nsCString& aType, const nsString& aData) {
-  mAlertObservers.RemoveElementsBy([&aData, &aType](const auto& observer) {
-    const bool notified = observer->Observes(aData) && observer->Notify(aType);
-    // if the observer was notified and aType == alertfinished, this alert is
-    // done.  we can remove the observer.
-    return notified && aType.EqualsLiteral("alertfinished");
+  nsTArray<nsCOMPtr<nsIObserver>> observersToNotify;
+
+  mAlertObservers.RemoveElementsBy([&](UniquePtr<AlertObserver>& observer) {
+    if (!observer->mData.Equals(aData)) {
+      return false;
+    }
+
+    // aType == alertfinished, this alert is done and we can remove the
+    // observer.
+    observersToNotify.AppendElement(observer->mObserver);
+    return aType.EqualsLiteral("alertfinished");
   });
 
+  for (auto& observer : observersToNotify) {
+    observer->Observe(nullptr, aType.get(), aData.get());
+  }
+
   return IPC_OK();
 }