Remove the restriction on importing bridge:cxxreact

Reviewed By: javache

Differential Revision: D5724406

fbshipit-source-id: 24974601d161fd23805d8e925b2b20a1cf11850d

ReactAndroid/src/main/jni/react/jni/Android.mk

@@ -5,6 +5,7 @@ include $(CLEAR_VARS)
 LOCAL_MODULE := reactnativejni
 
 LOCAL_SRC_FILES := \
+  AndroidJSCFactory.cpp \
   CatalystInstanceImpl.cpp \
   CxxModuleWrapper.cpp \
   JavaModuleWrapper.cpp \

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

@@ -0,0 +1,145 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#include <cxxreact/JSCExecutor.h>
+
+#include <string>
+
+#include <cxxreact/Platform.h>
+#include <fb/fbjni.h>
+#include <folly/Conv.h>
+#include <folly/dynamic.h>
+#include <folly/Memory.h>
+#include <jschelpers/JSCHelpers.h>
+
+#include "JSCPerfLogging.h"
+#include "JSLogging.h"
+
+using namespace facebook::jni;
+
+namespace facebook {
+namespace react {
+
+namespace {
+
+class JReactMarker : public JavaClass<JReactMarker> {
+ public:
+  static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;";
+
+  static void logMarker(const std::string& marker) {
+    static auto cls = javaClassStatic();
+    static auto meth = cls->getStaticMethod<void(std::string)>("logMarker");
+    meth(cls, marker);
+  }
+
+  static void logMarker(const std::string& marker, const std::string& tag) {
+    static auto cls = javaClassStatic();
+    static auto meth = cls->getStaticMethod<void(std::string, std::string)>("logMarker");
+    meth(cls, marker, tag);
+  }
+};
+
+void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) {
+  switch (markerId) {
+    case ReactMarker::RUN_JS_BUNDLE_START:
+      JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag);
+      break;
+    case ReactMarker::RUN_JS_BUNDLE_STOP:
+      JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag);
+      break;
+    case ReactMarker::CREATE_REACT_CONTEXT_STOP:
+      JReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
+      break;
+    case ReactMarker::JS_BUNDLE_STRING_CONVERT_START:
+      JReactMarker::logMarker("loadApplicationScript_startStringConvert");
+      break;
+    case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP:
+      JReactMarker::logMarker("loadApplicationScript_endStringConvert");
+      break;
+    case ReactMarker::NATIVE_MODULE_SETUP_START:
+      JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag);
+      break;
+    case ReactMarker::NATIVE_MODULE_SETUP_STOP:
+      JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag);
+      break;
+    case ReactMarker::NATIVE_REQUIRE_START:
+    case ReactMarker::NATIVE_REQUIRE_STOP:
+      // These are not used on Android.
+      break;
+  }
+}
+
+ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) {
+  auto jniEx = dynamic_cast<const jni::JniException *>(&ex);
+  if (!jniEx) {
+    return {};
+  }
+
+  auto stackTrace = jniEx->getThrowable()->getStackTrace();
+  std::ostringstream stackStr;
+  for (int i = 0, count = stackTrace->size(); i < count; ++i) {
+    auto frame = stackTrace->getElement(i);
+
+    auto methodName = folly::to<std::string>(frame->getClassName(), ".",
+      frame->getMethodName());
+
+    // Cut off stack traces at the Android looper, to keep them simple
+    if (methodName == "android.os.Looper.loop") {
+      break;
+    }
+
+    stackStr << std::move(methodName) << '@' << frame->getFileName();
+    if (frame->getLineNumber() > 0) {
+      stackStr << ':' << frame->getLineNumber();
+    }
+    stackStr << std::endl;
+  }
+
+  auto msg = folly::to<std::string>("Java exception in '", context, "'\n\n", jniEx->what());
+  return {.message = msg, .stack = stackStr.str()};
+}
+
+JSValueRef nativePerformanceNow(
+    JSContextRef ctx,
+    JSObjectRef function,
+    JSObjectRef thisObject,
+    size_t argumentCount,
+    const JSValueRef arguments[], JSValueRef *exception) {
+  static const int64_t NANOSECONDS_IN_SECOND = 1000000000LL;
+  static const int64_t NANOSECONDS_IN_MILLISECOND = 1000000LL;
+
+  // Since SystemClock.uptimeMillis() is commonly used for performance measurement in Java
+  // and uptimeMillis() internally uses clock_gettime(CLOCK_MONOTONIC),
+  // we use the same API here.
+  // We need that to make sure we use the same time system on both JS and Java sides.
+  // Links to the source code:
+  // https://android.googlesource.com/platform/frameworks/native/+/jb-mr1-release/libs/utils/SystemClock.cpp
+  // https://android.googlesource.com/platform/system/core/+/master/libutils/Timers.cpp
+  struct timespec now;
+  clock_gettime(CLOCK_MONOTONIC, &now);
+  int64_t nano = now.tv_sec * NANOSECONDS_IN_SECOND + now.tv_nsec;
+  return Value::makeNumber(ctx, (nano / (double)NANOSECONDS_IN_MILLISECOND));
+}
+
+}
+
+namespace detail {
+
+void injectJSCExecutorAndroidPlatform() {
+  // Inject some behavior into react/
+  ReactMarker::logTaggedMarker = logPerfMarker;
+  ExceptionHandling::platformErrorExtractor = extractJniError;
+  JSCNativeHooks::loggingHook = nativeLoggingHook;
+  JSCNativeHooks::nowHook = nativePerformanceNow;
+  JSCNativeHooks::installPerfHooks = addNativePerfLoggingHooks;
+}
+
+}
+
+std::unique_ptr<JSExecutorFactory> makeAndroidJSCExecutorFactory(
+    const folly::dynamic& jscConfig) {
+  detail::injectJSCExecutorAndroidPlatform();
+  return folly::make_unique<JSCExecutorFactory>(std::move(jscConfig));
+}
+
+}
+}

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

@@ -0,0 +1,29 @@
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#pragma once
+
+#include <memory>
+
+namespace folly {
+
+class dynamic;
+
+}
+
+namespace facebook {
+namespace react {
+
+class JSExecutorFactory;
+
+namespace detail {
+
+// This is only exposed so instrumentation tests can call it.
+void injectJSCExecutorAndroidPlatform();
+
+}
+
+std::unique_ptr<JSExecutorFactory> makeAndroidJSCExecutorFactory(
+    const folly::dynamic& jscConfig);
+
+}
+}

ReactAndroid/src/main/jni/react/jni/BUCK

@@ -1,6 +1,7 @@
 include_defs("//ReactAndroid/DEFS")
 
 EXPORTED_HEADERS = [
+    "AndroidJSCFactory.h",
     "CxxModuleWrapper.h",
     "CxxModuleWrapperBase.h",
     "CxxSharedModuleWrapper.h",

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

@@ -3,7 +3,6 @@
 #include <string>
 
 #include <jschelpers/JSCHelpers.h>
-#include <cxxreact/JSExecutor.h>
 #include <cxxreact/JSCExecutor.h>
 #include <cxxreact/Platform.h>
 #include <fb/fbjni.h>
@@ -12,12 +11,11 @@
 #include <folly/dynamic.h>
 #include <jschelpers/Value.h>
 
+#include "AndroidJSCFactory.h"
 #include "CatalystInstanceImpl.h"
 #include "CxxModuleWrapper.h"
 #include "JavaScriptExecutorHolder.h"
 #include "JCallback.h"
-#include "JSCPerfLogging.h"
-#include "JSLogging.h"
 #include "ProxyExecutor.h"
 #include "WritableNativeArray.h"
 #include "WritableNativeMap.h"
@@ -40,9 +38,7 @@ class JSCJavaScriptExecutorHolder : public HybridClass<JSCJavaScriptExecutorHold
   static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/JSCJavaScriptExecutor;";
 
   static local_ref<jhybriddata> initHybrid(alias_ref<jclass>, ReadableNativeMap* jscConfig) {
-    // See JSCJavaScriptExecutor.Factory() for the other side of this hack.
+    return makeCxxInstance(makeAndroidJSCExecutorFactory(jscConfig->consume()));
-    folly::dynamic jscConfigMap = jscConfig->consume();
-    return makeCxxInstance(std::make_shared<JSCExecutorFactory>(std::move(jscConfigMap)));
   }
 
   static void registerNatives() {
@@ -83,116 +79,11 @@ class ProxyJavaScriptExecutorHolder : public HybridClass<ProxyJavaScriptExecutor
   using HybridBase::HybridBase;
 };
 
-class JReactMarker : public JavaClass<JReactMarker> {
- public:
-  static constexpr auto kJavaDescriptor = "Lcom/facebook/react/bridge/ReactMarker;";
-
-  static void logMarker(const std::string& marker) {
-    static auto cls = javaClassStatic();
-    static auto meth = cls->getStaticMethod<void(std::string)>("logMarker");
-    meth(cls, marker);
-  }
-
-  static void logMarker(const std::string& marker, const std::string& tag) {
-    static auto cls = javaClassStatic();
-    static auto meth = cls->getStaticMethod<void(std::string, std::string)>("logMarker");
-    meth(cls, marker, tag);
-  }
-};
-
-static JSValueRef nativePerformanceNow(
-    JSContextRef ctx,
-    JSObjectRef function,
-    JSObjectRef thisObject,
-    size_t argumentCount,
-    const JSValueRef arguments[], JSValueRef *exception) {
-  static const int64_t NANOSECONDS_IN_SECOND = 1000000000LL;
-  static const int64_t NANOSECONDS_IN_MILLISECOND = 1000000LL;
-
-  // Since SystemClock.uptimeMillis() is commonly used for performance measurement in Java
-  // and uptimeMillis() internally uses clock_gettime(CLOCK_MONOTONIC),
-  // we use the same API here.
-  // We need that to make sure we use the same time system on both JS and Java sides.
-  // Links to the source code:
-  // https://android.googlesource.com/platform/frameworks/native/+/jb-mr1-release/libs/utils/SystemClock.cpp
-  // https://android.googlesource.com/platform/system/core/+/master/libutils/Timers.cpp
-  struct timespec now;
-  clock_gettime(CLOCK_MONOTONIC, &now);
-  int64_t nano = now.tv_sec * NANOSECONDS_IN_SECOND + now.tv_nsec;
-  return Value::makeNumber(ctx, (nano / (double)NANOSECONDS_IN_MILLISECOND));
-}
-
-static void logPerfMarker(const ReactMarker::ReactMarkerId markerId, const char* tag) {
-  switch (markerId) {
-    case ReactMarker::RUN_JS_BUNDLE_START:
-      JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag);
-      break;
-    case ReactMarker::RUN_JS_BUNDLE_STOP:
-      JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag);
-      break;
-    case ReactMarker::CREATE_REACT_CONTEXT_STOP:
-      JReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
-      break;
-    case ReactMarker::JS_BUNDLE_STRING_CONVERT_START:
-      JReactMarker::logMarker("loadApplicationScript_startStringConvert");
-      break;
-    case ReactMarker::JS_BUNDLE_STRING_CONVERT_STOP:
-      JReactMarker::logMarker("loadApplicationScript_endStringConvert");
-      break;
-    case ReactMarker::NATIVE_MODULE_SETUP_START:
-      JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag);
-      break;
-    case ReactMarker::NATIVE_MODULE_SETUP_STOP:
-      JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag);
-      break;
-    case ReactMarker::NATIVE_REQUIRE_START:
-    case ReactMarker::NATIVE_REQUIRE_STOP:
-      // These are not used on Android.
-      break;
-  }
-}
-
-static ExceptionHandling::ExtractedEror extractJniError(const std::exception& ex, const char *context) {
-  auto jniEx = dynamic_cast<const jni::JniException *>(&ex);
-  if (!jniEx) {
-    return {};
-  }
-
-  auto stackTrace = jniEx->getThrowable()->getStackTrace();
-  std::ostringstream stackStr;
-  for (int i = 0, count = stackTrace->size(); i < count; ++i) {
-    auto frame = stackTrace->getElement(i);
-
-    auto methodName = folly::to<std::string>(frame->getClassName(), ".",
-      frame->getMethodName());
-
-    // Cut off stack traces at the Android looper, to keep them simple
-    if (methodName == "android.os.Looper.loop") {
-      break;
-    }
-
-    stackStr << std::move(methodName) << '@' << frame->getFileName();
-    if (frame->getLineNumber() > 0) {
-      stackStr << ':' << frame->getLineNumber();
-    }
-    stackStr << std::endl;
-  }
-
-  auto msg = folly::to<std::string>("Java exception in '", context, "'\n\n", jniEx->what());
-  return {.message = msg, .stack = stackStr.str()};
-}
-
 }
 
 extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
   return initialize(vm, [] {
     gloginit::initialize();
-    // Inject some behavior into react/
-    ReactMarker::logTaggedMarker = logPerfMarker;
-    ExceptionHandling::platformErrorExtractor = extractJniError;
-    JSCNativeHooks::loggingHook = nativeLoggingHook;
-    JSCNativeHooks::nowHook = nativePerformanceNow;
-    JSCNativeHooks::installPerfHooks = addNativePerfLoggingHooks;
     JSCJavaScriptExecutorHolder::registerNatives();
     ProxyJavaScriptExecutorHolder::registerNatives();
     CatalystInstanceImpl::registerNatives();

ReactCommon/cxxreact/BUCK

@@ -122,28 +122,11 @@ rn_xplat_cxx_library(
         "-DWITH_JSC_MEMORY_PRESSURE=1",
         "-DWITH_FB_MEMORY_PROFILING=1",
     ],
-    fbandroid_visibility = [
-        # TL;DR: If you depend on this target directly, you're gonna have a
-        # Bad Time(TM).
-        #
-        # `facebook::react::JSCExecutor::initOnJSVMThread` (in `:bridge`) does
-        # some platform-dependant setup. Exactly what setup to do is
-        # determined by some static functors, defined in `Platform.h`, which
-        # are initially `nullptr`. On Android, these functors are properly
-        # assigned as part of `react/jni`'s `JNI_OnLoad`.  By depending directly
-        # on the bridge, we can mess up the SO initialisation order, causing
-        # `initOnJSVMThread` to be called before the platform-specific hooks
-        # have been properly initialised. Bad Times(TM).
-        #     -- @ashokmenon (2017/01/03)
-        react_native_target("jni/react/jni:jni"),
-        react_native_xplat_target("cxxreact/..."),
-    ],
     fbobjc_frameworks = [
         "$SDKROOT/System/Library/Frameworks/JavaScriptCore.framework",
     ],
     fbobjc_inherited_buck_flags = STATIC_LIBRARY_IOS_FLAGS,
     fbobjc_preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS,
-    fbobjc_visibility = ["PUBLIC"],
     force_static = True,
     preprocessor_flags = [
         "-DLOG_TAG=\"ReactNative\"",
@@ -152,6 +135,7 @@ rn_xplat_cxx_library(
     tests = [
         react_native_xplat_target("cxxreact/tests:tests"),
     ],
+    visibility = ["PUBLIC"],
     deps = [
         ":module",
         ":jsbigstring",