From 8819bef56bb860d4ec10366e414c29f1fac6a84a Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Tue, 10 Jan 2017 07:04:49 -0800 Subject: [PATCH] Getting rid of File descriptor based loading APIs Reviewed By: javache Differential Revision: D4333725 fbshipit-source-id: ba8591c7380e8eb90660a912f6fc86fc3d2d4774 --- ReactCommon/cxxreact/BUCK | 23 ++++++-- ReactCommon/cxxreact/Executor.cpp | 9 ++- ReactCommon/cxxreact/Executor.h | 14 ++--- ReactCommon/cxxreact/JSCExecutor.cpp | 85 +++++++++++++--------------- ReactCommon/cxxreact/JSCExecutor.h | 12 +--- 5 files changed, 71 insertions(+), 72 deletions(-) diff --git a/ReactCommon/cxxreact/BUCK b/ReactCommon/cxxreact/BUCK index 84124e1b5..c020a0511 100644 --- a/ReactCommon/cxxreact/BUCK +++ b/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' ], ) diff --git a/ReactCommon/cxxreact/Executor.cpp b/ReactCommon/cxxreact/Executor.cpp index adbf20587..63110f748 100644 --- a/ReactCommon/cxxreact/Executor.cpp +++ b/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 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(fd, fileInfo.st_size); - return loadApplicationScript(std::move(bundle), std::move(sourceURL)); + return folly::make_unique(fd, fileInfo.st_size); } static JSBigOptimizedBundleString::Encoding encodingFromByte(uint8_t byte) { diff --git a/ReactCommon/cxxreact/Executor.h b/ReactCommon/cxxreact/Executor.h index 5eaa853d9..3db53ffe6 100644 --- a/ReactCommon/cxxreact/Executor.h +++ b/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 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 */ diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 136e14732..d612cd672 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/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 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(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 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 diff --git a/ReactCommon/cxxreact/JSCExecutor.h b/ReactCommon/cxxreact/JSCExecutor.h index 9c7c87f8d..d631e0056 100644 --- a/ReactCommon/cxxreact/JSCExecutor.h +++ b/ReactCommon/cxxreact/JSCExecutor.h @@ -22,9 +22,7 @@ namespace react { class MessageQueueThread; -#define RN_JSC_EXECUTOR_EXPORT __attribute__((visibility("default"))) - -class RN_JSC_EXECUTOR_EXPORT JSCExecutorFactory : public JSExecutorFactory { +class RN_EXPORT JSCExecutorFactory : public JSExecutorFactory { public: JSCExecutorFactory(const std::string& cacheDir, const folly::dynamic& jscConfig) : m_cacheDir(cacheDir), @@ -51,7 +49,7 @@ public: template 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 unbundle) override;