CallbackList: Add support for removing receivers

Bug: webrtc:11943
Change-Id: I7a646729dd1e4f5abe20900412f4105414e1a98f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195332
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32700}

rtc_base/callback_list.cc

@@ -21,16 +21,75 @@ CallbackListReceivers::~CallbackListReceivers() {
   RTC_CHECK(!send_in_progress_);
 }
 
+void CallbackListReceivers::RemoveReceivers(const void* removal_tag) {
+  RTC_CHECK(!send_in_progress_);
+  RTC_DCHECK(removal_tag != nullptr);
+
+  // We divide the receivers_ vector into three regions: from right to left, the
+  // "keep" region, the "todo" region, and the "remove" region. The "todo"
+  // region initially covers the whole vector.
+  size_t first_todo = 0;                    // First element of the "todo"
+                                            // region.
+  size_t first_remove = receivers_.size();  // First element of the "remove"
+                                            // region.
+
+  // Loop until the "todo" region is empty.
+  while (first_todo != first_remove) {
+    if (receivers_[first_todo].removal_tag != removal_tag) {
+      // The first element of the "todo" region should be kept. Move the
+      // "keep"/"todo" boundary.
+      ++first_todo;
+    } else if (receivers_[first_remove - 1].removal_tag == removal_tag) {
+      // The last element of the "todo" region should be removed. Move the
+      // "todo"/"remove" boundary.
+      --first_remove;
+    } else {
+      // The first element of the "todo" region should be removed, and the last
+      // element of the "todo" region should be kept. Swap them, and then shrink
+      // the "todo" region from both ends.
+      RTC_DCHECK_NE(first_todo, first_remove - 1);
+      using std::swap;
+      swap(receivers_[first_todo], receivers_[first_remove - 1]);
+      RTC_DCHECK_NE(receivers_[first_todo].removal_tag, removal_tag);
+      ++first_todo;
+      RTC_DCHECK_EQ(receivers_[first_remove - 1].removal_tag, removal_tag);
+      --first_remove;
+    }
+  }
+
+  // Discard the remove region.
+  receivers_.resize(first_remove);
+}
+
 void CallbackListReceivers::Foreach(
     rtc::FunctionView<void(UntypedFunction&)> fv) {
   RTC_CHECK(!send_in_progress_);
   send_in_progress_ = true;
   for (auto& r : receivers_) {
-    fv(r);
+    fv(r.function);
   }
   send_in_progress_ = false;
 }
 
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<1>);
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<2>);
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<3>);
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<4>);
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::NontrivialUntypedFunctionArgs);
+template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::FunctionPointerUntypedFunctionArgs);
+
 template void CallbackListReceivers::AddReceiver(
     UntypedFunction::TrivialUntypedFunctionArgs<1>);
 template void CallbackListReceivers::AddReceiver(

rtc_base/callback_list.h

@@ -32,19 +32,52 @@ class CallbackListReceivers {
   CallbackListReceivers& operator=(CallbackListReceivers&&) = delete;
   ~CallbackListReceivers();
 
+  template <typename UntypedFunctionArgsT>
+  RTC_NO_INLINE void AddReceiver(const void* removal_tag,
+                                 UntypedFunctionArgsT args) {
+    RTC_CHECK(!send_in_progress_);
+    RTC_DCHECK(removal_tag != nullptr);
+    receivers_.push_back({removal_tag, UntypedFunction::Create(args)});
+  }
+
   template <typename UntypedFunctionArgsT>
   RTC_NO_INLINE void AddReceiver(UntypedFunctionArgsT args) {
     RTC_CHECK(!send_in_progress_);
-    receivers_.push_back(UntypedFunction::Create(args));
+    receivers_.push_back({nullptr, UntypedFunction::Create(args)});
   }
 
+  void RemoveReceivers(const void* removal_tag);
+
   void Foreach(rtc::FunctionView<void(UntypedFunction&)> fv);
 
  private:
-  std::vector<UntypedFunction> receivers_;
+  struct Callback {
+    const void* removal_tag;
+    UntypedFunction function;
+  };
+  std::vector<Callback> receivers_;
   bool send_in_progress_ = false;
 };
 
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<1>);
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<2>);
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<3>);
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::TrivialUntypedFunctionArgs<4>);
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::NontrivialUntypedFunctionArgs);
+extern template void CallbackListReceivers::AddReceiver(
+    const void*,
+    UntypedFunction::FunctionPointerUntypedFunctionArgs);
+
 extern template void CallbackListReceivers::AddReceiver(
     UntypedFunction::TrivialUntypedFunctionArgs<1>);
 extern template void CallbackListReceivers::AddReceiver(
@@ -125,11 +158,6 @@ extern template void CallbackListReceivers::AddReceiver(
 //     foo_callbacks_.AddReceiver(std::forward<F>(callback));
 //   }
 //
-// Removing callbacks
-// ------------------
-//
-// TODO(kwiberg): The current design doesn’t support removing callbacks, only
-// adding them, but removal support can easily be added.
 template <typename... ArgT>
 class CallbackList {
  public:
@@ -141,16 +169,35 @@ class CallbackList {
 
   // Adds a new receiver. The receiver (a callable object or a function pointer)
   // must be movable, but need not be copyable. Its call signature should be
-  // `void(ArgT...)`.
+  // `void(ArgT...)`. The removal tag is a pointer to an arbitrary object that
+  // you own, and that will stay alive until the CallbackList is gone, or until
+  // all receivers using it as a removal tag have been removed; you can use it
+  // to remove the receiver.
+  template <typename F>
+  void AddReceiver(const void* removal_tag, F&& f) {
+    receivers_.AddReceiver(
+        removal_tag,
+        UntypedFunction::PrepareArgs<void(ArgT...)>(std::forward<F>(f)));
+  }
+
+  // Adds a new receiver with no removal tag.
   template <typename F>
   void AddReceiver(F&& f) {
     receivers_.AddReceiver(
         UntypedFunction::PrepareArgs<void(ArgT...)>(std::forward<F>(f)));
   }
 
+  // Removes all receivers that were added with the given removal tag.
+  void RemoveReceivers(const void* removal_tag) {
+    receivers_.RemoveReceivers(removal_tag);
+  }
+
   // Calls all receivers with the given arguments. While the Send is in
   // progress, no method calls are allowed; specifically, this means that the
   // callbacks may not do anything with this CallbackList instance.
+  //
+  // Note: Receivers are called serially, but not necessarily in the same order
+  // they were added.
   template <typename... ArgU>
   void Send(ArgU&&... args) {
     receivers_.Foreach([&](UntypedFunction& f) {

rtc_base/callback_list_unittest.cc

@@ -207,8 +207,53 @@ TEST(CallbackList, MemberFunctionTest) {
 
   EXPECT_EQ(index, 2);
 }
+
 // todo(glahiru): Add a test case to catch some error for Karl's first fix
 // todo(glahiru): Add a test for rtc::Bind
 // which used the following code in the Send
+
+TEST(CallbackList, RemoveOneReceiver) {
+  int removal_tag[2];
+  CallbackList<> c;
+  int accumulator = 0;
+  c.AddReceiver([&accumulator] { accumulator += 1; });
+  c.AddReceiver(&removal_tag[0], [&accumulator] { accumulator += 10; });
+  c.AddReceiver(&removal_tag[1], [&accumulator] { accumulator += 100; });
+  c.Send();
+  EXPECT_EQ(accumulator, 111);
+  c.RemoveReceivers(&removal_tag[0]);
+  c.Send();
+  EXPECT_EQ(accumulator, 212);
+}
+
+TEST(CallbackList, RemoveZeroReceivers) {
+  int removal_tag[3];
+  CallbackList<> c;
+  int accumulator = 0;
+  c.AddReceiver([&accumulator] { accumulator += 1; });
+  c.AddReceiver(&removal_tag[0], [&accumulator] { accumulator += 10; });
+  c.AddReceiver(&removal_tag[1], [&accumulator] { accumulator += 100; });
+  c.Send();
+  EXPECT_EQ(accumulator, 111);
+  c.RemoveReceivers(&removal_tag[2]);
+  c.Send();
+  EXPECT_EQ(accumulator, 222);
+}
+
+TEST(CallbackList, RemoveManyReceivers) {
+  int removal_tag;
+  CallbackList<> c;
+  int accumulator = 0;
+  c.AddReceiver([&accumulator] { accumulator += 1; });
+  c.AddReceiver(&removal_tag, [&accumulator] { accumulator += 10; });
+  c.AddReceiver([&accumulator] { accumulator += 100; });
+  c.AddReceiver(&removal_tag, [&accumulator] { accumulator += 1000; });
+  c.Send();
+  EXPECT_EQ(accumulator, 1111);
+  c.RemoveReceivers(&removal_tag);
+  c.Send();
+  EXPECT_EQ(accumulator, 1212);
+}
+
 }  // namespace
 }  // namespace webrtc