Cleanup listener callbacks on DocumentReference and Query.

This CL is the last in a series described in cl/312713181, cl/317397413, and resolves b/156024690.

In this CL the new pattern for managing listener callbacks implemented in `FirebaseFirestore.ListenForSnapshotsInSync()` is being applied to all other places where we expose listeners (i.e. `Query` and `DocumentReference`).

PiperOrigin-RevId: 319089024

Author: Googler

firestore/src/include/firebase/csharp/document_event_listener.cc

@@ -7,66 +7,27 @@ namespace firebase {
 namespace firestore {
 namespace csharp {
 
-::firebase::Mutex DocumentEventListener::g_mutex;
-DocumentEventListenerCallback
-    DocumentEventListener::g_document_snapshot_event_listener_callback =
-        nullptr;
-
 void DocumentEventListener::OnEvent(const DocumentSnapshot& value,
                                     Error error) {
-  MutexLock lock(g_mutex);
-  if (g_document_snapshot_event_listener_callback) {
-    firebase::callback::AddCallback(firebase::callback::NewCallback(
-        DocumentSnapshotEvent, callback_id_, value, error));
-  }
-}
-
-/* static */
-void DocumentEventListener::SetCallback(
-    DocumentEventListenerCallback callback) {
-  MutexLock lock(g_mutex);
-  if (!callback) {
-    g_document_snapshot_event_listener_callback = nullptr;
-    return;
-  }
+  // Ownership of this pointer is passed into the C# handler
+  auto* copy = new DocumentSnapshot(value);
 
-  if (g_document_snapshot_event_listener_callback) {
-    FIREBASE_ASSERT(g_document_snapshot_event_listener_callback == callback);
-  } else {
-    g_document_snapshot_event_listener_callback = callback;
-  }
+  callback_(callback_id_, copy, error);
 }
 
 /* static */
 ListenerRegistration DocumentEventListener::AddListenerTo(
-    int32_t callback_id, DocumentReference reference,
-    MetadataChanges metadata_changes) {
-  DocumentEventListener listener(callback_id);
-  // TODO(zxu): For now, we call the one with lambda instead of EventListener
-  // pointer so we do not have to manage the ownership of the listener. We
-  // might want to extend the design to manage listeners and call the one with
-  // EventListener parameter e.g. adding extra parameter in the API to specify
-  // whether ownership is transferred.
-  return reference.AddSnapshotListener(
+    DocumentReference* reference, MetadataChanges metadata_changes,
+    int32_t callback_id, DocumentEventListenerCallback callback) {
+  DocumentEventListener listener(callback_id, callback);
+
+  return reference->AddSnapshotListener(
       metadata_changes,
       [listener](const DocumentSnapshot& value, Error error) mutable {
         listener.OnEvent(value, error);
       });
 }
 
-/* static */
-void DocumentEventListener::DocumentSnapshotEvent(int callback_id,
-                                                  DocumentSnapshot value,
-                                                  Error error) {
-  MutexLock lock(g_mutex);
-  if (g_document_snapshot_event_listener_callback == nullptr) {
-    return;
-  }
-  // The ownership is passed through the call to C# handler.
-  DocumentSnapshot* copy = new DocumentSnapshot(value);
-  g_document_snapshot_event_listener_callback(callback_id, copy, error);
-}
-
 }  // namespace csharp
 }  // namespace firestore
 }  // namespace firebase

firestore/src/include/firebase/csharp/document_event_listener.h

@@ -34,28 +34,23 @@ typedef void(SWIGSTDCALL* DocumentEventListenerCallback)(int callback_id,
 // can forward the calls back to the C# delegates.
 class DocumentEventListener : public EventListener<DocumentSnapshot> {
  public:
-  explicit DocumentEventListener(int32_t callback_id)
-      : callback_id_(callback_id) {}
+  explicit DocumentEventListener(int32_t callback_id,
+                                 DocumentEventListenerCallback callback)
+      : callback_id_(callback_id), callback_(callback) {}
 
   void OnEvent(const DocumentSnapshot& value, Error error) override;
 
-  static void SetCallback(DocumentEventListenerCallback callback);
-
-  static ListenerRegistration AddListenerTo(int32_t callback_id,
-                                            DocumentReference reference,
-                                            MetadataChanges metadataChanges);
+  // This method is a proxy to DocumentReference::AddSnapshotListener()
+  // that can be easily called from C#. It allows our C# wrapper to
+  // track user callbacks in a dictionary keyed off of a unique int
+  // for each user callback and then raise the correct one later.
+  static ListenerRegistration AddListenerTo(
+      DocumentReference* reference, MetadataChanges metadata_changes,
+      int32_t callback_id, DocumentEventListenerCallback callback);
 
  private:
-  static void DocumentSnapshotEvent(int callback_id, DocumentSnapshot value,
-                                    Error error);
-
   int32_t callback_id_;
-
-  // These static variables are named as global variable instead of private
-  // class member.
-  static Mutex g_mutex;
-  static DocumentEventListenerCallback
-      g_document_snapshot_event_listener_callback;
+  DocumentEventListenerCallback callback_;
 };
 
 }  // namespace csharp

firestore/src/include/firebase/csharp/query_event_listener.cc

@@ -7,61 +7,26 @@ namespace firebase {
 namespace firestore {
 namespace csharp {
 
-::firebase::Mutex QueryEventListener::g_mutex;
-QueryEventListenerCallback
-    QueryEventListener::g_query_snapshot_event_listener_callback = nullptr;
-
 void QueryEventListener::OnEvent(const QuerySnapshot& value, Error error) {
-  MutexLock lock(g_mutex);
-  if (g_query_snapshot_event_listener_callback) {
-    firebase::callback::AddCallback(firebase::callback::NewCallback(
-        QuerySnapshotEvent, callback_id_, value, error));
-  }
-}
-
-/* static */
-void QueryEventListener::SetCallback(QueryEventListenerCallback callback) {
-  MutexLock lock(g_mutex);
-  if (!callback) {
-    g_query_snapshot_event_listener_callback = nullptr;
-    return;
-  }
+  // Ownership of this pointer is passed into the C# handler
+  auto* copy = new QuerySnapshot(value);
 
-  if (g_query_snapshot_event_listener_callback) {
-    FIREBASE_ASSERT(g_query_snapshot_event_listener_callback == callback);
-  } else {
-    g_query_snapshot_event_listener_callback = callback;
-  }
+  callback_(callback_id_, copy, error);
 }
 
 /* static */
 ListenerRegistration QueryEventListener::AddListenerTo(
-    int32_t callback_id, Query query, MetadataChanges metadata_changes) {
-  QueryEventListener listener(callback_id);
-  // TODO(zxu): For now, we call the one with lambda instead of EventListener
-  // pointer so we do not have to manage the ownership of the listener. We
-  // might want to extend the design to manage listeners and call the one with
-  // EventListener parameter e.g. adding extra parameter in the API to specify
-  // whether ownership is transferred.
-  return query.AddSnapshotListener(
+    Query* query, MetadataChanges metadata_changes, int32_t callback_id,
+    QueryEventListenerCallback callback) {
+  QueryEventListener listener(callback_id, callback);
+
+  return query->AddSnapshotListener(
       metadata_changes,
       [listener](const QuerySnapshot& value, Error error) mutable {
         listener.OnEvent(value, error);
       });
 }
 
-/* static */
-void QueryEventListener::QuerySnapshotEvent(int callback_id,
-                                            QuerySnapshot value, Error error) {
-  MutexLock lock(g_mutex);
-  if (g_query_snapshot_event_listener_callback == nullptr) {
-    return;
-  }
-  // The ownership is passed through the call to C# handler.
-  QuerySnapshot* copy = new QuerySnapshot(value);
-  g_query_snapshot_event_listener_callback(callback_id, copy, error);
-}
-
 }  // namespace csharp
 }  // namespace firestore
 }  // namespace firebase

firestore/src/include/firebase/csharp/query_event_listener.h

@@ -33,27 +33,23 @@ typedef void(SWIGSTDCALL* QueryEventListenerCallback)(int callback_id,
 // can forward the calls back to the C# delegates.
 class QueryEventListener : public EventListener<QuerySnapshot> {
  public:
-  QueryEventListener(int32_t callback_id) : callback_id_(callback_id) {}
-
-  ~QueryEventListener() override {}
+  explicit QueryEventListener(int32_t callback_id,
+                              QueryEventListenerCallback callback)
+      : callback_id_(callback_id), callback_(callback) {}
 
   void OnEvent(const QuerySnapshot& value, Error error) override;
 
-  static void SetCallback(QueryEventListenerCallback callback);
-
-  static ListenerRegistration AddListenerTo(int32_t callback_id, Query query,
-                                            MetadataChanges metadataChanges);
+  // This method is a proxy to Query::AddSnapshotsListener()
+  // that can be easily called from C#. It allows our C# wrapper to
+  // track user callbacks in a dictionary keyed off of a unique int
+  // for each user callback and then raise the correct one later.
+  static ListenerRegistration AddListenerTo(
+      Query* query, MetadataChanges metadata_changes, int32_t callback_id,
+      QueryEventListenerCallback callback);
 
  private:
-  static void QuerySnapshotEvent(int callback_id, QuerySnapshot value,
-                                 Error error);
-
   int32_t callback_id_;
-
-  // These static variables are named as global variable instead of private
-  // class member.
-  static Mutex g_mutex;
-  static QueryEventListenerCallback g_query_snapshot_event_listener_callback;
+  QueryEventListenerCallback callback_;
 };
 
 }  // namespace csharp

firestore/src/include/firebase/csharp/snapshots_in_sync_listener.cc

@@ -6,10 +6,7 @@ namespace firebase {
 namespace firestore {
 namespace csharp {
 
-void SnapshotsInSyncListener::OnEvent(Error error) {
-  firebase::callback::AddCallback(
-      firebase::callback::NewCallback(callback_, callback_id_));
-}
+void SnapshotsInSyncListener::OnEvent(Error error) { callback_(callback_id_); }
 
 /* static */
 ListenerRegistration SnapshotsInSyncListener::AddListenerTo(