Cache batched bridge methods and provide better error messages when c…

…alling functions without bridge config Summary: Maybe a slight perf improvement. Also helps get rid of the 'ReferenceError: __fbBatchedBridge' error in favor of a more explicit "you're calling JS functions before the bundle is loaded" Differential Revision: D3142966 fb-gh-sync-id: 6a50efe7d1634248107c8c95a3e014cb263a9ca5 fbshipit-source-id: 6a50efe7d1634248107c8c95a3e014cb263a9ca5
esp0 · Apr 11, 2016 · c24fae9 · c24fae9
1 parent 8ef5511
commit c24fae9
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 11 deletions.
diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.cpp b/ReactAndroid/src/main/jni/react/JSCExecutor.cpp
@@ -9,6 +9,7 @@
 #include <string>
 #include <glog/logging.h>
 #include <folly/json.h>
+#include <folly/Memory.h>
 #include <folly/String.h>
 #include <sys/time.h>
 
@@ -199,6 +200,9 @@ void JSCExecutor::terminateOnJSVMThread() {
     terminateOwnedWebWorker(workerId);
   }
 
+  m_batchedBridge.reset();
+  m_flushedQueueObj.reset();
+
   s_globalContextRefToJSCExecutor.erase(m_context);
   JSGlobalContextRelease(m_context);
   m_context = nullptr;
@@ -257,28 +261,62 @@ void JSCExecutor::loadApplicationUnbundle(
   loadApplicationScript(startupCode, sourceURL);
 }
 
+bool JSCExecutor::ensureBatchedBridgeObject() {
+  if (m_batchedBridge) {
+    return true;
+  }
+
+  Value batchedBridgeValue = Object::getGlobalObject(m_context).getProperty("__fbBatchedBridge");
+  if (batchedBridgeValue.isUndefined()) {
+    return false;
+  }
+  m_batchedBridge = folly::make_unique<Object>(batchedBridgeValue.asObject());
+  m_flushedQueueObj = folly::make_unique<Object>(m_batchedBridge->getProperty("flushedQueue").asObject());
+  return true;
+}
+
 void JSCExecutor::flush() {
-  // TODO: Make this a first class function instead of evaling. #9317773
-  std::string calls = executeJSCallWithJSC(m_context, "flushedQueue", std::vector<folly::dynamic>());
+  #ifdef WITH_FBSYSTRACE
+  FbSystraceSection s(
+      TRACE_TAG_REACT_CXX_BRIDGE, "JSCExecutor.flush");
+  #endif
+
+  if (!ensureBatchedBridgeObject()) {
+    throwJSExecutionException(
+        "Couldn't get the native call queue: bridge configuration isn't available. This shouldn't be possible. Congratulations.");
+  }
+
+  std::string calls = m_flushedQueueObj->callAsFunction().toJSONString();
   m_bridge->callNativeModules(*this, calls, true);
 }
 
 void JSCExecutor::callFunction(const std::string& moduleId, const std::string& methodId, const folly::dynamic& arguments) {
-  // TODO:  Make this a first class function instead of evaling. #9317773
-  std::vector<folly::dynamic> call{
-    moduleId,
-    methodId,
-    std::move(arguments),
+  if (!ensureBatchedBridgeObject()) {
+    throwJSExecutionException(
+        "Couldn't call JS module %s, method %s: bridge configuration isn't available. This "
+        "probably means you're calling a JS module method before bridge setup has completed or without a JS bundle loaded.",
+        moduleId.c_str(),
+        methodId.c_str());
+  }
+
+  std::vector<folly::dynamic> call {
+      moduleId,
+      methodId,
+      std::move(arguments),
   };
   std::string calls = executeJSCallWithJSC(m_context, "callFunctionReturnFlushedQueue", std::move(call));
   m_bridge->callNativeModules(*this, calls, true);
 }
 
 void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) {
-  // TODO: Make this a first class function instead of evaling. #9317773
-  std::vector<folly::dynamic> call{
-    (double) callbackId,
-    std::move(arguments)
+  if (!ensureBatchedBridgeObject()) {
+    throwJSExecutionException(
+        "Couldn't invoke JS callback %d: bridge configuration isn't available. This shouldn't be possible. Congratulations.", (int) callbackId);
+  }
+
+  std::vector<folly::dynamic> call {
+      (double) callbackId,
+      std::move(arguments)
   };
   std::string calls = executeJSCallWithJSC(m_context, "invokeCallbackAndReturnFlushedQueue", std::move(call));
   m_bridge->callNativeModules(*this, calls, true);

diff --git a/ReactAndroid/src/main/jni/react/JSCExecutor.h b/ReactAndroid/src/main/jni/react/JSCExecutor.h
@@ -88,6 +88,8 @@ class JSCExecutor : public JSExecutor {
   std::shared_ptr<MessageQueueThread> m_messageQueueThread;
   std::unique_ptr<JSModulesUnbundle> m_unbundle;
   folly::dynamic m_jscConfig;
+  std::unique_ptr<Object> m_batchedBridge;
+  std::unique_ptr<Object> m_flushedQueueObj;
 
   /**
    * WebWorker constructor. Must be invoked from thread this Executor will run on.
@@ -105,6 +107,7 @@ class JSCExecutor : public JSExecutor {
   void flush();
   void flushQueueImmediate(std::string queueJSON);
   void loadModule(uint32_t moduleId);
+  bool ensureBatchedBridgeObject();
 
   int addWebWorker(const std::string& script, JSValueRef workerRef, JSValueRef globalObjRef);
   void postMessageToOwnedWebWorker(int worker, JSValueRef message, JSValueRef *exn);

diff --git a/ReactAndroid/src/main/jni/react/Value.cpp b/ReactAndroid/src/main/jni/react/Value.cpp
@@ -69,6 +69,11 @@ Value Object::callAsFunction(int nArgs, JSValueRef args[]) {
   return Value(m_context, result);
 }
 
+Value Object::callAsFunction() {
+  JSValueRef args[0];
+  return callAsFunction(0, args);
+}
+
 Value Object::getProperty(const String& propName) const {
   JSValueRef exn;
   JSValueRef property = JSObjectGetProperty(m_context, m_obj, propName, &exn);

diff --git a/ReactAndroid/src/main/jni/react/Value.h b/ReactAndroid/src/main/jni/react/Value.h
@@ -125,6 +125,7 @@ class Object : public noncopyable {
   }
 
   Value callAsFunction(int nArgs, JSValueRef args[]);
+  Value callAsFunction();
 
   Value getProperty(const String& propName) const;
   Value getProperty(const char *propName) const;