diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java index 0baef21f2..780cdb123 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java @@ -108,8 +108,8 @@ import com.facebook.systrace.Systrace; new ReactInstanceDevCommandsHandler() { @Override - public void onReloadWithJSDebugger(JavaJSExecutor jsExecutor) { - ReactInstanceManagerImpl.this.onReloadWithJSDebugger(jsExecutor); + public void onReloadWithJSDebugger(JavaJSExecutor.Factory jsExecutorFactory) { + ReactInstanceManagerImpl.this.onReloadWithJSDebugger(jsExecutorFactory); } @Override @@ -132,18 +132,18 @@ import com.facebook.systrace.Systrace; }; private class ReactContextInitParams { - private final JavaScriptExecutor mJsExecutor; + private final JavaScriptExecutor.Factory mJsExecutorFactory; private final JSBundleLoader mJsBundleLoader; public ReactContextInitParams( - JavaScriptExecutor jsExecutor, + JavaScriptExecutor.Factory jsExecutorFactory, JSBundleLoader jsBundleLoader) { - mJsExecutor = Assertions.assertNotNull(jsExecutor); + mJsExecutorFactory = Assertions.assertNotNull(jsExecutorFactory); mJsBundleLoader = Assertions.assertNotNull(jsBundleLoader); } - public JavaScriptExecutor getJsExecutor() { - return mJsExecutor; + public JavaScriptExecutor.Factory getJsExecutorFactory() { + return mJsExecutorFactory; } public JSBundleLoader getJsBundleLoader() { @@ -156,8 +156,7 @@ import com.facebook.systrace.Systrace; * be executing one at time, see {@link #recreateReactContextInBackground()}. */ private final class ReactContextInitAsyncTask extends - AsyncTask { - + AsyncTask> { @Override protected void onPreExecute() { if (mCurrentReactContext != null) { @@ -167,15 +166,23 @@ import com.facebook.systrace.Systrace; } @Override - protected ReactApplicationContext doInBackground(ReactContextInitParams... params) { + protected Result doInBackground(ReactContextInitParams... params) { Assertions.assertCondition(params != null && params.length > 0 && params[0] != null); - return createReactContext(params[0].getJsExecutor(), params[0].getJsBundleLoader()); + try { + JavaScriptExecutor jsExecutor = params[0].getJsExecutorFactory().create(); + return Result.of(createReactContext(jsExecutor, params[0].getJsBundleLoader())); + } catch (Exception e) { + // Pass exception to onPostExecute() so it can be handled on the main thread + return Result.of(e); + } } @Override - protected void onPostExecute(ReactApplicationContext reactContext) { + protected void onPostExecute(Result result) { try { - setupReactContext(reactContext); + setupReactContext(result.get()); + } catch (Exception e) { + mDevSupportManager.handleException(e); } finally { mIsContextInitAsyncTaskRunning = false; } @@ -183,13 +190,46 @@ import com.facebook.systrace.Systrace; // Handle enqueued request to re-initialize react context. if (mPendingReactContextInitParams != null) { recreateReactContextInBackground( - mPendingReactContextInitParams.getJsExecutor(), + mPendingReactContextInitParams.getJsExecutorFactory(), mPendingReactContextInitParams.getJsBundleLoader()); mPendingReactContextInitParams = null; } } } + private static class Result { + @Nullable private final T mResult; + @Nullable private final Exception mException; + + public static Result of(U result) { + return new Result(result); + } + + public static Result of(Exception exception) { + return new Result<>(exception); + } + + private Result(T result) { + mException = null; + mResult = result; + } + + private Result(Exception exception) { + mException = exception; + mResult = null; + } + + public T get() throws Exception { + if (mException != null) { + throw mException; + } + + Assertions.assertNotNull(mResult); + + return mResult; + } + } + /* package */ ReactInstanceManagerImpl( Context applicationContext, @Nullable String jsBundleFile, @@ -311,7 +351,7 @@ import com.facebook.systrace.Systrace; private void recreateReactContextInBackgroundFromBundleFile() { recreateReactContextInBackground( - new JSCJavaScriptExecutor(), + new JSCJavaScriptExecutor.Factory(), JSBundleLoader.createFileLoader(mApplicationContext, mJSBundleFile)); } @@ -504,9 +544,9 @@ import com.facebook.systrace.Systrace; return mCurrentReactContext; } - private void onReloadWithJSDebugger(JavaJSExecutor jsExecutor) { + private void onReloadWithJSDebugger(JavaJSExecutor.Factory jsExecutorFactory) { recreateReactContextInBackground( - new ProxyJavaScriptExecutor(jsExecutor), + new ProxyJavaScriptExecutor.Factory(jsExecutorFactory), JSBundleLoader.createRemoteDebuggerBundleLoader( mDevSupportManager.getJSBundleURLForRemoteDebugging(), mDevSupportManager.getSourceUrl())); @@ -514,18 +554,19 @@ import com.facebook.systrace.Systrace; private void onJSBundleLoadedFromServer() { recreateReactContextInBackground( - new JSCJavaScriptExecutor(), + new JSCJavaScriptExecutor.Factory(), JSBundleLoader.createCachedBundleFromNetworkLoader( mDevSupportManager.getSourceUrl(), mDevSupportManager.getDownloadedJSBundleFile())); } private void recreateReactContextInBackground( - JavaScriptExecutor jsExecutor, + JavaScriptExecutor.Factory jsExecutorFactory, JSBundleLoader jsBundleLoader) { UiThreadUtil.assertOnUiThread(); - ReactContextInitParams initParams = new ReactContextInitParams(jsExecutor, jsBundleLoader); + ReactContextInitParams initParams = + new ReactContextInitParams(jsExecutorFactory, jsBundleLoader); if (!mIsContextInitAsyncTaskRunning) { // No background task to create react context is currently running, create and execute one. ReactContextInitAsyncTask initTask = new ReactContextInitAsyncTask(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java index d371cf5be..32b261930 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JSCJavaScriptExecutor.java @@ -14,6 +14,12 @@ import com.facebook.soloader.SoLoader; @DoNotStrip public class JSCJavaScriptExecutor extends JavaScriptExecutor { + public static class Factory implements JavaScriptExecutor.Factory { + @Override + public JavaScriptExecutor create() throws Exception { + return new JSCJavaScriptExecutor(); + } + } static { SoLoader.loadLibrary(ReactBridge.REACT_NATIVE_LIB); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaJSExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaJSExecutor.java index 3c8eb4574..5c1b01b22 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaJSExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaJSExecutor.java @@ -18,7 +18,11 @@ import com.facebook.proguard.annotations.DoNotStrip; */ @DoNotStrip public interface JavaJSExecutor { - public static class ProxyExecutorException extends Exception { + interface Factory { + JavaJSExecutor create() throws Exception; + } + + class ProxyExecutorException extends Exception { public ProxyExecutorException(Throwable cause) { super(cause); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptExecutor.java index 2bc5e26c5..bdf7e4fc6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaScriptExecutor.java @@ -14,6 +14,9 @@ import com.facebook.proguard.annotations.DoNotStrip; @DoNotStrip public abstract class JavaScriptExecutor extends Countable { + public interface Factory { + JavaScriptExecutor create() throws Exception; + } /** * Close this executor and cleanup any resources that it was using. No further calls are diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java index f9e58869b..80cd6f4b3 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java @@ -24,6 +24,18 @@ import com.facebook.proguard.annotations.DoNotStrip; */ @DoNotStrip public class ProxyJavaScriptExecutor extends JavaScriptExecutor { + public static class Factory implements JavaScriptExecutor.Factory { + private final JavaJSExecutor.Factory mJavaJSExecutorFactory; + + public Factory(JavaJSExecutor.Factory javaJSExecutorFactory) { + mJavaJSExecutorFactory = javaJSExecutorFactory; + } + + @Override + public JavaScriptExecutor create() throws Exception { + return new ProxyJavaScriptExecutor(mJavaJSExecutorFactory.create()); + } + } static { SoLoader.loadLibrary(ReactBridge.REACT_NATIVE_LIB); diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/WebsocketJavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/WebsocketJavaScriptExecutor.java index 0fba2b0a3..cddadba00 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/WebsocketJavaScriptExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/WebsocketJavaScriptExecutor.java @@ -13,10 +13,10 @@ import javax.annotation.Nullable; import java.util.HashMap; import java.util.concurrent.Semaphore; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import android.os.Handler; +import android.os.Looper; import com.facebook.infer.annotation.Assertions; @@ -97,9 +97,13 @@ public class WebsocketJavaScriptExecutor implements JavaJSExecutor { String webSocketServerUrl, final JSExecutorConnectCallback callback) { final JSDebuggerWebSocketClient client = new JSDebuggerWebSocketClient(); - final Handler timeoutHandler = new Handler(); + final Handler timeoutHandler = new Handler(Looper.getMainLooper()); client.connect( webSocketServerUrl, new JSDebuggerWebSocketClient.JSDebuggerCallback() { + // It's possible that both callbacks can fire on an error so make sure we only + // dispatch results once to our callback. + private boolean didSendResult = false; + @Override public void onSuccess(@Nullable String response) { client.prepareJSRuntime( @@ -108,20 +112,30 @@ public class WebsocketJavaScriptExecutor implements JavaJSExecutor { public void onSuccess(@Nullable String response) { timeoutHandler.removeCallbacksAndMessages(null); mWebSocketClient = client; - callback.onSuccess(); + if (!didSendResult) { + callback.onSuccess(); + didSendResult = true; + } } @Override public void onFailure(Throwable cause) { timeoutHandler.removeCallbacksAndMessages(null); - callback.onFailure(cause); + if (!didSendResult) { + callback.onFailure(cause); + didSendResult = true; + } } }); } @Override public void onFailure(Throwable cause) { - callback.onFailure(cause); + timeoutHandler.removeCallbacksAndMessages(null); + if (!didSendResult) { + callback.onFailure(cause); + didSendResult = true; + } } }); timeoutHandler.postDelayed( diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java index bcc094062..ce60e861c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java @@ -15,6 +15,9 @@ import java.io.File; import java.io.IOException; import java.util.LinkedHashMap; import java.util.Locale; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import android.app.AlertDialog; import android.app.ProgressDialog; @@ -35,14 +38,15 @@ import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.react.R; import com.facebook.react.bridge.CatalystInstance; +import com.facebook.react.bridge.JavaJSExecutor; import com.facebook.react.bridge.NativeModuleCallExceptionHandler; -import com.facebook.react.bridge.ProxyJavaScriptExecutor; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.bridge.WebsocketJavaScriptExecutor; import com.facebook.react.common.ReactConstants; import com.facebook.react.common.ShakeDetector; +import com.facebook.react.common.futures.SimpleSettableFuture; import com.facebook.react.devsupport.StackTraceHelper.StackFrame; import com.facebook.react.modules.debug.DeveloperSettings; @@ -534,38 +538,47 @@ public class DevSupportManager implements NativeModuleCallExceptionHandler { // anyway mDevServerHelper.launchChromeDevtools(); - final WebsocketJavaScriptExecutor webSocketJSExecutor = new WebsocketJavaScriptExecutor(); - webSocketJSExecutor.connect( - mDevServerHelper.getWebsocketProxyURL(), - new WebsocketJavaScriptExecutor.JSExecutorConnectCallback() { - @Override - public void onSuccess() { - progressDialog.dismiss(); - UiThreadUtil.runOnUiThread( - new Runnable() { - @Override - public void run() { - mReactInstanceCommandsHandler.onReloadWithJSDebugger( - webSocketJSExecutor); - } - }); - } + JavaJSExecutor.Factory factory = new JavaJSExecutor.Factory() { + @Override + public JavaJSExecutor create() throws Exception { + WebsocketJavaScriptExecutor executor = new WebsocketJavaScriptExecutor(); + SimpleSettableFuture future = new SimpleSettableFuture<>(); + executor.connect( + mDevServerHelper.getWebsocketProxyURL(), + getExecutorConnectCallback(progressDialog, future)); + // TODO(t9349129) Don't use timeout + try { + future.get(90, TimeUnit.SECONDS); + return executor; + } catch (ExecutionException e) { + throw (Exception) e.getCause(); + } catch (InterruptedException | TimeoutException e) { + throw new RuntimeException(e); + } + } + }; + mReactInstanceCommandsHandler.onReloadWithJSDebugger(factory); + } - @Override - public void onFailure(final Throwable cause) { - progressDialog.dismiss(); - FLog.e(ReactConstants.TAG, "Unable to connect to remote debugger", cause); - UiThreadUtil.runOnUiThread( - new Runnable() { - @Override - public void run() { - showNewJavaError( - mApplicationContext.getString(R.string.catalyst_remotedbg_error), - cause); - } - }); - } - }); + private WebsocketJavaScriptExecutor.JSExecutorConnectCallback getExecutorConnectCallback( + final ProgressDialog progressDialog, + final SimpleSettableFuture future) { + return new WebsocketJavaScriptExecutor.JSExecutorConnectCallback() { + @Override + public void onSuccess() { + future.set(true); + progressDialog.dismiss(); + } + + @Override + public void onFailure(final Throwable cause) { + progressDialog.dismiss(); + FLog.e(ReactConstants.TAG, "Unable to connect to remote debugger", cause); + future.setException( + new IOException( + mApplicationContext.getString(R.string.catalyst_remotedbg_error), cause)); + } + }; } private void reloadJSFromServer(final ProgressDialog progressDialog) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReactInstanceDevCommandsHandler.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReactInstanceDevCommandsHandler.java index e97b16f7d..ce3639be8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReactInstanceDevCommandsHandler.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReactInstanceDevCommandsHandler.java @@ -20,7 +20,7 @@ public interface ReactInstanceDevCommandsHandler { /** * Request react instance recreation with JS debugging enabled. */ - void onReloadWithJSDebugger(JavaJSExecutor proxyExecutor); + void onReloadWithJSDebugger(JavaJSExecutor.Factory proxyExecutorFactory); /** * Notify react instance manager about new JS bundle version downloaded from the server.