Move thread jump for js loading into NativeToJSBridge, out of platform code

Reviewed By: javache

Differential Revision: D3906009

fbshipit-source-id: b9782a6c209e3c1626899dac7fd50233cdef87f3

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

@@ -18,8 +18,6 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
 
 import android.app.Activity;
 import android.app.Application;
@@ -924,33 +922,8 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;
       catalystInstance.addBridgeIdleDebugListener(mBridgeIdleDebugListener);
     }
 
-    ReactMarker.logMarker(RUN_JS_BUNDLE_START);
-    try {
-      catalystInstance.getReactQueueConfiguration().getJSQueueThread().callOnQueue(
-        new Callable<Void>() {
-          @Override
-          public Void call() throws Exception {
     reactContext.initializeWithInstance(catalystInstance);
-
-            Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "runJSBundle");
-            try {
     catalystInstance.runJSBundle();
-            } finally {
-              Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
-              ReactMarker.logMarker(RUN_JS_BUNDLE_END);
-            }
-            return null;
-          }
-        }).get();
-    } 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/CatalystInstanceImpl.java

@@ -167,14 +167,14 @@ public class CatalystInstanceImpl implements CatalystInstance {
 
   @Override
   public void runJSBundle() {
-    // This should really be done when we post the task that runs the JS bundle
-    // (don't even need to wait for it to finish). Since that is currently done
-    // synchronously, marking it here is fine.
-    mAcceptCalls = true;
     Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");
     mJSBundleHasLoaded = true;
     // incrementPendingJSCalls();
     mJSBundleLoader.loadScript(CatalystInstanceImpl.this);
+    // Loading the bundle is queued on the JS thread, but may not have
+    // run yet.  It's save to set this here, though, since any work it
+    // gates will be queued on the JS thread behind the load.
+    mAcceptCalls = true;
     // This is registered after JS starts since it makes a JS call
     Systrace.registerListener(mTraceListener);
   }

ReactCommon/cxxreact/Instance.cpp

@@ -49,8 +49,7 @@ void Instance::loadScriptFromString(std::unique_ptr<const JSBigString> string,
   callback_->incrementPendingJSCalls();
   SystraceSection s("reactbridge_xplat_loadScriptFromString",
                     "sourceURL", sourceURL);
-  // TODO mhorowitz: ReactMarker around loadApplicationScript
+  nativeToJsBridge_->loadApplication(nullptr, std::move(string), std::move(sourceURL));
-  nativeToJsBridge_->loadApplicationScript(std::move(string), std::move(sourceURL));
 }
 
 void Instance::loadScriptFromFile(const std::string& filename,
@@ -90,7 +89,7 @@ void Instance::loadUnbundle(std::unique_ptr<JSModulesUnbundle> unbundle,
                             std::string startupScriptSourceURL) {
   callback_->incrementPendingJSCalls();
   SystraceSection s("reactbridge_xplat_setJSModulesUnbundle");
-  nativeToJsBridge_->loadApplicationUnbundle(std::move(unbundle), std::move(startupScript),
+  nativeToJsBridge_->loadApplication(std::move(unbundle), std::move(startupScript),
                                      std::move(startupScriptSourceURL));
 }
 

ReactCommon/cxxreact/JSCExecutor.cpp

@@ -271,6 +271,8 @@ void JSCExecutor::loadApplicationScript(
     return loadApplicationScript(std::move(jsScriptBigString), sourceURL);
   }
 
+  ReactMarker::logMarker("RUN_JS_BUNDLE_START");
+
   if (flags & UNPACKED_BC_CACHE) {
     configureJSCBCCache(m_context, bundlePath);
   }
@@ -290,6 +292,7 @@ void JSCExecutor::loadApplicationScript(
 
   flush();
   ReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
+  ReactMarker::logMarker("RUN_JS_BUNDLE_END");
 }
 #endif
 
@@ -303,6 +306,8 @@ void JSCExecutor::loadApplicationScript(std::unique_ptr<const JSBigString> scrip
     "JSCExecutor::loadApplicationScript-createExpectingAscii");
   #endif
 
+  ReactMarker::logMarker("RUN_JS_BUNDLE_START");
+
   ReactMarker::logMarker("loadApplicationScript_startStringConvert");
   String jsScript = jsStringFromBigString(*script);
   ReactMarker::logMarker("loadApplicationScript_endStringConvert");
@@ -313,11 +318,11 @@ 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");
+  ReactMarker::logMarker("RUN_JS_BUNDLE_END");
 }
 
 void JSCExecutor::setJSModulesUnbundle(std::unique_ptr<JSModulesUnbundle> unbundle) {

ReactCommon/cxxreact/NativeToJsBridge.cpp

@@ -118,12 +118,6 @@ NativeToJsBridge::~NativeToJsBridge() {
     "NativeToJsBridge::destroy() must be called before deallocating the NativeToJsBridge!";
 }
 
-void NativeToJsBridge::loadApplicationScript(std::unique_ptr<const JSBigString> script,
-                                             std::string sourceURL) {
-  // TODO(t11144533): Add assert that we are on the correct thread
-  m_mainExecutor->loadApplicationScript(std::move(script), std::move(sourceURL));
-}
-
 void NativeToJsBridge::loadOptimizedApplicationScript(
     std::string bundlePath,
     std::string sourceURL,
@@ -138,18 +132,21 @@ void NativeToJsBridge::loadOptimizedApplicationScript(
   });
 }
 
-void NativeToJsBridge::loadApplicationUnbundle(
+void NativeToJsBridge::loadApplication(
     std::unique_ptr<JSModulesUnbundle> unbundle,
     std::unique_ptr<const JSBigString> startupScript,
     std::string startupScriptSourceURL) {
   runOnExecutorQueue(
       m_mainExecutorToken,
-      [unbundle=folly::makeMoveWrapper(std::move(unbundle)),
+      [unbundleWrap=folly::makeMoveWrapper(std::move(unbundle)),
        startupScript=folly::makeMoveWrapper(std::move(startupScript)),
        startupScriptSourceURL=std::move(startupScriptSourceURL)]
         (JSExecutor* executor) mutable {
 
-    executor->setJSModulesUnbundle(unbundle.move());
+    auto unbundle = unbundleWrap.move();
+    if (unbundle) {
+      executor->setJSModulesUnbundle(std::move(unbundle));
+    }
     executor->loadApplicationScript(std::move(*startupScript),
                                     std::move(startupScriptSourceURL));
   });

ReactCommon/cxxreact/NativeToJsBridge.h

@@ -78,11 +78,14 @@ public:
   void invokeCallback(ExecutorToken executorToken, double callbackId, folly::dynamic&& args);
 
   /**
-   * Starts the JS application from an "bundle", i.e. a JavaScript file that
+   * Starts the JS application.  If unbundle is non-null, then it is
-   * contains code for all modules and a runtime that resolves and
+   * used to fetch JavaScript modules as individual scripts.
-   * executes modules.
+   * Otherwise, the script is assumed to include all the modules.
    */
-  void loadApplicationScript(std::unique_ptr<const JSBigString> script, std::string sourceURL);
+  void loadApplication(
+    std::unique_ptr<JSModulesUnbundle> unbundle,
+    std::unique_ptr<const JSBigString> startupCode,
+    std::string sourceURL);
 
   /**
    * Similar to loading a "bundle", but instead of passing js source this method accepts
@@ -90,17 +93,6 @@ public:
    */
   void loadOptimizedApplicationScript(std::string bundlePath, std::string sourceURL, int flags);
 
-  /**
-   * An "unbundle" is a backend that stores and injects JavaScript modules as
-   * individual scripts, rather than bundling all of them into a single scrupt.
-   *
-   * Loading an unbundle means setting the storage backend and executing the
-   * startup script.
-   */
-  void loadApplicationUnbundle(
-    std::unique_ptr<JSModulesUnbundle> unbundle,
-    std::unique_ptr<const JSBigString> startupCode,
-    std::string sourceURL);
   void setGlobalVariable(std::string propName, std::unique_ptr<const JSBigString> jsonValue);
   void* getJavaScriptContext();
   bool supportsProfiling();