From 68aeffe01f442a67a53dd0eff43e9357b0ac3b22 Mon Sep 17 00:00:00 2001 From: Andy Street Date: Thu, 3 Nov 2016 09:33:10 -0700 Subject: [PATCH] Queue JS calls that come in before JS bundle has started loading instead of crashing Summary: This mimics (some of) the behavior we have on iOS where if you call a JS module method before the JS bundle has started loading, we just queue up those calls and execute them after the bundle has started loading. Reviewed By: javache Differential Revision: D4117581 fbshipit-source-id: 58c5a6f87aeeb86083385334d92f2716a0574ba1 --- .../react/bridge/CatalystInstance.java | 1 - .../facebook/react/bridge/ReactContext.java | 5 +- .../react/cxxbridge/CatalystInstanceImpl.java | 53 +++++++++++++++---- 3 files changed, 46 insertions(+), 13 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 index fbf8925cb..a6ab8fd65 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstance.java @@ -40,7 +40,6 @@ public interface CatalystInstance extends MemoryPressureListener { */ void destroy(); boolean isDestroyed(); - boolean isAcceptingCalls(); /** * Initialize all the native modules diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java index 35ec76084..c43a23ac9 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java @@ -40,7 +40,8 @@ public class ReactContext extends ContextWrapper { private static final String EARLY_JS_ACCESS_EXCEPTION_MESSAGE = "Tried to access a JS module before the React instance was fully set up. Calls to " + - "ReactContext#getJSModule should be protected by ReactContext#hasActiveCatalystInstance()."; + "ReactContext#getJSModule should only happen once initialize() has been called on your " + + "native module."; private final CopyOnWriteArraySet mLifecycleEventListeners = new CopyOnWriteArraySet<>(); @@ -143,7 +144,7 @@ public class ReactContext extends ContextWrapper { } public boolean hasActiveCatalystInstance() { - return mCatalystInstance != null && mCatalystInstance.isAcceptingCalls(); + return mCatalystInstance != null && !mCatalystInstance.isDestroyed(); } public LifecycleState getLifecycleState() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java index fad0ff908..7150ce21b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/CatalystInstanceImpl.java @@ -12,6 +12,7 @@ package com.facebook.react.cxxbridge; import javax.annotation.Nullable; import java.lang.ref.WeakReference; +import java.util.ArrayList; import java.util.Collection; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicInteger; @@ -57,6 +58,25 @@ public class CatalystInstanceImpl implements CatalystInstance { private static final AtomicInteger sNextInstanceIdForTrace = new AtomicInteger(1); + private static class PendingJSCall { + + public ExecutorToken mExecutorToken; + public String mModule; + public String mMethod; + public NativeArray mArguments; + + public PendingJSCall( + ExecutorToken executorToken, + String module, + String method, + NativeArray arguments) { + mExecutorToken = executorToken; + mModule = module; + mMethod = method; + mArguments = arguments; + } + } + // Access from any thread private final ReactQueueConfigurationImpl mReactQueueConfiguration; private final CopyOnWriteArrayList mBridgeIdleListeners; @@ -67,6 +87,8 @@ public class CatalystInstanceImpl implements CatalystInstance { private final TraceListener mTraceListener; private final JavaScriptModuleRegistry mJSModuleRegistry; private final JSBundleLoader mJSBundleLoader; + private final ArrayList mJSCallsPendingInit = new ArrayList(); + private final Object mJSCallsPendingInitLock = new Object(); private ExecutorToken mMainExecutorToken; private final NativeModuleRegistry mJavaRegistry; @@ -168,10 +190,20 @@ public class CatalystInstanceImpl implements CatalystInstance { 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; + + synchronized (mJSCallsPendingInitLock) { + // 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; + + for (PendingJSCall call : mJSCallsPendingInit) { + callJSFunction(call.mExecutorToken, call.mModule, call.mMethod, call.mArguments); + } + mJSCallsPendingInit.clear(); + } + + // This is registered after JS starts since it makes a JS call Systrace.registerListener(mTraceListener); } @@ -193,7 +225,13 @@ public class CatalystInstanceImpl implements CatalystInstance { return; } if (!mAcceptCalls) { - throw new RuntimeException("Attempt to call JS function before JS bundle is loaded."); + // Most of the time the instance is initialized and we don't need to acquire the lock + synchronized (mJSCallsPendingInitLock) { + if (!mAcceptCalls) { + mJSCallsPendingInit.add(new PendingJSCall(executorToken, module, method, arguments)); + return; + } + } } callJSFunction(executorToken, module, method, arguments); @@ -244,11 +282,6 @@ public class CatalystInstanceImpl implements CatalystInstance { return mDestroyed; } - @Override - public boolean isAcceptingCalls() { - return !mDestroyed && mAcceptCalls; - } - /** * Initialize all the native modules */