diff --git a/React/CxxBridge/RCTCxxBridge.mm b/React/CxxBridge/RCTCxxBridge.mm index dd92f7df2b1..e1db3bdf24c 100644 --- a/React/CxxBridge/RCTCxxBridge.mm +++ b/React/CxxBridge/RCTCxxBridge.mm @@ -1065,11 +1065,11 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR [self->_performanceLogger setValue:scriptStr->size() forTag:RCTPLRAMStartupCodeSize]; if (self->_reactInstance) { self->_reactInstance->loadUnbundle(std::move(ramBundle), std::move(scriptStr), - [[url absoluteString] UTF8String]); + [[url absoluteString] UTF8String], false); } } else if (self->_reactInstance) { self->_reactInstance->loadScriptFromString(std::make_unique(script), - [[url absoluteString] UTF8String]); + [[url absoluteString] UTF8String], false); } else { throw std::logic_error("Attempt to call loadApplicationScript: on uninitialized bridge"); } @@ -1094,12 +1094,12 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR [self->_performanceLogger markStopForTag:RCTPLRAMBundleLoad]; [self->_performanceLogger setValue:scriptStr->size() forTag:RCTPLRAMStartupCodeSize]; if (self->_reactInstance) { - self->_reactInstance->loadUnbundleSync(std::move(ramBundle), std::move(scriptStr), - [[url absoluteString] UTF8String]); + self->_reactInstance->loadUnbundle(std::move(ramBundle), std::move(scriptStr), + [[url absoluteString] UTF8String], true); } } else if (self->_reactInstance) { - self->_reactInstance->loadScriptFromStringSync(std::make_unique(script), - [[url absoluteString] UTF8String]); + self->_reactInstance->loadScriptFromString(std::make_unique(script), + [[url absoluteString] UTF8String], true); } else { throw std::logic_error("Attempt to call loadApplicationScriptSync: on uninitialized bridge"); } diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java index f37c9a83f95..1349655a0a7 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactTestHelper.java @@ -89,8 +89,9 @@ public class ReactTestHelper { .setRegistry(mNativeModuleRegistryBuilder.build()) .setJSModuleRegistry(mJSModuleRegistryBuilder.build()) .setJSBundleLoader(JSBundleLoader.createAssetLoader( - mContext, - "assets://AndroidTestBundle.js")) + mContext, + "assets://AndroidTestBundle.js", + false/* Asynchronous */)) .setNativeModuleCallExceptionHandler( new NativeModuleCallExceptionHandler() { @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java index 941c204bd46..322bf7cd768 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerBuilder.java @@ -233,7 +233,8 @@ public class ReactInstanceManagerBuilder { mCurrentActivity, mDefaultHardwareBackBtnHandler, (mJSBundleLoader == null && mJSBundleAssetUrl != null) ? - JSBundleLoader.createAssetLoader(mApplication, mJSBundleAssetUrl) : mJSBundleLoader, + JSBundleLoader.createAssetLoader(mApplication, mJSBundleAssetUrl, false /*Asynchronous*/) : + mJSBundleLoader, mJSMainModuleName, mPackages, mUseDeveloperSupport, diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java index a9f5c880d6d..c6bab4da62b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -27,6 +27,9 @@ public interface CatalystInstance extends MemoryPressureListener, JSInstance { void runJSBundle(); + // Returns the status of running the JS bundle; waits for an answer if runJSBundle is running + boolean hasRunJSBundle(); + /** * Return the source URL of the JS Bundle that was run, or {@code null} if no JS * bundle has been run yet. diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index d3a312b886a..a656782fb50 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -85,6 +85,7 @@ public class CatalystInstanceImpl implements CatalystInstance { private boolean mInitialized = false; private volatile boolean mAcceptCalls = false; + private boolean mJSBundleHasStartedLoading; private boolean mJSBundleHasLoaded; private @Nullable String mSourceURL; @@ -185,29 +186,31 @@ public class CatalystInstanceImpl implements CatalystInstance { jniSetSourceURL(remoteURL); } - /* package */ void loadScriptFromAssets(AssetManager assetManager, String assetURL) { + /* package */ void loadScriptFromAssets(AssetManager assetManager, String assetURL, boolean loadSynchronously) { mSourceURL = assetURL; - jniLoadScriptFromAssets(assetManager, assetURL); + jniLoadScriptFromAssets(assetManager, assetURL, loadSynchronously); } - /* package */ void loadScriptFromFile(String fileName, String sourceURL) { + /* package */ void loadScriptFromFile(String fileName, String sourceURL, boolean loadSynchronously) { mSourceURL = sourceURL; - jniLoadScriptFromFile(fileName, sourceURL); + jniLoadScriptFromFile(fileName, sourceURL, loadSynchronously); } private native void jniSetSourceURL(String sourceURL); - private native void jniLoadScriptFromAssets(AssetManager assetManager, String assetURL); - private native void jniLoadScriptFromFile(String fileName, String sourceURL); + private native void jniLoadScriptFromAssets(AssetManager assetManager, String assetURL, boolean loadSynchronously); + private native void jniLoadScriptFromFile(String fileName, String sourceURL, boolean loadSynchronously); @Override public void runJSBundle() { Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!"); - mJSBundleHasLoaded = true; + + mJSBundleHasStartedLoading = true; // incrementPendingJSCalls(); mJSBundleLoader.loadScript(CatalystInstanceImpl.this); synchronized (mJSCallsPendingInitLock) { + // Loading the bundle is queued on the JS thread, but may not have // run yet. It's safe to set this here, though, since any work it // gates will be queued on the JS thread behind the load. @@ -217,12 +220,20 @@ public class CatalystInstanceImpl implements CatalystInstance { jniCallJSFunction(call.mModule, call.mMethod, call.mArguments); } mJSCallsPendingInit.clear(); + mJSBundleHasLoaded = true; } // This is registered after JS starts since it makes a JS call Systrace.registerListener(mTraceListener); } + @Override + public boolean hasRunJSBundle() { + synchronized (mJSCallsPendingInitLock) { + return mJSBundleHasLoaded && mAcceptCalls; + } + } + @Override public @Nullable String getSourceURL() { return mSourceURL; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java index 965d65bec4f..0597dc8f109 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java @@ -26,11 +26,12 @@ public abstract class JSBundleLoader { */ public static JSBundleLoader createAssetLoader( final Context context, - final String assetUrl) { + final String assetUrl, + final boolean loadSynchronously) { return new JSBundleLoader() { @Override public String loadScript(CatalystInstanceImpl instance) { - instance.loadScriptFromAssets(context.getAssets(), assetUrl); + instance.loadScriptFromAssets(context.getAssets(), assetUrl, loadSynchronously); return assetUrl; } }; @@ -41,16 +42,17 @@ public abstract class JSBundleLoader { * passing large strings from java to native memorory. */ public static JSBundleLoader createFileLoader(final String fileName) { - return createFileLoader(fileName, fileName); + return createFileLoader(fileName, fileName, false); } public static JSBundleLoader createFileLoader( final String fileName, - final String assetUrl) { + final String assetUrl, + final boolean loadSynchronously) { return new JSBundleLoader() { @Override public String loadScript(CatalystInstanceImpl instance) { - instance.loadScriptFromFile(fileName, assetUrl); + instance.loadScriptFromFile(fileName, assetUrl, loadSynchronously); return fileName; } }; @@ -70,7 +72,7 @@ public abstract class JSBundleLoader { @Override public String loadScript(CatalystInstanceImpl instance) { try { - instance.loadScriptFromFile(cachedFileLocation, sourceURL); + instance.loadScriptFromFile(cachedFileLocation, sourceURL, false); return sourceURL; } catch (Exception e) { throw DebugServerException.makeGeneric(e.getMessage(), e); diff --git a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp index 2b7dfd35c99..47a38c8e7d2 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -166,7 +167,8 @@ void CatalystInstanceImpl::jniSetSourceURL(const std::string& sourceURL) { void CatalystInstanceImpl::jniLoadScriptFromAssets( jni::alias_ref assetManager, - const std::string& assetURL) { + const std::string& assetURL, + bool loadSynchronously) { const int kAssetsLength = 9; // strlen("assets://"); auto sourceURL = assetURL.substr(kAssetsLength); @@ -176,10 +178,11 @@ void CatalystInstanceImpl::jniLoadScriptFromAssets( instance_->loadUnbundle( folly::make_unique(manager, sourceURL), std::move(script), - sourceURL); + sourceURL, + loadSynchronously); return; } else { - instance_->loadScriptFromString(std::move(script), sourceURL); + instance_->loadScriptFromString(std::move(script), sourceURL, loadSynchronously); } } @@ -195,7 +198,8 @@ bool CatalystInstanceImpl::isIndexedRAMBundle(const char *sourcePath) { } void CatalystInstanceImpl::jniLoadScriptFromFile(const std::string& fileName, - const std::string& sourceURL) { + const std::string& sourceURL, + bool loadSynchronously) { auto zFileName = fileName.c_str(); if (isIndexedRAMBundle(zFileName)) { auto bundle = folly::make_unique(zFileName); @@ -203,9 +207,15 @@ void CatalystInstanceImpl::jniLoadScriptFromFile(const std::string& fileName, instance_->loadUnbundle( std::move(bundle), std::move(startupScript), - sourceURL); + sourceURL, + loadSynchronously); } else { - instance_->loadScriptFromFile(fileName, sourceURL); + std::unique_ptr script; + RecoverableError::runRethrowingAsRecoverable( + [&fileName, &script]() { + script = JSBigFileString::fromPath(fileName); + }); + instance_->loadScriptFromString(std::move(script), sourceURL, loadSynchronously); } } diff --git a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h index 774be5f85d7..9679339c38c 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h +++ b/ReactAndroid/src/main/jni/xreact/jni/CatalystInstanceImpl.h @@ -57,8 +57,8 @@ class CatalystInstanceImpl : public jni::HybridClass { */ void jniSetSourceURL(const std::string& sourceURL); - void jniLoadScriptFromAssets(jni::alias_ref assetManager, const std::string& assetURL); - void jniLoadScriptFromFile(const std::string& fileName, const std::string& sourceURL); + void jniLoadScriptFromAssets(jni::alias_ref assetManager, const std::string& assetURL, bool loadSynchronously); + void jniLoadScriptFromFile(const std::string& fileName, const std::string& sourceURL, bool loadSynchronously); void jniCallJSFunction(std::string module, std::string method, NativeArray* arguments); void jniCallJSCallback(jint callbackId, NativeArray* arguments); void setGlobalVariable(std::string propName, diff --git a/ReactAndroid/src/main/jni/xreact/jni/JSLoader.cpp b/ReactAndroid/src/main/jni/xreact/jni/JSLoader.cpp index e1ccb5c71f4..c8c31d5fd4c 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JSLoader.cpp +++ b/ReactAndroid/src/main/jni/xreact/jni/JSLoader.cpp @@ -58,25 +58,4 @@ std::unique_ptr loadScriptFromAssets( "'. Make sure your bundle is packaged correctly or you're running a packager server.")); } -std::string loadScriptFromFile(const std::string& fileName) { - #ifdef WITH_FBSYSTRACE - FbSystraceSection s(TRACE_TAG_REACT_CXX_BRIDGE, "reactbridge_jni_loadScriptFromFile", - "fileName", fileName); - #endif - std::ifstream jsfile(fileName); - if (jsfile) { - std::string output; - jsfile.seekg(0, std::ios::end); - output.reserve(jsfile.tellg()); - jsfile.seekg(0, std::ios::beg); - output.assign( - (std::istreambuf_iterator(jsfile)), - std::istreambuf_iterator()); - return output; - } - - throw std::runtime_error(folly::to("Unable to load script from file: '", fileName, - "'. Make sure your bundle is packaged correctly or you're running a packager server.")); -} - -} } +}} diff --git a/ReactAndroid/src/main/jni/xreact/jni/JSLoader.h b/ReactAndroid/src/main/jni/xreact/jni/JSLoader.h index 1186926d470..3c3e15720f7 100644 --- a/ReactAndroid/src/main/jni/xreact/jni/JSLoader.h +++ b/ReactAndroid/src/main/jni/xreact/jni/JSLoader.h @@ -22,9 +22,4 @@ AAssetManager *extractAssetManager(jni::alias_ref ass std::unique_ptr loadScriptFromAssets(AAssetManager *assetManager, const std::string& assetName); -/** - * Helper method for loading JS script from a file - */ -std::string loadScriptFromFile(const std::string& fileName); - } } diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index 5cef8b4a924..24aa653d2e8 100644 --- a/ReactCommon/cxxreact/Instance.cpp +++ b/ReactCommon/cxxreact/Instance.cpp @@ -20,8 +20,6 @@ namespace facebook { namespace react { -using namespace detail; - Instance::~Instance() { if (nativeToJsBridge_) { nativeToJsBridge_->destroy(); @@ -48,64 +46,56 @@ void Instance::initializeBridge( CHECK(nativeToJsBridge_); } +void Instance::loadApplication( + std::unique_ptr unbundle, + std::unique_ptr string, + std::string sourceURL) { + callback_->incrementPendingJSCalls(); + SystraceSection s("reactbridge_xplat_loadApplication", "sourceURL", sourceURL); + nativeToJsBridge_->loadApplication(std::move(unbundle), std::move(string), std::move(sourceURL)); +} + +void Instance::loadApplicationSync( + std::unique_ptr unbundle, + std::unique_ptr string, + std::string sourceURL) { + std::unique_lock lock(m_syncMutex); + m_syncCV.wait(lock, [this] { return m_syncReady; }); + + SystraceSection s("reactbridge_xplat_loadApplicationSync", "sourceURL", sourceURL); + nativeToJsBridge_->loadApplicationSync(std::move(unbundle), std::move(string), std::move(sourceURL)); +} + void Instance::setSourceURL(std::string sourceURL) { callback_->incrementPendingJSCalls(); - SystraceSection s("reactbridge_xplat_setSourceURL", - "sourceURL", sourceURL); + SystraceSection s("reactbridge_xplat_setSourceURL", "sourceURL", sourceURL); nativeToJsBridge_->loadApplication(nullptr, nullptr, std::move(sourceURL)); } void Instance::loadScriptFromString(std::unique_ptr string, - std::string sourceURL) { - callback_->incrementPendingJSCalls(); - SystraceSection s("reactbridge_xplat_loadScriptFromString", - "sourceURL", sourceURL); - nativeToJsBridge_->loadApplication(nullptr, std::move(string), std::move(sourceURL)); -} - -void Instance::loadScriptFromStringSync(std::unique_ptr string, - std::string sourceURL) { - std::unique_lock lock(m_syncMutex); - m_syncCV.wait(lock, [this] { return m_syncReady; }); - - nativeToJsBridge_->loadApplicationSync(nullptr, std::move(string), std::move(sourceURL)); -} - -void Instance::loadScriptFromFile(const std::string& filename, - const std::string& sourceURL) { - callback_->incrementPendingJSCalls(); - SystraceSection s("reactbridge_xplat_loadScriptFromFile", - "fileName", filename); - - std::unique_ptr script; - - RecoverableError::runRethrowingAsRecoverable( - [&filename, &script]() { - script = JSBigFileString::fromPath(filename); - }); - - nativeToJsBridge_->loadApplication(nullptr, std::move(script), sourceURL); + std::string sourceURL, + bool loadSynchronously) { + SystraceSection s("reactbridge_xplat_loadScriptFromString", "sourceURL", sourceURL); + if (loadSynchronously) { + loadApplicationSync(nullptr, std::move(string), std::move(sourceURL)); + } else { + loadApplicationSync(nullptr, std::move(string), std::move(sourceURL)); + } } void Instance::loadUnbundle(std::unique_ptr unbundle, std::unique_ptr startupScript, - std::string startupScriptSourceURL) { - callback_->incrementPendingJSCalls(); - nativeToJsBridge_->loadApplication(std::move(unbundle), std::move(startupScript), - std::move(startupScriptSourceURL)); -} - -void Instance::loadUnbundleSync(std::unique_ptr unbundle, - std::unique_ptr startupScript, - std::string startupScriptSourceURL) { - std::unique_lock lock(m_syncMutex); - m_syncCV.wait(lock, [this] { return m_syncReady; }); - - SystraceSection s("reactbridge_xplat_loadApplicationSync"); - nativeToJsBridge_->loadApplicationSync(std::move(unbundle), std::move(startupScript), - std::move(startupScriptSourceURL)); -} + std::string startupScriptSourceURL, + bool loadSynchronously) { + if (loadSynchronously) { + loadApplicationSync(std::move(unbundle), std::move(startupScript), + std::move(startupScriptSourceURL)); + } else { + loadApplication(std::move(unbundle), std::move(startupScript), + std::move(startupScriptSourceURL)); + } + } bool Instance::supportsProfiling() { return nativeToJsBridge_->supportsProfiling(); diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index 3a1619e197a..ca875c56901 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -32,17 +32,15 @@ class Instance { void setSourceURL(std::string sourceURL); - void loadScriptFromString(std::unique_ptr string, std::string sourceURL); - void loadScriptFromStringSync(std::unique_ptr string, std::string sourceURL); - void loadScriptFromFile(const std::string& filename, const std::string& sourceURL); + void loadScriptFromString( + std::unique_ptr string, + std::string sourceURL, + bool loadSynchronously); void loadUnbundle( std::unique_ptr unbundle, std::unique_ptr startupScript, - std::string startupScriptSourceURL); - void loadUnbundleSync( - std::unique_ptr unbundle, - std::unique_ptr startupScript, - std::string startupScriptSourceURL); + std::string startupScriptSourceURL, + bool loadSynchronously); bool supportsProfiling(); void startProfiler(const std::string& title); void stopProfiler(const std::string& title, const std::string& filename); @@ -64,6 +62,14 @@ class Instance { private: void callNativeModules(folly::dynamic&& calls, bool isEndOfBatch); + void loadApplication( + std::unique_ptr unbundle, + std::unique_ptr startupScript, + std::string startupScriptSourceURL); + void loadApplicationSync( + std::unique_ptr unbundle, + std::unique_ptr startupScript, + std::string startupScriptSourceURL); std::shared_ptr callback_; std::unique_ptr nativeToJsBridge_; diff --git a/ReactCommon/cxxreact/JSCExecutor.cpp b/ReactCommon/cxxreact/JSCExecutor.cpp index 681914fa0b6..17b099ddcea 100644 --- a/ReactCommon/cxxreact/JSCExecutor.cpp +++ b/ReactCommon/cxxreact/JSCExecutor.cpp @@ -65,8 +65,6 @@ namespace facebook { namespace react { -using namespace detail; - namespace { template diff --git a/ReactCommon/cxxreact/RecoverableError.h b/ReactCommon/cxxreact/RecoverableError.h index f1775043f63..d71e1c95aa0 100644 --- a/ReactCommon/cxxreact/RecoverableError.h +++ b/ReactCommon/cxxreact/RecoverableError.h @@ -8,7 +8,6 @@ namespace facebook { namespace react { -namespace detail { /** * RecoverableError @@ -42,6 +41,5 @@ private: std::string m_what; }; -} // namespace detail } // namespace react } // namespace facebook diff --git a/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp b/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp index 94db13e535b..8b2f39f6b12 100644 --- a/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp +++ b/ReactCommon/cxxreact/tests/RecoverableErrorTest.cpp @@ -7,7 +7,7 @@ #include -using namespace facebook::react::detail; +using namespace facebook::react; TEST(RecoverableError, RunRethrowingAsRecoverableRecoverTest) { try {