Let JS modules use strings instead of ids on Android

Reviewed By: nicklockwood

Differential Revision: D3229626

fb-gh-sync-id: f8b2e8c9fc0d25d67f623cdbbe9930a02a40d1de
fbshipit-source-id: f8b2e8c9fc0d25d67f623cdbbe9930a02a40d1de
This commit is contained in:
Alexander Blom
2016-05-04 10:28:59 -07:00
committed by Facebook Github Bot 4
parent 5071907b27
commit 041185edfd
11 changed files with 45 additions and 82 deletions

View File

@@ -53,8 +53,6 @@ class MessageQueue {
this._callableModules = {};
this._queue = [[], [], [], 0];
this._moduleTable = {};
this._methodTable = {};
this._callbacks = [];
this._callbackID = 0;
this._callID = 0;
@@ -69,9 +67,6 @@ class MessageQueue {
let modulesConfig = this._genModulesConfig(remoteModules);
this._genModules(modulesConfig);
localModules && this._genLookupTables(
this._genModulesConfig(localModules),this._moduleTable, this._methodTable
);
this._debugInfo = {};
this._remoteModuleTable = {};
@@ -165,13 +160,9 @@ class MessageQueue {
}
}
__callFunction(module, method, args) {
__callFunction(module: string, method: string, args: any) {
this._lastFlush = new Date().getTime();
this._eventLoopStartTime = this._lastFlush;
if (isFinite(module)) {
method = this._methodTable[module][method];
module = this._moduleTable[module];
}
Systrace.beginEvent(`${module}.${method}()`);
if (__DEV__ && SPY_MODE) {
console.log('N->JS : ' + module + '.' + method + '(' + JSON.stringify(args) + ')');

View File

@@ -48,12 +48,6 @@ describe('MessageQueue', () => {
assertQueue(flushedQueue, 0, 0, 1, [2]);
});
it('should call a local function with id', () => {
expect(TestModule.testHook1.calls.count()).toEqual(0);
queue.__callFunction(0, 0, [1]);
expect(TestModule.testHook1.calls.count()).toEqual(1);
});
it('should call a local function with the function name', () => {
expect(TestModule.testHook2.calls.count()).toEqual(0);
queue.__callFunction('one', 'testHook2', [2]);

View File

@@ -30,8 +30,8 @@ public interface CatalystInstance extends MemoryPressureListener {
@DoNotStrip
void callFunction(
ExecutorToken executorToken,
int moduleId,
int methodId,
String module,
String method,
NativeArray arguments,
String tracingName);
/**

View File

@@ -172,8 +172,8 @@ public class CatalystInstanceImpl implements CatalystInstance {
@Override
public void callFunction(
ExecutorToken executorToken,
int moduleId,
int methodId,
String module,
String method,
NativeArray arguments,
String tracingName) {
synchronized (mJavaToJSCallsTeardownLock) {
@@ -184,7 +184,9 @@ public class CatalystInstanceImpl implements CatalystInstance {
incrementPendingJSCalls();
Assertions.assertNotNull(mBridge).callFunction(executorToken, moduleId, methodId, arguments, tracingName);
Assertions.assertNotNull(mBridge).callFunction(executorToken,
module,
method, arguments, tracingName);
}
}

View File

@@ -13,12 +13,13 @@ import javax.annotation.concurrent.Immutable;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import com.facebook.react.common.MapBuilder;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.common.build.ReactBuildConfig;
/**
* Registration info for a {@link JavaScriptModule}. Maps its methods to method ids.
@@ -26,54 +27,32 @@ import com.facebook.infer.annotation.Assertions;
@Immutable
public class JavaScriptModuleRegistration {
private final int mModuleId;
private final Class<? extends JavaScriptModule> mModuleInterface;
private final Map<Method, Integer> mMethodsToIds;
private final Map<Method, String> mMethodsToTracingNames;
public JavaScriptModuleRegistration(int moduleId, Class<? extends JavaScriptModule> moduleInterface) {
mModuleId = moduleId;
public JavaScriptModuleRegistration(Class<? extends JavaScriptModule> moduleInterface) {
mModuleInterface = moduleInterface;
mMethodsToTracingNames = new HashMap<>();
mMethodsToIds = MapBuilder.newHashMap();
mMethodsToTracingNames = MapBuilder.newHashMap();
final Method[] declaredMethods = mModuleInterface.getDeclaredMethods();
Arrays.sort(declaredMethods, new Comparator<Method>() {
@Override
public int compare(Method lhs, Method rhs) {
return lhs.getName().compareTo(rhs.getName());
if (ReactBuildConfig.DEBUG) {
Set<String> methodNames = new LinkedHashSet<>();
for (Method method : mModuleInterface.getDeclaredMethods()) {
if (!methodNames.add(method.getName())) {
throw new AssertionError(
"Method overloading is unsupported: " + mModuleInterface.getName() + "#" +
method.getName());
}
}
});
// Methods are sorted by name so we can dupe check and have obvious ordering
String previousName = null;
for (int i = 0; i < declaredMethods.length; i++) {
Method method = declaredMethods[i];
String name = method.getName();
Assertions.assertCondition(
!name.equals(previousName),
"Method overloading is unsupported: " + mModuleInterface.getName() + "#" + name);
previousName = name;
mMethodsToIds.put(method, i);
mMethodsToTracingNames.put(method, "JSCall__" + getName() + "_" + method.getName());
}
}
public int getModuleId() {
return mModuleId;
}
public int getMethodId(Method method) {
final Integer id = mMethodsToIds.get(method);
if (id == null) {
Assertions.assertUnreachable("Unknown method: " + method.getName());
}
return id.intValue();
}
public String getTracingName(Method method) {
return Assertions.assertNotNull(mMethodsToTracingNames.get(method));
String name = mMethodsToTracingNames.get(method);
if (name == null) {
name = "JSCall__" + getName() + "_" + method.getName();
mMethodsToTracingNames.put(method, name);
}
return name;
}
public Class<? extends JavaScriptModule> getModuleInterface() {
@@ -92,7 +71,7 @@ public class JavaScriptModuleRegistration {
return name;
}
public Set<Method> getMethods() {
return mMethodsToIds.keySet();
public List<Method> getMethods() {
return Arrays.asList(mModuleInterface.getDeclaredMethods());
}
}

View File

@@ -96,8 +96,8 @@ public class JavaScriptModuleRegistry {
NativeArray jsArgs = args != null ? Arguments.fromJavaArgs(args) : new WritableNativeArray();
mCatalystInstance.callFunction(
executorToken,
mModuleRegistration.getModuleId(),
mModuleRegistration.getMethodId(method),
mModuleRegistration.getName(),
method.getName(),
jsArgs,
tracingName);
return null;

View File

@@ -42,11 +42,11 @@ public class JavaScriptModulesConfig {
private void appendJSModuleToJSONObject(
JsonWriter writer,
JavaScriptModuleRegistration registration) throws IOException {
writer.name("moduleID").value(registration.getModuleId());
writer.name("moduleID").value(registration.getName());
writer.name("methods").beginObject();
for (Method method : registration.getMethods()) {
writer.name(method.getName()).beginObject();
writer.name("methodID").value(registration.getMethodId(method));
writer.name("methodID").value(method.getName());
writer.endObject();
}
writer.endObject();
@@ -56,14 +56,11 @@ public class JavaScriptModulesConfig {
}
public static class Builder {
private int mLastJSModuleId = 0;
private List<JavaScriptModuleRegistration> mModules =
new ArrayList<JavaScriptModuleRegistration>();
public Builder add(Class<? extends JavaScriptModule> moduleInterfaceClass) {
int moduleId = mLastJSModuleId++;
mModules.add(new JavaScriptModuleRegistration(moduleId, moduleInterfaceClass));
mModules.add(new JavaScriptModuleRegistration(moduleInterfaceClass));
return this;
}

View File

@@ -79,7 +79,7 @@ public class ReactBridge extends Countable {
*/
public native void loadScriptFromAssets(AssetManager assetManager, String assetName);
public native void loadScriptFromFile(@Nullable String fileName, @Nullable String sourceURL);
public native void callFunction(ExecutorToken executorToken, int moduleId, int methodId, NativeArray arguments, String tracingName);
public native void callFunction(ExecutorToken executorToken, String module, String method, NativeArray arguments, String tracingName);
public native void invokeCallback(ExecutorToken executorToken, int callbackID, NativeArray arguments);
public native void setGlobalVariable(String propertyName, String jsonEncodedArgument);
public native boolean supportsProfiling();

View File

@@ -66,8 +66,8 @@ public:
*/
void callFunction(
ExecutorToken executorToken,
const std::string& moduleId,
const std::string& methodId,
const std::string& module,
const std::string& method,
const folly::dynamic& args,
const std::string& tracingName);

View File

@@ -776,15 +776,15 @@ static void loadScriptFromFile(JNIEnv* env, jobject obj, jstring fileName, jstri
env->CallStaticVoidMethod(markerClass, gLogMarkerMethod, env->NewStringUTF("loadScriptFromFile_exec"));
}
static void callFunction(JNIEnv* env, jobject obj, JExecutorToken::jhybridobject jExecutorToken, jint moduleId, jint methodId,
static void callFunction(JNIEnv* env, jobject obj, JExecutorToken::jhybridobject jExecutorToken, jstring module, jstring method,
NativeArray::jhybridobject args, jstring tracingName) {
auto bridge = extractRefPtr<CountableBridge>(env, obj);
auto arguments = cthis(wrap_alias(args));
try {
bridge->callFunction(
cthis(wrap_alias(jExecutorToken))->getExecutorToken(wrap_alias(jExecutorToken)),
folly::to<std::string>(moduleId),
folly::to<std::string>(methodId),
fromJString(env, module),
fromJString(env, method),
std::move(arguments->array),
fromJString(env, tracingName)
);

View File

@@ -49,11 +49,11 @@ public class JavaScriptModuleConfigTest {
JsonNode intMethodNode = methods.get("intMethod");
assertThat(intMethodNode).isNotNull();
assertThat(intMethodNode.get("methodID").asInt()).isEqualTo(0);
assertThat(intMethodNode.get("methodID").asText()).isEqualTo("intMethod");
JsonNode stringMethod = methods.get("stringMethod");
assertThat(stringMethod).isNotNull();
assertThat(stringMethod.get("methodID").asInt()).isEqualTo(1);
assertThat(stringMethod.get("methodID").asText()).isEqualTo("stringMethod");
}
@Test
@@ -69,11 +69,11 @@ public class JavaScriptModuleConfigTest {
JsonNode someModuleNode = node.get("SomeModule");
assertThat(someModuleNode).isNotNull();
int someModuleID = someModuleNode.get("moduleID").asInt();
String someModuleID = someModuleNode.get("moduleID").asText();
JsonNode otherModuleNode = node.get("OtherModule");
assertThat(otherModuleNode).isNotNull();
int otherModuleID = otherModuleNode.get("moduleID").asInt();
String otherModuleID = otherModuleNode.get("moduleID").asText();
assertThat(otherModuleID)
.isNotEqualTo(someModuleID);
}