Change JSCallInvoker to be an abstract class

Summary: The existing implementation of JSCallInvoker holds a reference to Instance (aka the bridge). This diff makes JSCallInvoker an abstract base class that's extended by BridgeJSCallInvoker, which is what's returned by CatalystInstance to initialize TurboModules. This will allow us to add another class that derives from JSCallInvoker that doesn't rely on the bridge.

Reviewed By: shergin

Differential Revision: D15252375

fbshipit-source-id: 75eee2ca149235a63e2a2cd8cdd361b163d1d1ab

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

@@ -268,7 +268,7 @@ void CatalystInstanceImpl::handleMemoryPressure(int pressureLevel) {
 
 jni::alias_ref<JSCallInvokerHolder::javaobject> CatalystInstanceImpl::getJSCallInvokerHolder() {
   if (!javaInstanceHolder_) {
-    jsCallInvoker_ = std::make_shared<JSCallInvoker>(instance_);
+    jsCallInvoker_ = std::make_shared<BridgeJSCallInvoker>(instance_);
     javaInstanceHolder_ = jni::make_global(JSCallInvokerHolder::newObjectCxxArgs(jsCallInvoker_));
   }
 

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h

@@ -8,6 +8,7 @@
 #include <fb/fbjni.h>
 #include <folly/Memory.h>
 #include <jsireact/JSCallInvokerHolder.h>
+#include <jsireact/BridgeJSCallInvoker.h>
 
 #include "CxxModuleWrapper.h"
 #include "JavaModuleWrapper.h"
@@ -86,7 +87,7 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
   std::shared_ptr<ModuleRegistry> moduleRegistry_;
   std::shared_ptr<JMessageQueueThread> moduleMessageQueue_;
   jni::global_ref<JSCallInvokerHolder::javaobject> javaInstanceHolder_;
-  std::shared_ptr<JSCallInvoker> jsCallInvoker_;
+  std::shared_ptr<BridgeJSCallInvoker> jsCallInvoker_;
 };
 
 }}

ReactCommon/jscallinvoker/BUCK

@@ -3,10 +3,11 @@ load("//tools/build_defs/oss:rn_defs.bzl", "ANDROID", "APPLE", "react_native_xpl
 rn_xplat_cxx_library(
     name = "jscallinvoker",
     srcs = [
-        "jsireact/JSCallInvoker.cpp",
+        "jsireact/BridgeJSCallInvoker.cpp",
     ],
     header_namespace = "",
     exported_headers = {
+        "jsireact/BridgeJSCallInvoker.h": "jsireact/BridgeJSCallInvoker.h",
         "jsireact/JSCallInvoker.h": "jsireact/JSCallInvoker.h",
     },
     compiler_flags = [

ReactCommon/jscallinvoker/jsireact/BridgeJSCallInvoker.cpp

@@ -5,16 +5,16 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-#include <jsireact/JSCallInvoker.h>
+#include <jsireact/BridgeJSCallInvoker.h>
 #include <cxxreact/Instance.h>
 
 namespace facebook {
 namespace react {
 
-JSCallInvoker::JSCallInvoker(std::weak_ptr<Instance> reactInstance)
+BridgeJSCallInvoker::BridgeJSCallInvoker(std::weak_ptr<Instance> reactInstance)
     : reactInstance_(reactInstance) {}
 
-void JSCallInvoker::invokeAsync(std::function<void()> &&func) {
+void BridgeJSCallInvoker::invokeAsync(std::function<void()> &&func) {
   auto instance = reactInstance_.lock();
   if (instance == nullptr) {
     return;

ReactCommon/jscallinvoker/jsireact/BridgeJSCallInvoker.h

@@ -0,0 +1,42 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <functional>
+#include <memory>
+
+#include <jsireact/JSCallInvoker.h>
+
+namespace facebook {
+namespace react {
+
+class Instance;
+
+/**
+ * A native-to-JS call invoker that uses the bridge ('Instance').
+ * It guarantees that any calls from any thread are queued on the right JS
+ * thread.
+ *
+ * For now, this is a thin-wrapper around existing bridge. Eventually,
+ * it should be consolidated with Fabric implementation so there's only one
+ * API to call JS from native, whether synchronously or asynchronously.
+ * Also, this class should not depend on `Instance` in the future.
+ */
+class BridgeJSCallInvoker : public JSCallInvoker {
+ public:
+  BridgeJSCallInvoker(std::weak_ptr<Instance> reactInstance);
+
+  void invokeAsync(std::function<void()> &&func);
+  // TODO: add sync support
+
+ private:
+  std::weak_ptr<Instance> reactInstance_;
+};
+
+} // namespace react
+} // namespace facebook

ReactCommon/jscallinvoker/jsireact/JSCallInvoker.h

@@ -13,26 +13,15 @@
 namespace facebook {
 namespace react {
 
-class Instance;
-
 /**
- * A generic native-to-JS call invoker. It guarantees that any calls from any
- * thread are queued on the right JS thread.
- *
- * For now, this is a thin-wrapper around existing bridge (`Instance`). Eventually,
- * it should be consolidated with Fabric implementation so there's only one
- * API to call JS from native, whether synchronously or asynchronously.
- * Also, this class should not depend on `Instance` in the future.
+ * An interface for a generic native-to-JS call invoker. See BridgeJSCallInvoker
+ * for an implementation.
  */
 class JSCallInvoker {
-public:
-  JSCallInvoker(std::weak_ptr<Instance> reactInstance);
-
-  void invokeAsync(std::function<void()>&& func);
+ public:
+  virtual void invokeAsync(std::function<void()> &&func) = 0;
   // TODO: add sync support
-
-private:
-  std::weak_ptr<Instance> reactInstance_;
+  virtual ~JSCallInvoker() {}
 };
 
 } // namespace react

ReactCommon/turbomodule/core/platform/ios/RCTTurboModuleManager.mm

@@ -13,6 +13,7 @@
 #import <React/RCTBridgeModule.h>
 #import <React/RCTCxxModule.h>
 #import <React/RCTLog.h>
+#import <jsireact/BridgeJSCallInvoker.h>
 #import <jsireact/TurboCxxModule.h>
 #import <jsireact/TurboModuleBinding.h>
 
@@ -48,7 +49,7 @@ static Class getFallbackClassFromName(const char *name) {
 - (instancetype)initWithBridge:(RCTBridge *)bridge delegate:(id<RCTTurboModuleManagerDelegate>)delegate
 {
   if (self = [super init]) {
-    _jsInvoker = std::make_shared<react::JSCallInvoker>(bridge.reactInstance);
+    _jsInvoker = std::make_shared<react::BridgeJSCallInvoker>(bridge.reactInstance);
     _delegate = delegate;
     _bridge = bridge;