Getting rid of File descriptor based loading APIs

Reviewed By: javache

Differential Revision: D4333725

fbshipit-source-id: ba8591c7380e8eb90660a912f6fc86fc3d2d4774

ReactCommon/cxxreact/BUCK

@@ -40,7 +40,23 @@ if THIS_IS_FBANDROID:
           '-DWITH_FB_MEMORY_PROFILING=1',
           '-DWITH_INSPECTOR=1',
         ],
-        deps = JSC_DEPS
+        deps = JSC_DEPS,
+        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 `xreact`'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/xreact/jni:jni'),
+          react_native_xplat_target('cxxreact/...'),
+        ],
       )
     )
 
@@ -61,7 +77,8 @@ elif THIS_IS_FBOBJC:
         preprocessor_flags = DEBUG_PREPROCESSOR_FLAGS,
         deps = [
           '//xplat/folly:molly',
-        ]
+        ],
+        visibility = [ 'PUBLIC' ],
       )
     )
 
@@ -131,7 +148,6 @@ CXXREACT_PUBLIC_HEADERS = [
 ]
 
 react_library(
-  soname = 'libreactnativefb.$(ext)',
   header_namespace = 'cxxreact',
   force_static = True,
   srcs = glob(['*.cpp'], excludes=['SampleCxxModule.cpp']),
@@ -155,5 +171,4 @@ react_library(
     react_native_xplat_target('jschelpers:jschelpers'),
     react_native_xplat_target('microprofiler:microprofiler'),
   ],
-  visibility = [ 'PUBLIC' ],
 )

ReactCommon/cxxreact/Executor.cpp

@@ -23,12 +23,15 @@ void JSExecutor::loadApplicationScript(std::string bundlePath, std::string sourc
       std::move(sourceURL));
 }
 
-void JSExecutor::loadApplicationScript(int fd, std::string sourceURL) {
+std::unique_ptr<const JSBigFileString> JSBigFileString::fromPath(const std::string& sourceURL) {
+  int fd = ::open(sourceURL.c_str(), O_RDONLY);
+  folly::checkUnixError(fd, "Could not open file", sourceURL);
+  SCOPE_EXIT { CHECK(::close(fd) == 0); };
+
   struct stat fileInfo;
   folly::checkUnixError(::fstat(fd, &fileInfo), "fstat on bundle failed.");
 
-  auto bundle = folly::make_unique<JSBigFileString>(fd, fileInfo.st_size);
+  return folly::make_unique<const JSBigFileString>(fd, fileInfo.st_size);
-  return loadApplicationScript(std::move(bundle), std::move(sourceURL));
 }
 
 static JSBigOptimizedBundleString::Encoding encodingFromByte(uint8_t byte) {

ReactCommon/cxxreact/Executor.h

@@ -15,6 +15,8 @@
 
 #include "JSModulesUnbundle.h"
 
+#define RN_EXPORT __attribute__((visibility("default")))
+
 namespace facebook {
 namespace react {
 
@@ -150,7 +152,7 @@ private:
 };
 
 // JSBigString interface implemented by a file-backed mmap region.
-class JSBigFileString : public JSBigString {
+class RN_EXPORT JSBigFileString : public JSBigString {
 public:
 
   JSBigFileString(int fd, size_t size, off_t offset = 0)
@@ -204,6 +206,8 @@ public:
     return m_fd;
   }
 
+  static std::unique_ptr<const JSBigFileString> fromPath(const std::string& sourceURL);
+
 private:
   int m_fd;                     // The file descriptor being mmaped
   size_t m_size;                // The size of the mmaped region
@@ -292,14 +296,6 @@ public:
    */
   virtual void loadApplicationScript(std::string bundlePath, std::string source, int flags);
 
-  /**
-   * @experimental
-   *
-   * Read an app bundle from a file descriptor, determine how it should be
-   * loaded, load and execute it in the JS context.
-   */
-  virtual void loadApplicationScript(int fd, std::string source);
-
   /**
    * Add an application "unbundle" file
    */

ReactCommon/cxxreact/JSCExecutor.cpp

@@ -397,61 +397,55 @@ void JSCExecutor::loadApplicationScript(
 }
 #endif
 
-#ifdef WITH_FBJSCEXTENSIONS
-void JSCExecutor::loadApplicationScript(
-    int fd,
-    std::string sourceURL)
-{
-  String jsSourceURL(m_context, sourceURL.c_str());
-
-  JSLoadSourceError jsError;
-  auto bcSourceCode = JSCreateCompiledSourceCode(fd, jsSourceURL, &jsError);
-
-  switch (jsError) {
-  case JSLoadSourceErrorOnRead:
-  case JSLoadSourceErrorNotCompiled:
-    // Not bytecode, fall through.
-    return JSExecutor::loadApplicationScript(fd, sourceURL);
-
-  case JSLoadSourceErrorNone:
-    if (!bcSourceCode) {
-      throw std::runtime_error("Unexpected error opening compiled bundle");
-    }
-    break;
-
-  case JSLoadSourceErrorVersionMismatch:
-  case JSLoadSourceErrorUnknown:
-    throw RecoverableError(explainLoadSourceError(jsError));
-  }
-
-  ReactMarker::logMarker("RUN_JS_BUNDLE_START");
-
-  evaluateSourceCode(m_context, bcSourceCode, jsSourceURL);
-
-  // TODO(luk): t13903306 Remove this check once we make native modules
-  // working for java2js
-  if (m_delegate) {
-    bindBridge();
-    flush();
-  }
-
-  ReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
-  ReactMarker::logMarker("RUN_JS_BUNDLE_END");
-}
-#endif
-
 void JSCExecutor::loadApplicationScript(std::unique_ptr<const JSBigString> script, std::string sourceURL) throw(JSException) {
   SystraceSection s("JSCExecutor::loadApplicationScript",
                     "sourceURL", sourceURL);
 
+  ReactMarker::logMarker("RUN_JS_BUNDLE_START");
+  String jsSourceURL(m_context, sourceURL.c_str());
+
+#ifdef WITH_FBJSCEXTENSIONS
+  if (auto fileStr = dynamic_cast<const JSBigFileString *>(script.get())) {
+    JSLoadSourceError jsError;
+    auto bcSourceCode = JSCreateCompiledSourceCode(fileStr->fd(), jsSourceURL, &jsError);
+
+    switch (jsError) {
+    case JSLoadSourceErrorNone:
+      if (!bcSourceCode) {
+        throw std::runtime_error("Unexpected error opening compiled bundle");
+      }
+
+      evaluateSourceCode(m_context, bcSourceCode, jsSourceURL);
+
+      // TODO(luk): t13903306 Remove this check once we make native modules
+      // working for java2js
+      if (m_delegate) {
+        bindBridge();
+        flush();
+      }
+
+      ReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
+      ReactMarker::logMarker("RUN_JS_BUNDLE_END");
+      return;
+
+    case JSLoadSourceErrorVersionMismatch:
+    case JSLoadSourceErrorUnknown:
+      throw RecoverableError(explainLoadSourceError(jsError));
+
+    case JSLoadSourceErrorOnRead:
+    case JSLoadSourceErrorNotCompiled:
+      // Not bytecode, fall through.
+      break;
+    }
+  }
+#endif
+
   #ifdef WITH_FBSYSTRACE
   fbsystrace_begin_section(
     TRACE_TAG_REACT_CXX_BRIDGE,
     "JSCExecutor::loadApplicationScript-createExpectingAscii");
   #endif
 
-  ReactMarker::logMarker("RUN_JS_BUNDLE_START");
-
   ReactMarker::logMarker("loadApplicationScript_startStringConvert");
   String jsScript = jsStringFromBigString(m_context, *script);
   ReactMarker::logMarker("loadApplicationScript_endStringConvert");
@@ -460,7 +454,6 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr<const JSBigString> scrip
   fbsystrace_end_section(TRACE_TAG_REACT_CXX_BRIDGE);
   #endif
 
-  String jsSourceURL(m_context, sourceURL.c_str());
   evaluateScript(m_context, jsScript, jsSourceURL);
 
   // TODO(luk): t13903306 Remove this check once we make native modules working for java2js

ReactCommon/cxxreact/JSCExecutor.h

@@ -22,9 +22,7 @@ namespace react {
 
 class MessageQueueThread;
 
-#define RN_JSC_EXECUTOR_EXPORT __attribute__((visibility("default")))
+class RN_EXPORT JSCExecutorFactory : public JSExecutorFactory {
-
-class RN_JSC_EXECUTOR_EXPORT JSCExecutorFactory : public JSExecutorFactory {
 public:
   JSCExecutorFactory(const std::string& cacheDir, const folly::dynamic& jscConfig) :
   m_cacheDir(cacheDir),
@@ -51,7 +49,7 @@ public:
 template <typename T>
 struct ValueEncoder;
 
-class RN_JSC_EXECUTOR_EXPORT JSCExecutor : public JSExecutor {
+class RN_EXPORT JSCExecutor : public JSExecutor {
 public:
   /**
    * Must be invoked from thread this Executor will run on.
@@ -73,12 +71,6 @@ public:
     int flags) override;
 #endif
 
-#ifdef WITH_FBJSCEXTENSIONS
-  virtual void loadApplicationScript(
-    int fd,
-    std::string sourceURL) override;
-#endif
-
   virtual void setJSModulesUnbundle(
     std::unique_ptr<JSModulesUnbundle> unbundle) override;