BREAKING - (Re)moving `JSBundleLoader.getSourceUrl()`

Summary:
== What ==

Changing the `JSBundleLoader` API, to remove `String getSourceUrl()`, instead `JSBundleLoader.loadScript` now returns the source URL it loaded.

This change has a knock-on effect: We can no longer populate `SourceCodeModule` when we construct it, because at that time, we do not know the source URL.

In order to solve this I have made the following changes:

 -  Added `CatalystInstance.getSourceURL()`, to return the source URL from the instance after the JS Bundle has been loaded, or `null` otherwise.
 -  Removed `ReactInstanceManager.getSourceUrl()`, because its only purpose was to populate `SourceCodeModule`.
 -  Also removed `ReactInstanceManager.getJSBundleFile()` because it was only being used in a test confirming that the `ReactInstanceManager` knew its bundle file as soon as it was constructed, which is no longer necessarily true.
 -  Initialise `SourceCodeModule` with the `ReactContext` instance it belongs to.
 -  Override `NativeModule.initialize()` in `SourceCodeModule` to fetch the source URL. When the `SourceCodeModule` is constructed, the context does not have a properly initialised `CatalystInstance`, but by the time we call initialise on it, the `ReactContext` has a `CatalystInstance` and that in turn has a source URL.

== Why ==

The reason for this change is that it allows us to add implementations of `JSBundleLoader`, that cannot determine their source URL until after having performed a load successfully. In particular I plan to introduce `FallbackJSBundleLoader` which will try to load from multiple sources in sequence stopping after the first successful load. As load failures could happen for a variety of reasons, we can't know what the true source URL is without performing the load.

Reviewed By: javache

Differential Revision: D4398956

fbshipit-source-id: 51ff4e289c8723e9d242f23267181c775a6abe6f

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

@@ -128,7 +128,7 @@ import static com.facebook.react.bridge.ReactMarkerConstants.CREATE_UI_MANAGER_M
       new ModuleSpec(SourceCodeModule.class, new Provider<NativeModule>() {
         @Override
         public NativeModule get() {
-          return new SourceCodeModule(mReactInstanceManager.getSourceUrl());
+          return new SourceCodeModule(reactContext);
         }
       }));
     moduleSpecList.add(

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

@@ -154,16 +154,6 @@ public abstract class ReactInstanceManager {
     Intent data);
   public abstract void showDevOptionsDialog();
 
-  /**
-   * Get the URL where the last bundle was loaded from.
-   */
-  public abstract String getSourceUrl();
-
-  /**
-   * The JS Bundle file that this Instance Manager was constructed with.
-   */
-  public abstract @Nullable String getJSBundleFile();
-
   /**
    * Attach given {@param rootView} to a catalyst instance manager and start JS application using
    * JS module provided by {@link ReactRootView#getJSModuleName}. If the react context is currently

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

@@ -127,7 +127,6 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;
   private @Nullable volatile ReactContext mCurrentReactContext;
   private final Context mApplicationContext;
   private @Nullable DefaultHardwareBackBtnHandler mDefaultBackButtonImpl;
-  private String mSourceUrl;
   private @Nullable Activity mCurrentActivity;
   private final Collection<ReactInstanceEventListener> mReactInstanceEventListeners =
       Collections.synchronizedSet(new HashSet<ReactInstanceEventListener>());
@@ -634,22 +633,6 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;
     mDevSupportManager.showDevOptionsDialog();
   }
 
-  /**
-   * Get the URL where the last bundle was loaded from.
-   */
-  @Override
-  public String getSourceUrl() {
-    return Assertions.assertNotNull(mSourceUrl);
-  }
-
-  @Override
-  public @Nullable String getJSBundleFile() {
-    if (mBundleLoader == null) {
-      return null;
-    }
-    return mBundleLoader.getSourceUrl();
-  }
-
   /**
    * Attach given {@param rootView} to a catalyst instance manager and start JS application using
    * JS module provided by {@link ReactRootView#getJSModuleName}. If the react context is currently
@@ -843,7 +826,6 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;
       JSBundleLoader jsBundleLoader) {
     FLog.i(ReactConstants.TAG, "Creating react context.");
     ReactMarker.logMarker(CREATE_REACT_CONTEXT_START);
-    mSourceUrl = jsBundleLoader.getSourceUrl();
     List<ModuleSpec> moduleSpecs = new ArrayList<>();
     Map<Class, ReactModuleInfo> reactModuleInfoMap = new HashMap<>();
     JavaScriptModuleRegistry.Builder jsModulesBuilder = new JavaScriptModuleRegistry.Builder();

ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java

@@ -9,6 +9,8 @@
 
 package com.facebook.react.bridge;
 
+import javax.annotation.Nullable;
+
 import java.util.Collection;
 
 import com.facebook.proguard.annotations.DoNotStrip;
@@ -23,6 +25,13 @@ import com.facebook.react.common.annotations.VisibleForTesting;
 @DoNotStrip
 public interface CatalystInstance extends MemoryPressureListener {
   void runJSBundle();
+
+  /**
+   * Return the source URL of the JS Bundle that was run, or {@code null} if no JS
+   * bundle has been run yet.
+   */
+  @Nullable String getSourceURL();
+
   // This is called from java code, so it won't be stripped anyway, but proguard will rename it,
   // which this prevents.
   @DoNotStrip

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java

@@ -97,6 +97,7 @@ public class CatalystInstanceImpl implements CatalystInstance {
   private volatile boolean mAcceptCalls = false;
 
   private boolean mJSBundleHasLoaded;
+  private @Nullable String mSourceURL;
 
   // C++ parts
   private final HybridData mHybridData;
@@ -188,8 +189,9 @@ public class CatalystInstanceImpl implements CatalystInstance {
   public void runJSBundle() {
     Assertions.assertCondition(!mJSBundleHasLoaded, "JS bundle was already loaded!");
     mJSBundleHasLoaded = true;
+
     // incrementPendingJSCalls();
-    mJSBundleLoader.loadScript(CatalystInstanceImpl.this);
+    mSourceURL = mJSBundleLoader.loadScript(CatalystInstanceImpl.this);
 
     synchronized (mJSCallsPendingInitLock) {
       // Loading the bundle is queued on the JS thread, but may not have
@@ -208,6 +210,11 @@ public class CatalystInstanceImpl implements CatalystInstance {
     Systrace.registerListener(mTraceListener);
   }
 
+  @Override
+  public @Nullable String getSourceURL() {
+    return mSourceURL;
+  }
+
   private native void callJSFunction(
     ExecutorToken token,
     String module,

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/JSBundleLoader.java

@@ -29,12 +29,8 @@ public abstract class JSBundleLoader {
       final String assetUrl) {
     return new JSBundleLoader() {
       @Override
-      public void loadScript(CatalystInstanceImpl instance) {
+      public String loadScript(CatalystInstanceImpl instance) {
         instance.loadScriptFromAssets(context.getAssets(), assetUrl);
-      }
-
-      @Override
-      public String getSourceUrl() {
         return assetUrl;
       }
     };
@@ -47,12 +43,8 @@ public abstract class JSBundleLoader {
   public static JSBundleLoader createFileLoader(final String fileName) {
     return new JSBundleLoader() {
       @Override
-      public void loadScript(CatalystInstanceImpl instance) {
+      public String loadScript(CatalystInstanceImpl instance) {
         instance.loadScriptFromFile(fileName, fileName);
-      }
-
-      @Override
-      public String getSourceUrl() {
         return fileName;
       }
     };
@@ -70,18 +62,14 @@ public abstract class JSBundleLoader {
       final String cachedFileLocation) {
     return new JSBundleLoader() {
       @Override
-      public void loadScript(CatalystInstanceImpl instance) {
+      public String loadScript(CatalystInstanceImpl instance) {
         try {
           instance.loadScriptFromFile(cachedFileLocation, sourceURL);
+          return sourceURL;
         } catch (Exception e) {
           throw DebugServerException.makeGeneric(e.getMessage(), e);
         }
       }
-
-      @Override
-      public String getSourceUrl() {
-        return sourceURL;
-      }
     };
   }
 
@@ -94,17 +82,15 @@ public abstract class JSBundleLoader {
       final String realSourceURL) {
     return new JSBundleLoader() {
       @Override
-      public void loadScript(CatalystInstanceImpl instance) {
+      public String loadScript(CatalystInstanceImpl instance) {
         instance.loadScriptFromFile(null, proxySourceURL);
-      }
-
-      @Override
-      public String getSourceUrl() {
         return realSourceURL;
       }
     };
   }
 
-  public abstract void loadScript(CatalystInstanceImpl instance);
-  public abstract String getSourceUrl();
+  /**
+   * Loads the script, returning the URL of the source it loaded.
+   */
+  public abstract String loadScript(CatalystInstanceImpl instance);
 }

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/OptimizedJSBundleLoader.java

@@ -24,12 +24,8 @@ public class OptimizedJSBundleLoader extends JSBundleLoader {
   }
 
   @Override
-  public void loadScript(CatalystInstanceImpl instance) {
+  public String loadScript(CatalystInstanceImpl instance) {
     instance.loadScriptFromOptimizedBundle(mPath, mSourceURL, mLoadFlags);
-  }
-
-  @Override
-  public String getSourceUrl() {
     return mSourceURL;
   }
 }

ReactAndroid/src/main/java/com/facebook/react/modules/debug/SourceCodeModule.java

@@ -14,7 +14,9 @@ import javax.annotation.Nullable;
 import java.util.HashMap;
 import java.util.Map;
 
+import com.facebook.infer.annotation.Assertions;
 import com.facebook.react.bridge.BaseJavaModule;
+import com.facebook.react.bridge.ReactContext;
 import com.facebook.react.module.annotations.ReactModule;
 
 /**
@@ -23,10 +25,21 @@ import com.facebook.react.module.annotations.ReactModule;
 @ReactModule(name = "RCTSourceCode")
 public class SourceCodeModule extends BaseJavaModule {
 
-  private final String mSourceUrl;
+  private final ReactContext mReactContext;
+  private @Nullable String mSourceUrl;
 
-  public SourceCodeModule(String sourceUrl) {
-    mSourceUrl = sourceUrl;
+  public SourceCodeModule(ReactContext reactContext) {
+    mReactContext = reactContext;
+  }
+
+  @Override
+  public void initialize() {
+    super.initialize();
+
+    mSourceUrl =
+      Assertions.assertNotNull(
+        mReactContext.getCatalystInstance().getSourceURL(),
+        "No source URL loaded, have you initialised the instance?");
   }
 
   @Override