Skip to content

Commit

Permalink
Clean up RuntimeExecutor experiment and remove jsContext param
Browse files Browse the repository at this point in the history
Summary:
Cleaning up the test for switching to the shared RuntimeExecutor, and removing the jsContext arg from Fabric's Android APIs entirely.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22026752

fbshipit-source-id: df70faa70eaa2a04717ae89e8ad3216dfd486a35
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Jul 24, 2020
1 parent ab56ed7 commit eddf90f
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

package com.facebook.react.testing;

import com.facebook.react.bridge.JavaScriptContextHolder;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.UIManager;
import com.facebook.react.uimanager.ViewManagerRegistry;
Expand All @@ -16,7 +15,5 @@
public interface FabricUIManagerFactory {

UIManager getFabricUIManager(
ReactApplicationContext reactApplicationContext,
ViewManagerRegistry viewManagerRegistry,
JavaScriptContextHolder jsContext);
ReactApplicationContext reactApplicationContext, ViewManagerRegistry viewManagerRegistry);
}
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ public UIManager get() {
FabricUIManagerFactory factory = spec.getFabricUIManagerFactory();
return factory != null
? factory.getFabricUIManager(
reactApplicationContext, viewManagerRegistry, jsContext)
reactApplicationContext, viewManagerRegistry)
: null;
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import androidx.annotation.NonNull;
import com.facebook.jni.HybridData;
import com.facebook.proguard.annotations.DoNotStrip;
import com.facebook.react.bridge.JavaScriptContextHolder;
import com.facebook.react.bridge.NativeMap;
import com.facebook.react.bridge.RuntimeExecutor;
import com.facebook.react.bridge.queue.MessageQueueThread;
Expand All @@ -35,7 +34,6 @@ public Binding() {
}

private native void installFabricUIManager(
long jsContextNativePointer,
RuntimeExecutor runtimeExecutor,
Object uiManager,
EventBeatManager eventBeatManager,
Expand Down Expand Up @@ -76,7 +74,6 @@ public native void setConstraints(

// TODO (T67721598) Remove the jsContext param once we've migrated to using RuntimeExecutor
public void register(
@NonNull JavaScriptContextHolder jsContext,
@NonNull RuntimeExecutor runtimeExecutor,
@NonNull FabricUIManager fabricUIManager,
@NonNull EventBeatManager eventBeatManager,
Expand All @@ -85,7 +82,6 @@ public void register(
@NonNull ReactNativeConfig reactNativeConfig) {
fabricUIManager.setBinding(this);
installFabricUIManager(
jsContext.get(),
runtimeExecutor,
fabricUIManager,
eventBeatManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import androidx.annotation.NonNull;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSIModuleProvider;
import com.facebook.react.bridge.JavaScriptContextHolder;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.UIManager;
import com.facebook.react.bridge.queue.MessageQueueThread;
Expand Down Expand Up @@ -42,18 +41,15 @@

public class FabricJSIModuleProvider implements JSIModuleProvider<UIManager> {

@NonNull private final JavaScriptContextHolder mJSContext;
@NonNull private final ReactApplicationContext mReactApplicationContext;
@NonNull private final ComponentFactoryDelegate mComponentFactoryDelegate;
@NonNull private final ReactNativeConfig mConfig;

public FabricJSIModuleProvider(
@NonNull ReactApplicationContext reactApplicationContext,
@NonNull JavaScriptContextHolder jsContext,
@NonNull ComponentFactoryDelegate componentFactoryDelegate,
@NonNull ReactNativeConfig config) {
mReactApplicationContext = reactApplicationContext;
mJSContext = jsContext;
mComponentFactoryDelegate = componentFactoryDelegate;
mConfig = config;
}
Expand All @@ -74,7 +70,6 @@ public UIManager get() {
.getJSQueueThread();

binding.register(
mJSContext,
mReactApplicationContext.getCatalystInstance().getRuntimeExecutor(),
uiManager,
eventBeatManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ void Binding::setConstraints(
}

void Binding::installFabricUIManager(
jlong jsContextNativePointer,
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutorHolder,
jni::alias_ref<jobject> javaUIManager,
EventBeatManager *eventBeatManager,
Expand Down Expand Up @@ -240,24 +239,7 @@ void Binding::installFabricUIManager(

auto sharedJSMessageQueueThread =
std::make_shared<JMessageQueueThread>(jsMessageQueueThread);

bool useRuntimeExecutor =
config->getBool("react_fabric:use_shared_runtime_executor_android");

RuntimeExecutor runtimeExecutor;
if (useRuntimeExecutor) {
runtimeExecutor = runtimeExecutorHolder->cthis()->get();
} else {
Runtime *runtime = (Runtime *)jsContextNativePointer;
runtimeExecutor =
[runtime, sharedJSMessageQueueThread](
std::function<void(facebook::jsi::Runtime & runtime)> &&callback) {
sharedJSMessageQueueThread->runOnQueue(
[runtime, callback = std::move(callback)]() {
callback(*runtime);
});
};
}
auto runtimeExecutor = runtimeExecutorHolder->cthis()->get();

// TODO: T31905686 Create synchronous Event Beat
jni::global_ref<jobject> localJavaUIManager = javaUIManager_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ class Binding : public jni::HybridClass<Binding>,
static jni::local_ref<jhybriddata> initHybrid(jni::alias_ref<jclass>);

void installFabricUIManager(
jlong jsContextNativePointer,
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutorHolder,
jni::alias_ref<jobject> javaUIManager,
EventBeatManager *eventBeatManager,
Expand Down

0 comments on commit eddf90f

Please sign in to comment.