Bind methods instead of using eval

Reviewed By: astreet

Differential Revision: D3417452

fbshipit-source-id: 437daa2dfcd01efb749465a94c1a72ce8a2cb315

ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java

@@ -914,8 +914,14 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;
             return null;
           }
         }).get();
-    } catch (InterruptedException | ExecutionException e) {
+    } catch (InterruptedException e) {
       throw new RuntimeException(e);
+    } catch (ExecutionException e) {
+      if (e.getCause() instanceof RuntimeException) {
+        throw (RuntimeException) e.getCause();
+      } else {
+        throw new RuntimeException(e);
+      }
     }
 
     return reactContext;

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/BUCK

@@ -27,7 +27,8 @@ android_library(
     react_native_dep('third-party/java/okhttp:okhttp3-ws'),
     react_native_target('java/com/facebook/react/bridge:bridge'),
     react_native_target('java/com/facebook/react/common:common'),
-  ],
+    react_native_target('java/com/facebook/react/devsupport:devsupport'),
+],
   visibility = [
     'PUBLIC',
   ],

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/JSBundleLoader.java

@@ -11,6 +11,9 @@ package com.facebook.react.cxxbridge;
 
 import android.content.Context;
 
+import com.facebook.react.devsupport.DebugServerException;
+import com.facebook.react.devsupport.DevServerHelper;
+
 /**
  * A class that stores JS bundle information and allows {@link CatalystInstance} to load a correct
  * bundle through {@link ReactBridge}.
@@ -55,7 +58,11 @@ public abstract class JSBundleLoader {
     return new JSBundleLoader() {
       @Override
       public void loadScript(CatalystInstanceImpl instance) {
+        try {
           instance.loadScriptFromFile(cachedFileLocation, sourceURL);
+        } catch (Exception e) {
+          throw DebugServerException.makeGeneric(e.getMessage(), e);
+        }
       }
 
       @Override

ReactAndroid/src/main/java/com/facebook/react/devsupport/DebugServerException.java

@@ -25,7 +25,22 @@ import org.json.JSONObject;
  * Tracks errors connecting to or received from the debug derver.
  * The debug server returns errors as json objects. This exception represents that error.
  */
-public class DebugServerException extends IOException {
+public class DebugServerException extends RuntimeException {
+  private static final String GENERIC_ERROR_MESSAGE =
+      "\n\nTry the following to fix the issue:\n" +
+      "\u2022 Ensure that the packager server is running\n" +
+      "\u2022 Ensure that your device/emulator is connected to your machine and has USB debugging enabled - run 'adb devices' to see a list of connected devices\n" +
+      "\u2022 If you're on a physical device connected to the same machine, run 'adb reverse tcp:8081 tcp:8081' to forward requests from your device\n" +
+      "\u2022 If your device is on the same Wi-Fi network, set 'Debug server host & port for device' in 'Dev settings' to your machine's IP address and the port of the local dev server - e.g. 10.0.1.1:8081\n\n";
+
+  public static DebugServerException makeGeneric(String reason, Throwable t) {
+    return makeGeneric(reason, "", t);
+  }
+
+  public static DebugServerException makeGeneric(String reason, String extra, Throwable t) {
+    return new DebugServerException(reason + GENERIC_ERROR_MESSAGE + extra, t);
+  }
+
   private DebugServerException(String description, String fileName, int lineNumber, int column) {
     super(description + "\n  at " + fileName + ":" + lineNumber + ":" + column);
   }
@@ -34,6 +49,10 @@ public class DebugServerException extends IOException {
     super(description);
   }
 
+  public DebugServerException(String detailMessage, Throwable throwable) {
+    super(detailMessage, throwable);
+  }
+
   /**
    * Parse a DebugServerException from the server json string.
    * @param str json string returned by the debug server

ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java

@@ -47,7 +47,6 @@ import okio.Sink;
  *  - Genymotion emulator with default settings: 10.0.3.2
  */
 public class DevServerHelper {
-
   public static final String RELOAD_APP_EXTRA_JS_PROXY = "jsproxy";
   private static final String RELOAD_APP_ACTION_SUFFIX = ".RELOAD_APP_ACTION";
 
@@ -185,15 +184,10 @@ public class DevServerHelper {
         }
         mDownloadBundleFromURLCall = null;
 
-        StringBuilder sb = new StringBuilder();
-        sb.append("Could not connect to development server.\n\n")
-          .append("Try the following to fix the issue:\n")
-          .append("\u2022 Ensure that the packager server is running\n")
-          .append("\u2022 Ensure that your device/emulator is connected to your machine and has USB debugging enabled - run 'adb devices' to see a list of connected devices\n")
-          .append("\u2022 If you're on a physical device connected to the same machine, run 'adb reverse tcp:8081 tcp:8081' to forward requests from your device\n")
-          .append("\u2022 If your device is on the same Wi-Fi network, set 'Debug server host & port for device' in 'Dev settings' to your machine's IP address and the port of the local dev server - e.g. 10.0.1.1:8081\n\n")
-          .append("URL: ").append(call.request().url().toString());
-        callback.onFailure(new DebugServerException(sb.toString()));
+        callback.onFailure(DebugServerException.makeGeneric(
+            "Could not connect to development server.",
+            "URL: " + call.request().url().toString(),
+            e));
       }
 
       @Override

ReactCommon/cxxreact/JSCExecutor.cpp

@@ -90,22 +90,6 @@ static JSValueRef nativeInjectHMRUpdate(
     const JSValueRef arguments[],
     JSValueRef *exception);
 
-static std::string executeJSCallWithJSC(
-    JSGlobalContextRef ctx,
-    const std::string& methodName,
-    const std::vector<folly::dynamic>& arguments) throw(JSException) {
-  SystraceSection s("JSCExecutor.executeJSCall",
-                    "method", methodName);
-
-  // Evaluate script with JSC
-  folly::dynamic jsonArgs(arguments.begin(), arguments.end());
-  auto js = folly::to<folly::fbstring>(
-      "__fbBatchedBridge.", methodName, ".apply(null, ",
-      folly::toJson(jsonArgs), ")");
-  auto result = evaluateScript(ctx, String(js.c_str()), nullptr);
-  return Value(ctx, result).toJSONString();
-}
-
 std::unique_ptr<JSExecutor> JSCExecutorFactory::createJSExecutor(
     std::shared_ptr<ExecutorDelegate> delegate, std::shared_ptr<MessageQueueThread> jsQueue) {
   return std::unique_ptr<JSExecutor>(
@@ -289,6 +273,9 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr<const JSBigString> scrip
 
   String jsSourceURL(sourceURL.c_str());
   evaluateScript(m_context, jsScript, jsSourceURL);
+
+  bindBridge();
+
   flush();
   ReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
 }
@@ -300,32 +287,41 @@ void JSCExecutor::setJSModulesUnbundle(std::unique_ptr<JSModulesUnbundle> unbund
   m_unbundle = std::move(unbundle);
 }
 
+void JSCExecutor::bindBridge() throw(JSException) {
+  auto global = Object::getGlobalObject(m_context);
+  auto batchedBridgeValue = global.getProperty("__fbBatchedBridge");
+  if (batchedBridgeValue.isUndefined()) {
+    throwJSExecutionException("Could not get BatchedBridge, make sure your bundle is packaged correctly");
+  }
+
+  auto batchedBridge = batchedBridgeValue.asObject();
+  m_callFunctionReturnFlushedQueueJS = batchedBridge.getProperty("callFunctionReturnFlushedQueue").asObject();
+  m_invokeCallbackAndReturnFlushedQueueJS = batchedBridge.getProperty("invokeCallbackAndReturnFlushedQueue").asObject();
+  m_flushedQueueJS = batchedBridge.getProperty("flushedQueue").asObject();
+}
+
 void JSCExecutor::flush() throw(JSException) {
-  // TODO: Make this a first class function instead of evaling. #9317773
-  std::string calls = executeJSCallWithJSC(m_context, "flushedQueue", std::vector<folly::dynamic>());
+  auto result = m_flushedQueueJS->callAsFunction({});
+  auto calls = Value(m_context, result).toJSONString();
   m_delegate->callNativeModules(*this, std::move(calls), true);
 }
 
 void JSCExecutor::callFunction(const std::string& moduleId, const std::string& methodId, const folly::dynamic& arguments) throw(JSException) {
-  // TODO:  Make this a first class function instead of evaling. #9317773
-  // TODO(cjhopman): This copies args.
-  std::vector<folly::dynamic> call{
-    moduleId,
-    methodId,
-    std::move(arguments),
-  };
-  std::string calls = executeJSCallWithJSC(m_context, "callFunctionReturnFlushedQueue", std::move(call));
+  auto result = m_callFunctionReturnFlushedQueueJS->callAsFunction({
+    Value(m_context, String::createExpectingAscii(moduleId)),
+    Value(m_context, String::createExpectingAscii(methodId)),
+    Value::fromDynamic(m_context, std::move(arguments))
+  });
+  auto calls = Value(m_context, result).toJSONString();
   m_delegate->callNativeModules(*this, std::move(calls), true);
 }
 
 void JSCExecutor::invokeCallback(const double callbackId, const folly::dynamic& arguments) throw(JSException) {
-  // TODO: Make this a first class function instead of evaling. #9317773
-  // TODO(cjhopman): This copies args.
-  std::vector<folly::dynamic> call{
-    (double) callbackId,
-    std::move(arguments)
-  };
-  std::string calls = executeJSCallWithJSC(m_context, "invokeCallbackAndReturnFlushedQueue", std::move(call));
+  auto result = m_invokeCallbackAndReturnFlushedQueueJS->callAsFunction({
+    JSValueMakeNumber(m_context, callbackId),
+    Value::fromDynamic(m_context, std::move(arguments))
+  });
+  auto calls = Value(m_context, result).toJSONString();
   m_delegate->callNativeModules(*this, std::move(calls), true);
 }
 

ReactCommon/cxxreact/JSCExecutor.h

@@ -9,6 +9,7 @@
 #include <JavaScriptCore/JSContextRef.h>
 
 #include <folly/json.h>
+#include <folly/Optional.h>
 
 #include "Executor.h"
 #include "ExecutorToken.h"
@@ -91,6 +92,10 @@ private:
   std::unique_ptr<JSModulesUnbundle> m_unbundle;
   folly::dynamic m_jscConfig;
 
+  folly::Optional<Object> m_invokeCallbackAndReturnFlushedQueueJS;
+  folly::Optional<Object> m_callFunctionReturnFlushedQueueJS;
+  folly::Optional<Object> m_flushedQueueJS;
+
   /**
    * WebWorker constructor. Must be invoked from thread this Executor will run on.
    */
@@ -105,6 +110,7 @@ private:
 
   void initOnJSVMThread() throw(JSException);
   void terminateOnJSVMThread();
+  void bindBridge() throw(JSException);
   void flush() throw(JSException);
   void flushQueueImmediate(std::string queueJSON);
   void loadModule(uint32_t moduleId);

ReactCommon/cxxreact/Value.cpp

@@ -1,5 +1,7 @@
 // Copyright 2004-present Facebook. All Rights Reserved.
 
+#include <folly/json.h>
+
 #include "Value.h"
 
 #include "JSCHelpers.h"
@@ -49,6 +51,11 @@ Value Value::fromJSON(JSContextRef ctx, const String& json) throw(JSException) {
   return Value(ctx, result);
 }
 
+Value Value::fromDynamic(JSContextRef ctx, folly::dynamic value) throw(JSException) {
+  auto json = folly::toJson(value);
+  return fromJSON(ctx, String(json.c_str()));
+}
+
 Object Value::asObject() {
   JSValueRef exn;
   JSObjectRef jsObj = JSValueToObject(context(), m_value, &exn);
@@ -65,7 +72,11 @@ Object::operator Value() const {
   return Value(m_context, m_obj);
 }
 
-Value Object::callAsFunction(int nArgs, JSValueRef args[]) {
+Value Object::callAsFunction(std::initializer_list<JSValueRef> args) const {
+  return callAsFunction(args.size(), args.begin());
+}
+
+Value Object::callAsFunction(int nArgs, const JSValueRef args[]) const {
   JSValueRef exn;
   JSValueRef result = JSObjectCallAsFunction(m_context, m_obj, NULL, nArgs, args, &exn);
   if (!result) {

ReactCommon/cxxreact/Value.h

@@ -12,6 +12,8 @@
 #include <JavaScriptCore/JSStringRef.h>
 #include <JavaScriptCore/JSValueRef.h>
 
+#include <folly/dynamic.h>
+
 #include "noncopyable.h"
 
 #if WITH_FBJSCEXTENSIONS
@@ -148,6 +150,13 @@ public:
     }
   }
 
+  Object& operator=(Object&& other) {
+    std::swap(m_context, other.m_context);
+    std::swap(m_obj, other.m_obj);
+    std::swap(m_isProtected, other.m_isProtected);
+    return *this;
+  }
+
   operator JSObjectRef() const {
     return m_obj;
   }
@@ -158,7 +167,9 @@ public:
     return JSObjectIsFunction(m_context, m_obj);
   }
 
-  Value callAsFunction(int nArgs, JSValueRef args[]);
+  Value callAsFunction(std::initializer_list<JSValueRef> args) const;
+
+  Value callAsFunction(int nArgs, const JSValueRef args[]) const;
 
   Value getProperty(const String& propName) const;
   Value getProperty(const char *propName) const;
@@ -253,6 +264,7 @@ public:
 
   std::string toJSONString(unsigned indent = 0) const throw(JSException);
   static Value fromJSON(JSContextRef ctx, const String& json) throw(JSException);
+  static Value fromDynamic(JSContextRef ctx, folly::dynamic value) throw(JSException);
 protected:
   JSContextRef context() const;
   JSContextRef m_context;