Skip to content

Commit

Permalink
Explicitly make UIManagerModule use OnBatchComplete on Android
Browse files Browse the repository at this point in the history
Summary:
Currently, we scan all native modules to see if they implement the OnBatchCompleteListerner. If they do, we add those modules to a list, and when C++ calls OnBactchComplete is called, we execute the callback on each of the modules.
The only native module implementing this callback today is the UIManagerModule. With Fabric, UIManager will also not be a native module anymore. This diff removes all the work done for creating the list and assumes that UIManagerModule is the only place that is interested in OnBatchComplete call - and calls it directly.

Reviewed By: achen1

Differential Revision: D9186651

fbshipit-source-id: 473586b37c2465ccd041985dcdd56132026f34f1
  • Loading branch information
axe-fb authored and kelset committed Aug 13, 2018
1 parent 095b8f0 commit 9db3c51
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,7 @@ private void putModuleTypeAndHolderToModuleMaps(
}

public NativeModuleRegistry build() {
ArrayList<ModuleHolder> batchCompleteListenerModules = new ArrayList<>();
for (Map.Entry<String, ModuleHolder> entry : mModules.entrySet()) {
if (entry.getValue().hasOnBatchCompleteListener()) {
batchCompleteListenerModules.add(entry.getValue());
}
}

return new NativeModuleRegistry(
mReactApplicationContext, mModules, batchCompleteListenerModules);
mReactApplicationContext, mModules);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ public class ModuleHolder {
private final boolean mCanOverrideExistingModule;
private final boolean mHasConstants;
private final boolean mIsCxxModule;
private final boolean mHasOnBatchCompleteListener;

private @Nullable Provider<? extends NativeModule> mProvider;
// Outside of the constructur, these should only be checked or set when synchronized on this
Expand All @@ -57,7 +56,6 @@ public ModuleHolder(ReactModuleInfo moduleInfo, Provider<? extends NativeModule>
mCanOverrideExistingModule = moduleInfo.canOverrideExistingModule();
mHasConstants = moduleInfo.hasConstants();
mProvider = provider;
mHasOnBatchCompleteListener = moduleInfo.hasOnBatchCompleteListener();
mIsCxxModule = moduleInfo.isCxxModule();
if (moduleInfo.needsEagerInit()) {
mModule = create();
Expand All @@ -69,7 +67,6 @@ public ModuleHolder(NativeModule nativeModule) {
mCanOverrideExistingModule = nativeModule.canOverrideExistingModule();
mHasConstants = true;
mIsCxxModule = CxxModuleWrapper.class.isAssignableFrom(nativeModule.getClass());
mHasOnBatchCompleteListener = OnBatchCompleteListener.class.isAssignableFrom(nativeModule.getClass());
mModule = nativeModule;
PrinterHolder.getPrinter()
.logMessage(ReactDebugOverlayTags.NATIVE_MODULE, "NativeModule init: %s", mName);
Expand Down Expand Up @@ -121,8 +118,6 @@ public boolean getHasConstants() {

public boolean isCxxModule() {return mIsCxxModule; }

public boolean hasOnBatchCompleteListener() {return mHasOnBatchCompleteListener; }

@DoNotStrip
public NativeModule getModule() {
NativeModule module;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@

package com.facebook.react.bridge;

import com.facebook.infer.annotation.Assertions;
import com.facebook.systrace.Systrace;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.HashMap;

import com.facebook.infer.annotation.Assertions;
import com.facebook.systrace.Systrace;

/**
* A set of Java APIs to expose to a particular JavaScript instance.
Expand All @@ -23,15 +21,12 @@ public class NativeModuleRegistry {

private final ReactApplicationContext mReactApplicationContext;
private final Map<String, ModuleHolder> mModules;
private final ArrayList<ModuleHolder> mBatchCompleteListenerModules;

public NativeModuleRegistry(
ReactApplicationContext reactApplicationContext,
Map<String, ModuleHolder> modules,
ArrayList<ModuleHolder> batchCompleteListenerModules) {
Map<String, ModuleHolder> modules) {
mReactApplicationContext = reactApplicationContext;
mModules = modules;
mBatchCompleteListenerModules = batchCompleteListenerModules;
}

/**
Expand All @@ -45,10 +40,6 @@ private ReactApplicationContext getReactApplicationContext() {
return mReactApplicationContext;
}

private ArrayList<ModuleHolder> getBatchCompleteListenerModules() {
return mBatchCompleteListenerModules;
}

/* package */ Collection<JavaModuleWrapper> getJavaModules(
JSInstance jsInstance) {
ArrayList<JavaModuleWrapper> javaModules = new ArrayList<>();
Expand Down Expand Up @@ -81,15 +72,11 @@ private ArrayList<ModuleHolder> getBatchCompleteListenerModules() {
"Extending native modules with non-matching application contexts.");

Map<String, ModuleHolder> newModules = newRegister.getModuleMap();
ArrayList<ModuleHolder> batchCompleteListeners = newRegister.getBatchCompleteListenerModules();

for (Map.Entry<String, ModuleHolder> entry : newModules.entrySet()) {
String key = entry.getKey();
if (!mModules.containsKey(key)) {
ModuleHolder value = entry.getValue();
if (batchCompleteListeners.contains(value)) {
mBatchCompleteListenerModules.add(value);
}
mModules.put(key, value);
}
}
Expand Down Expand Up @@ -129,10 +116,13 @@ private ArrayList<ModuleHolder> getBatchCompleteListenerModules() {
}

public void onBatchComplete() {
for (ModuleHolder moduleHolder : mBatchCompleteListenerModules) {
if (moduleHolder.hasInstance()) {
((OnBatchCompleteListener) moduleHolder.getModule()).onBatchComplete();
}
// The only native module that uses the onBatchComplete is the UI Manager. Hence, instead of
// iterating over all the modules for find this one instance, and then calling it, we short-circuit
// the search, and simply call OnBatchComplete on the UI Manager.
// With Fabric, UIManager would no longer be a NativeModule, so this call would simply go away
ModuleHolder moduleHolder = mModules.get("com.facebook.react.uimanager.UIManagerModule");
if (moduleHolder != null && moduleHolder.hasInstance()) {
((OnBatchCompleteListener) moduleHolder.getModule()).onBatchComplete();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,18 @@ public class ReactModuleInfo {
private final boolean mNeedsEagerInit;
private final boolean mHasConstants;
private final boolean mIsCxxModule;
private final boolean mHasOnBatchCompleteListener;

public ReactModuleInfo(
String name,
boolean canOverrideExistingModule,
boolean needsEagerInit,
boolean hasConstants,
boolean isCxxModule,
boolean hasOnBatchCompleteListener) {
boolean isCxxModule) {
mName = name;
mCanOverrideExistingModule = canOverrideExistingModule;
mNeedsEagerInit = needsEagerInit;
mHasConstants = hasConstants;
mIsCxxModule = isCxxModule;
mHasOnBatchCompleteListener = hasOnBatchCompleteListener;
}

public String name() {
Expand All @@ -51,7 +48,4 @@ public boolean hasConstants() {

public boolean isCxxModule() {return mIsCxxModule; }

public boolean hasOnBatchCompleteListener() {
return mHasOnBatchCompleteListener;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package com.facebook.react.module.processing;

import com.facebook.react.bridge.CxxModuleWrapper;
import com.facebook.react.bridge.OnBatchCompleteListener;
import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.Filer;
import javax.annotation.processing.Messager;
Expand Down Expand Up @@ -160,7 +159,6 @@ private CodeBlock getCodeBlockForReactModuleInfos(List<String> nativeModules)
builder.addStatement("$T map = new $T()", MAP_TYPE, INSTANTIATED_MAP_TYPE);

TypeMirror cxxModuleWrapperTypeMirror = mElements.getTypeElement(CxxModuleWrapper.class.getName()).asType();
TypeMirror onBatchCompleteListenerTypeMirror = mElements.getTypeElement(OnBatchCompleteListener.class.getName()).asType();

for (String nativeModule : nativeModules) {
String keyString = nativeModule;
Expand Down Expand Up @@ -192,26 +190,14 @@ private CodeBlock getCodeBlockForReactModuleInfos(List<String> nativeModules)
}

boolean isCxxModule = mTypes.isAssignable(typeElement.asType(), cxxModuleWrapperTypeMirror);
boolean hasOnBatchCompleteListener = false;
try {
hasOnBatchCompleteListener = mTypes.isAssignable(typeElement.asType(), onBatchCompleteListenerTypeMirror);
} catch (RuntimeException e) {
// This is SUPER ugly, but we need to do this, especially for AsyncStorageModule which implements ModuleDataCleaner
// In the case of that specific class, we get the exception
// com.sun.tools.javac.code.Symbol$CompletionFailure: class file for ModuleDataCleaner not found.
// The exception is caused because the class is not loaded the first time. However, catching it and
// running it again the second time loads the class and does what the following statement originally intended
hasOnBatchCompleteListener = mTypes.isAssignable(typeElement.asType(), onBatchCompleteListenerTypeMirror);
}

String valueString = new StringBuilder()
.append("new ReactModuleInfo(")
.append("\"").append(reactModule.name()).append("\"").append(", ")
.append(reactModule.canOverrideExistingModule()).append(", ")
.append(reactModule.needsEagerInit()).append(", ")
.append(hasConstants).append(", ")
.append(isCxxModule).append(", ")
.append(hasOnBatchCompleteListener)
.append(isCxxModule)
.append(")")
.toString();

Expand Down

0 comments on commit 9db3c51

Please sign in to comment.