Adopt split segments registration approach on Android

Differential Revision: D6284863 fbshipit-source-id: 0df6b90eb0cbeab4c8a2b11f1e4dcbd5d5dfab72
facebook · Nov 9, 2017 · a47431e · a47431e
1 parent 6812789
commit a47431e
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 37 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java
@@ -86,6 +86,9 @@ void callFunction(
    */
   void removeBridgeIdleDebugListener(NotThreadSafeBridgeIdleDebugListener listener);
 
+  /** This method registers the file path of an additional JS segment by its ID. */
+  void registerSegment(int segmentId, String path);
+
   @VisibleForTesting
   void setGlobalVariable(String propName, String jsonValue);
 

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java
@@ -210,8 +210,9 @@ private native void initializeBridge(
     jniSetSourceURL(remoteURL);
   }
 
-  /* package */ void setJsSegmentsDirectory(String directoryPath) {
-    jniSetJsSegmentsDirectory(directoryPath);
+  @Override
+  public void registerSegment(int segmentId, String path) {
+    jniRegisterSegment(segmentId, path);
   }
 
   /* package */ void loadScriptFromAssets(AssetManager assetManager, String assetURL, boolean loadSynchronously) {
@@ -225,7 +226,7 @@ private native void initializeBridge(
   }
 
   private native void jniSetSourceURL(String sourceURL);
-  private native void jniSetJsSegmentsDirectory(String directoryPath);
+  private native void jniRegisterSegment(int segmentId, String path);
   private native void jniLoadScriptFromAssets(AssetManager assetManager, String assetURL, boolean loadSynchronously);
   private native void jniLoadScriptFromFile(String fileName, String sourceURL, boolean loadSynchronously);
 

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSBundleLoader.java
@@ -96,21 +96,6 @@ public String loadScript(CatalystInstanceImpl instance) {
     };
   }
 
-  /**
-   * This loader is used to wrap other loaders and set js segments directory before executing
-   * application script.
-   */
-  public static JSBundleLoader createSplitBundlesLoader(
-      final String jsSegmentsDirectory, final JSBundleLoader delegate) {
-    return new JSBundleLoader() {
-      @Override
-      public String loadScript(CatalystInstanceImpl instance) {
-        instance.setJsSegmentsDirectory(jsSegmentsDirectory);
-        return delegate.loadScript(instance);
-      }
-    };
-  }
-
   /** Loads the script, returning the URL of the source it loaded. */
   public abstract String loadScript(CatalystInstanceImpl instance);
 }
diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp
@@ -100,7 +100,7 @@ void CatalystInstanceImpl::registerNatives() {
     makeNativeMethod("initializeBridge", CatalystInstanceImpl::initializeBridge),
     makeNativeMethod("jniExtendNativeModules", CatalystInstanceImpl::extendNativeModules),
     makeNativeMethod("jniSetSourceURL", CatalystInstanceImpl::jniSetSourceURL),
-    makeNativeMethod("jniSetJsSegmentsDirectory", CatalystInstanceImpl::jniSetJsSegmentsDirectory),
+    makeNativeMethod("jniRegisterSegment", CatalystInstanceImpl::jniRegisterSegment),
     makeNativeMethod("jniLoadScriptFromAssets", CatalystInstanceImpl::jniLoadScriptFromAssets),
     makeNativeMethod("jniLoadScriptFromFile", CatalystInstanceImpl::jniLoadScriptFromFile),
     makeNativeMethod("jniCallJSFunction", CatalystInstanceImpl::jniCallJSFunction),
@@ -177,8 +177,8 @@ void CatalystInstanceImpl::jniSetSourceURL(const std::string& sourceURL) {
   instance_->setSourceURL(sourceURL);
 }
 
-void CatalystInstanceImpl::jniSetJsSegmentsDirectory(const std::string& directoryPath) {
-  jsSegmentsDirectory_ = directoryPath;
+void CatalystInstanceImpl::jniRegisterSegment(int segmentId, const std::string& path) {
+  instance_->registerBundle((uint32_t)segmentId, path);
 }
 
 void CatalystInstanceImpl::jniLoadScriptFromAssets(
@@ -208,16 +208,7 @@ void CatalystInstanceImpl::jniLoadScriptFromFile(const std::string& fileName,
                                                  const std::string& sourceURL,
                                                  bool loadSynchronously) {
   if (Instance::isIndexedRAMBundle(fileName.c_str())) {
-    auto bundle = folly::make_unique<JSIndexedRAMBundle>(fileName.c_str());
-    auto script = bundle->getStartupCode();
-    auto registry = jsSegmentsDirectory_.empty()
-      ? RAMBundleRegistry::singleBundleRegistry(std::move(bundle))
-      : RAMBundleRegistry::multipleBundlesRegistry(std::move(bundle), JSIndexedRAMBundle::buildFactory());
-    instance_->loadRAMBundle(
-      std::move(registry),
-      std::move(script),
-      sourceURL,
-      loadSynchronously);
+    instance_->loadRAMBundleFromFile(fileName, sourceURL, loadSynchronously);
   } else {
     std::unique_ptr<const JSBigFileString> script;
     RecoverableError::runRethrowingAsRecoverable<std::system_error>(

diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h
@@ -60,10 +60,10 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
   void jniSetSourceURL(const std::string& sourceURL);
 
   /**
-   * Sets the path to folder where additional bundles are located.
-   * Needs to be invoked before "loadScript" methods are called.
+   * Registers the file path of an additional JS segment by its ID.
+   *
    */
-  void jniSetJsSegmentsDirectory(const std::string& directoryPath);
+  void jniRegisterSegment(int segmentId, const std::string& path);
 
   void jniLoadScriptFromAssets(jni::alias_ref<JAssetManager::javaobject> assetManager, const std::string& assetURL, bool loadSynchronously);
   void jniLoadScriptFromFile(const std::string& fileName, const std::string& sourceURL, bool loadSynchronously);
@@ -74,8 +74,6 @@ class CatalystInstanceImpl : public jni::HybridClass<CatalystInstanceImpl> {
   jlong getJavaScriptContext();
   void handleMemoryPressure(int pressureLevel);
 
-  std::string jsSegmentsDirectory_;
-
   // This should be the only long-lived strong reference, but every C++ class
   // will have a weak reference.
   std::shared_ptr<Instance> instance_;

diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp
@@ -111,7 +111,7 @@ void Instance::loadRAMBundleFromFile(const std::string& sourcePath,
                            bool loadSynchronously) {
     auto bundle = folly::make_unique<JSIndexedRAMBundle>(sourcePath.c_str());
     auto startupScript = bundle->getStartupCode();
-    auto registry = RAMBundleRegistry::singleBundleRegistry(std::move(bundle));
+    auto registry = RAMBundleRegistry::multipleBundlesRegistry(std::move(bundle), JSIndexedRAMBundle::buildFactory());
     loadRAMBundle(
       std::move(registry),
       std::move(startupScript),