Firestore: use C++ Executor for user queue. (#1238)

FSTDispatchQueue enforces serial execution, which is inappropriate for
user queue, because a user may configure usage of a concurrent queue in
settings, breaking FSTDispatchQueue invariants. Instead, use C++
ExecutorLibdispatch directly.

2 files changed, 32 insertions, 32 deletions

diff --git a/Firestore/Source/Core/FSTFirestoreClient.h b/Firestore/Source/Core/FSTFirestoreClient.h
index 7285e65..94c2284 100644
--- a/Firestore/Source/Core/FSTFirestoreClient.h
+++ b/Firestore/Source/Core/FSTFirestoreClient.h
@@ -15,6 +15,7 @@
  */
 
 #import <Foundation/Foundation.h>
+#include <memory>
 
 #import "Firestore/Source/Core/FSTTypes.h"
 #import "Firestore/Source/Core/FSTViewSnapshot.h"
@@ -23,6 +24,7 @@
 #include "Firestore/core/src/firebase/firestore/auth/credentials_provider.h"
 #include "Firestore/core/src/firebase/firestore/core/database_info.h"
 #include "Firestore/core/src/firebase/firestore/model/database_id.h"
+#include "Firestore/core/src/firebase/firestore/util/executor.h"
 
 @class FIRDocumentReference;
 @class FIRDocumentSnapshot;
@@ -50,14 +52,15 @@ NS_ASSUME_NONNULL_BEGIN
 /**
  * Creates and returns a FSTFirestoreClient with the given parameters.
  *
- * All callbacks and events will be triggered on the provided userDispatchQueue.
+ * All callbacks and events will be triggered on the provided userExecutor.
  */
-+ (instancetype)clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
-                        usePersistence:(BOOL)usePersistence
-                   credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
-                                           credentialsProvider  // no passing ownership
-                     userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
-                   workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue;
++ (instancetype)
+clientWithDatabaseInfo:(const firebase::firestore::core::DatabaseInfo &)databaseInfo
+        usePersistence:(BOOL)usePersistence
+   credentialsProvider:(firebase::firestore::auth::CredentialsProvider *)
+                           credentialsProvider  // no passing ownership
+          userExecutor:(std::unique_ptr<firebase::firestore::util::internal::Executor>)userExecutor
+   workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue;
 
 - (instancetype)init __attribute__((unavailable("Use static constructor method.")));
 
@@ -111,7 +114,7 @@ NS_ASSUME_NONNULL_BEGIN
  * Dispatch queue for user callbacks / events. This will often be the "Main Dispatch Queue" of the
  * app but the developer can configure it to a different queue if they so choose.
  */
-@property(nonatomic, strong, readonly) FSTDispatchQueue *userDispatchQueue;
+- (firebase::firestore::util::internal::Executor *)userExecutor;
 
 @end
 
diff --git a/Firestore/Source/Core/FSTFirestoreClient.mm b/Firestore/Source/Core/FSTFirestoreClient.mm
index 658cf57..93d1f7c 100644
--- a/Firestore/Source/Core/FSTFirestoreClient.mm
+++ b/Firestore/Source/Core/FSTFirestoreClient.mm
@@ -18,6 +18,7 @@
 
 #include <future>  // NOLINT(build/c++11)
 #include <memory>
+#include <utility>
 
 #import "FIRFirestoreErrors.h"
 #import "Firestore/Source/API/FIRDocumentReference+Internal.h"
@@ -58,6 +59,8 @@ using firebase::firestore::core::DatabaseInfo;
 using firebase::firestore::model::DatabaseId;
 using firebase::firestore::model::DocumentKeySet;
 
+using firebase::firestore::util::internal::Executor;
+
 NS_ASSUME_NONNULL_BEGIN
 
 @interface FSTFirestoreClient () {
@@ -68,7 +71,7 @@ NS_ASSUME_NONNULL_BEGIN
                       usePersistence:(BOOL)usePersistence
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
-                   userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+                        userExecutor:(std::unique_ptr<Executor>)userExecutor
                  workerDispatchQueue:(FSTDispatchQueue *)queue NS_DESIGNATED_INITIALIZER;
 
 @property(nonatomic, assign, readonly) const DatabaseInfo *databaseInfo;
@@ -91,18 +94,24 @@ NS_ASSUME_NONNULL_BEGIN
 
 @end
 
-@implementation FSTFirestoreClient
+@implementation FSTFirestoreClient {
+  std::unique_ptr<Executor> _userExecutor;
+}
+
+- (Executor *)userExecutor {
+  return _userExecutor.get();
+}
 
 + (instancetype)clientWithDatabaseInfo:(const DatabaseInfo &)databaseInfo
                         usePersistence:(BOOL)usePersistence
                    credentialsProvider:
                        (CredentialsProvider *)credentialsProvider  // no passing ownership
-                     userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+                          userExecutor:(std::unique_ptr<Executor>)userExecutor
                    workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
   return [[FSTFirestoreClient alloc] initWithDatabaseInfo:databaseInfo
                                            usePersistence:usePersistence
                                       credentialsProvider:credentialsProvider
-                                        userDispatchQueue:userDispatchQueue
+                                             userExecutor:std::move(userExecutor)
                                       workerDispatchQueue:workerDispatchQueue];
 }
 
@@ -110,12 +119,12 @@ NS_ASSUME_NONNULL_BEGIN
                       usePersistence:(BOOL)usePersistence
                  credentialsProvider:
                      (CredentialsProvider *)credentialsProvider  // no passing ownership
-                   userDispatchQueue:(FSTDispatchQueue *)userDispatchQueue
+                        userExecutor:(std::unique_ptr<Executor>)userExecutor
                  workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue {
   if (self = [super init]) {
     _databaseInfo = databaseInfo;
     _credentialsProvider = credentialsProvider;
-    _userDispatchQueue = userDispatchQueue;
+    _userExecutor = std::move(userExecutor);
     _workerDispatchQueue = workerDispatchQueue;
 
     auto userPromise = std::make_shared<std::promise<User>>();
@@ -230,9 +239,7 @@ NS_ASSUME_NONNULL_BEGIN
   [self.workerDispatchQueue dispatchAsync:^{
     [self.remoteStore disableNetwork];
     if (completion) {
-      [self.userDispatchQueue dispatchAsync:^{
-        completion(nil);
-      }];
+      _userExecutor->Execute([=] { completion(nil); });
     }
   }];
 }
@@ -241,9 +248,7 @@ NS_ASSUME_NONNULL_BEGIN
   [self.workerDispatchQueue dispatchAsync:^{
     [self.remoteStore enableNetwork];
     if (completion) {
-      [self.userDispatchQueue dispatchAsync:^{
-        completion(nil);
-      }];
+      _userExecutor->Execute([=] { completion(nil); });
     }
   }];
 }
@@ -255,9 +260,7 @@ NS_ASSUME_NONNULL_BEGIN
     [self.remoteStore shutdown];
     [self.persistence shutdown];
     if (completion) {
-      [self.userDispatchQueue dispatchAsync:^{
-        completion(nil);
-      }];
+      _userExecutor->Execute([=] { completion(nil); });
     }
   }];
 }
@@ -338,18 +341,14 @@ NS_ASSUME_NONNULL_BEGIN
   [self.workerDispatchQueue dispatchAsync:^{
     if (mutations.count == 0) {
       if (completion) {
-        [self.userDispatchQueue dispatchAsync:^{
-          completion(nil);
-        }];
+        _userExecutor->Execute([=] { completion(nil); });
       }
     } else {
       [self.syncEngine writeMutations:mutations
                            completion:^(NSError *error) {
                              // Dispatch the result back onto the user dispatch queue.
                              if (completion) {
-                               [self.userDispatchQueue dispatchAsync:^{
-                                 completion(error);
-                               }];
+                               _userExecutor->Execute([=] { completion(error); });
                              }
                            }];
     }
@@ -366,9 +365,7 @@ NS_ASSUME_NONNULL_BEGIN
                                  completion:^(id _Nullable result, NSError *_Nullable error) {
                                    // Dispatch the result back onto the user dispatch queue.
                                    if (completion) {
-                                     [self.userDispatchQueue dispatchAsync:^{
-                                       completion(result, error);
-                                     }];
+                                     _userExecutor->Execute([=] { completion(result, error); });
                                    }
                                  }];
   }];