Delay module creation on call for constants when module has none

Reviewed By: AaaChiuuu

Differential Revision: D4810252

fbshipit-source-id: b2b98c3a8355dbb5775f254f25304a21f0bfee5b

ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java

@@ -74,6 +74,7 @@ public class NativeModuleRegistryBuilder {
             reactModuleInfo.canOverrideExistingModule(),
             reactModuleInfo.supportsWebWorkers(),
             reactModuleInfo.needsEagerInit(),
+            reactModuleInfo.hasConstants(),
             moduleSpec.getProvider());
         }
 

ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java

@@ -71,7 +71,7 @@ import com.facebook.react.uimanager.UIManagerModule;
  * isolates us from the problems that may be caused by concurrent updates of animated graph while UI
  * thread is "executing" the animation loop.
  */
-@ReactModule(name = NativeAnimatedModule.NAME)
+@ReactModule(name = NativeAnimatedModule.NAME, hasConstants = false)
 public class NativeAnimatedModule extends ReactContextBaseJavaModule implements
     OnBatchCompleteListener, LifecycleEventListener {
 

ReactAndroid/src/main/java/com/facebook/react/bridge/BaseJavaModule.java

@@ -70,4 +70,8 @@ public abstract class BaseJavaModule implements NativeModule {
   public boolean supportsWebWorkers() {
     return false;
   }
+
+  public boolean hasConstants() {
+    return false;
+  }
 }

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/JavaMethodWrapper.java

@@ -233,6 +233,9 @@ public class JavaMethodWrapper implements NativeModule.NativeMethod {
     if (mArgumentsProcessed) {
       return;
     }
+    SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "processArguments")
+      .arg("method", mModuleWrapper.getName() + "." + mMethod.getName())
+      .flush();
     mArgumentsProcessed = true;
     mArgumentExtractors = buildArgumentExtractors(mParameterTypes);
     mSignature = buildSignature(mMethod, mParameterTypes, (mType.equals(BaseJavaModule.METHOD_TYPE_SYNC)));
@@ -240,6 +243,7 @@ public class JavaMethodWrapper implements NativeModule.NativeMethod {
     // safe to allocate only one arguments object per method that can be reused across calls
     mArguments = new Object[mParameterTypes.length];
     mJSArgumentsNeeded = calculateJSArgumentsNeeded();
+    com.facebook.systrace.Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
   }
 
   public Method getMethod() {

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/JavaModuleWrapper.java

@@ -9,6 +9,8 @@
 
 package com.facebook.react.cxxbridge;
 
+import javax.annotation.Nullable;
+
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -123,7 +125,10 @@ public class JavaModuleWrapper {
   // TODO mhorowitz: make this return NativeMap, which requires moving
   // NativeMap out of OnLoad.
   @DoNotStrip
-  public NativeArray getConstants() {
+  public @Nullable NativeArray getConstants() {
+    if (!mModuleHolder.getHasConstants()) {
+      return null;
+    }
     BaseJavaModule baseJavaModule = getModule();
     ReactMarker.logMarker(GET_CONSTANTS_START, getName());
     SystraceMessage.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "Map constants")

ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java

@@ -31,6 +31,7 @@ public class ModuleHolder {
   private final String mName;
   private final boolean mCanOverrideExistingModule;
   private final boolean mSupportsWebWorkers;
+  private final boolean mHasConstants;
 
   private @Nullable Provider<? extends NativeModule> mProvider;
   private @Nullable NativeModule mModule;
@@ -41,10 +42,12 @@ public class ModuleHolder {
     boolean canOverrideExistingModule,
     boolean supportsWebWorkers,
     boolean needsEagerInit,
+    boolean hasConstants,
     Provider<? extends NativeModule> provider) {
     mName = name;
     mCanOverrideExistingModule = canOverrideExistingModule;
     mSupportsWebWorkers = supportsWebWorkers;
+    mHasConstants = hasConstants;
     mProvider = provider;
     if (needsEagerInit) {
       mModule = create();
@@ -55,6 +58,7 @@ public class ModuleHolder {
     mName = nativeModule.getName();
     mCanOverrideExistingModule = nativeModule.canOverrideExistingModule();
     mSupportsWebWorkers = nativeModule.supportsWebWorkers();
+    mHasConstants = true;
     mModule = nativeModule;
   }
 
@@ -89,6 +93,10 @@ public class ModuleHolder {
     return mSupportsWebWorkers;
   }
 
+  public boolean getHasConstants() {
+    return mHasConstants;
+  }
+
   @DoNotStrip
   public synchronized NativeModule getModule() {
     if (mModule == null) {

ReactAndroid/src/main/java/com/facebook/react/devsupport/JSCSamplingProfiler.java

@@ -23,7 +23,7 @@ import com.facebook.react.module.annotations.ReactModule;
 
 // This module is being called only by Java via the static method "poke" that
 // requires it to alreay be initialized, thus we eagerly initialize this module
-@ReactModule(name = "JSCSamplingProfiler", needsEagerInit = true)
+@ReactModule(name = "JSCSamplingProfiler", needsEagerInit = true, hasConstants = false)
 public class JSCSamplingProfiler extends ReactContextBaseJavaModule {
   public interface SamplingProfiler extends JavaScriptModule {
     void poke(int token);

ReactAndroid/src/main/java/com/facebook/react/module/annotations/ReactModule.java

@@ -65,4 +65,10 @@ public @interface ReactModule {
    * Whether this module needs to be loaded immediately.
    */
   boolean needsEagerInit() default false;
+
+  /**
+   *  Whether this module has constants to add, defaults to true as that is safer for when a
+   *  correct annotation is not included
+   */
+  boolean hasConstants() default true;
 }

ReactAndroid/src/main/java/com/facebook/react/module/model/BUCK

@@ -8,5 +8,6 @@ android_library(
     ],
     deps = [
         react_native_dep("third-party/java/jsr-305:jsr-305"),
+        react_native_target("java/com/facebook/react/bridge:bridge"),
     ],
 )

ReactAndroid/src/main/java/com/facebook/react/module/model/ReactModuleInfo.java

@@ -12,16 +12,19 @@ public class ReactModuleInfo {
   private final boolean mCanOverrideExistingModule;
   private final boolean mSupportsWebWorkers;
   private final boolean mNeedsEagerInit;
+  private final boolean mHasConstants;
 
   public ReactModuleInfo(
     String name,
     boolean canOverrideExistingModule,
     boolean supportsWebWorkers,
-    boolean needsEagerInit) {
+    boolean needsEagerInit,
+    boolean hasConstants) {
     mName = name;
     mCanOverrideExistingModule = canOverrideExistingModule;
     mSupportsWebWorkers = supportsWebWorkers;
     mNeedsEagerInit = needsEagerInit;
+    mHasConstants = hasConstants;
   }
 
   public String name() {
@@ -39,4 +42,8 @@ public class ReactModuleInfo {
   public boolean needsEagerInit() {
     return mNeedsEagerInit;
   }
+
+  public boolean hasConstants() {
+    return mHasConstants;
+  }
 }

ReactAndroid/src/main/java/com/facebook/react/module/processing/BUCK

@@ -21,6 +21,7 @@ java_library(
         react_native_dep("third-party/java/javapoet:javapoet"),
         react_native_dep("third-party/java/jsr-305:jsr-305"),
         react_native_target("java/com/facebook/react/module/annotations:annotations"),
+        react_native_target("java/com/facebook/react/bridge:bridge"),
         react_native_target("java/com/facebook/react/module/model:model"),
     ],
 )

ReactAndroid/src/main/java/com/facebook/react/module/processing/ReactModuleSpecProcessor.java

@@ -160,7 +160,8 @@ public class ReactModuleSpecProcessor extends AbstractProcessor {
           .append("\"").append(reactModule.name()).append("\"").append(", ")
           .append(reactModule.canOverrideExistingModule()).append(", ")
           .append(reactModule.supportsWebWorkers()).append(", ")
-          .append(reactModule.needsEagerInit())
+          .append(reactModule.needsEagerInit()).append(", ")
+          .append(reactModule.hasConstants())
           .append(")")
           .toString();
 

ReactAndroid/src/main/java/com/facebook/react/modules/appstate/AppStateModule.java

@@ -19,7 +19,7 @@ import com.facebook.react.bridge.WritableMap;
 import com.facebook.react.module.annotations.ReactModule;
 import com.facebook.react.modules.core.DeviceEventManagerModule.RCTDeviceEventEmitter;
 
-@ReactModule(name = AppStateModule.NAME)
+@ReactModule(name = AppStateModule.NAME, hasConstants = false)
 public class AppStateModule extends ReactContextBaseJavaModule
         implements LifecycleEventListener {
 

ReactAndroid/src/main/java/com/facebook/react/modules/core/DeviceEventManagerModule.java

@@ -26,7 +26,7 @@ import com.facebook.react.module.annotations.ReactModule;
 /**
  * Native module that handles device hardware events like hardware back presses.
  */
-@ReactModule(name = "DeviceEventManager")
+@ReactModule(name = "DeviceEventManager", hasConstants = false)
 public class DeviceEventManagerModule extends ReactContextBaseJavaModule {
 
   @SupportsWebWorkers

ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java

@@ -23,7 +23,7 @@ import com.facebook.react.common.JavascriptException;
 import com.facebook.react.common.ReactConstants;
 import com.facebook.react.module.annotations.ReactModule;
 
-@ReactModule(name = ExceptionsManagerModule.NAME)
+@ReactModule(name = ExceptionsManagerModule.NAME, hasConstants = false)
 public class ExceptionsManagerModule extends BaseJavaModule {
 
   protected static final String NAME = "ExceptionsManager";

ReactAndroid/src/main/java/com/facebook/react/modules/core/HeadlessJsTaskSupportModule.java

@@ -20,7 +20,7 @@ import com.facebook.react.module.annotations.ReactModule;
  * Simple native module that allows JS to notify native of having completed some task work, so that
  * it can e.g. release any resources, stop timers etc.
  */
-@ReactModule(name = HeadlessJsTaskSupportModule.MODULE_NAME)
+@ReactModule(name = HeadlessJsTaskSupportModule.MODULE_NAME, hasConstants = false)
 public class HeadlessJsTaskSupportModule extends ReactContextBaseJavaModule {
 
   protected static final String MODULE_NAME = "HeadlessJsTaskSupport";

ReactAndroid/src/main/java/com/facebook/react/modules/core/Timing.java

@@ -42,7 +42,7 @@ import com.facebook.react.module.annotations.ReactModule;
 /**
  * Native module for JS timer execution. Timers fire on frame boundaries.
  */
-@ReactModule(name = Timing.NAME, supportsWebWorkers = true)
+@ReactModule(name = Timing.NAME, supportsWebWorkers = true, hasConstants = false)
 public final class Timing extends ReactContextBaseJavaModule implements LifecycleEventListener,
   OnExecutorUnregisteredListener, HeadlessJsTaskEventListener {
 

ReactAndroid/src/main/java/com/facebook/react/modules/debug/AnimationsDebugModule.java

@@ -29,7 +29,7 @@ import com.facebook.react.modules.debug.interfaces.DeveloperSettings;
  * Module that records debug information during transitions (animated navigation events such as
  * going from one screen to another).
  */
-@ReactModule(name = AnimationsDebugModule.NAME)
+@ReactModule(name = AnimationsDebugModule.NAME, hasConstants = false)
 public class AnimationsDebugModule extends ReactContextBaseJavaModule {
 
   protected static final String NAME = "AnimationsDebugModule";

ReactAndroid/src/main/java/com/facebook/react/modules/storage/AsyncStorageModule.java

@@ -33,7 +33,7 @@ import static com.facebook.react.modules.storage.ReactDatabaseSupplier.KEY_COLUM
 import static com.facebook.react.modules.storage.ReactDatabaseSupplier.TABLE_CATALYST;
 import static com.facebook.react.modules.storage.ReactDatabaseSupplier.VALUE_COLUMN;
 
-@ReactModule(name = AsyncStorageModule.NAME)
+@ReactModule(name = AsyncStorageModule.NAME, hasConstants = false)
 public final class AsyncStorageModule
     extends ReactContextBaseJavaModule implements ModuleDataCleaner.Cleanable {
 

ReactAndroid/src/main/java/com/facebook/react/modules/websocket/WebSocketModule.java

@@ -50,7 +50,7 @@ import java.util.concurrent.TimeUnit;
 import okio.Buffer;
 import okio.ByteString;
 
-@ReactModule(name = "WebSocketModule")
+@ReactModule(name = "WebSocketModule", hasConstants = false)
 public class WebSocketModule extends ReactContextBaseJavaModule {
 
   private final Map<Integer, WebSocket> mWebSocketConnections = new HashMap<>();
@@ -296,7 +296,6 @@ public class WebSocketModule extends ReactContextBaseJavaModule {
       }
 
       return defaultOrigin;
-
     } catch (URISyntaxException e) {
       throw new IllegalArgumentException("Unable to set " + uri + " as default origin header");
     }

ReactAndroid/src/main/java/com/facebook/react/processing/BUCK

@@ -20,7 +20,7 @@ java_library(
         react_native_dep("third-party/java/infer-annotations:infer-annotations"),
         react_native_dep("third-party/java/javapoet:javapoet"),
         react_native_dep("third-party/java/jsr-305:jsr-305"),
-        react_native_target("java/com/facebook/react/bridge:bridge"),
         react_native_target("java/com/facebook/react/uimanager/annotations:annotations"),
+        react_native_target("java/com/facebook/react/bridge:bridge"),
     ],
 )

ReactAndroid/src/main/java/com/facebook/react/uimanager/debug/DebugComponentOwnershipModule.java

@@ -27,7 +27,7 @@ import com.facebook.react.module.annotations.ReactModule;
  *
  * Example returned owner hierarchy: ['RootView', 'Dialog', 'TitleView', 'Text']
  */
-@ReactModule(name = "DebugComponentOwnershipModule")
+@ReactModule(name = "DebugComponentOwnershipModule", hasConstants = false)
 public class DebugComponentOwnershipModule extends ReactContextBaseJavaModule {
 
   public interface RCTDebugComponentOwnership extends JavaScriptModule {

ReactAndroid/src/test/java/com/facebook/react/cxxbridge/BUCK

@@ -9,10 +9,13 @@ rn_robolectric_test(
         "PUBLIC",
     ],
     deps = [
+        react_native_dep("libraries/fbcore/src/test/java/com/facebook/powermock:powermock"),
+        react_native_dep("libraries/soloader/java/com/facebook/soloader:soloader"),
         react_native_dep("third-party/java/fest:fest"),
         react_native_dep("third-party/java/junit:junit"),
         react_native_dep("third-party/java/mockito:mockito"),
         react_native_dep("third-party/java/robolectric3/robolectric:robolectric"),
+        react_native_target("java/com/facebook/react/bridge:bridge"),
         react_native_target("java/com/facebook/react/cxxbridge:bridge"),
         react_native_tests_target("java/com/facebook/common/logging:logging"),
     ],

ReactAndroid/src/test/java/com/facebook/react/cxxbridge/BaseJavaModuleTest.java

@@ -7,11 +7,16 @@
  * of patent rights can be found in the PATENTS file in the same directory.
  */
 
-package com.facebook.react.bridge;
+package com.facebook.react.cxxbridge;
 
-import java.util.Map;
+import javax.inject.Provider;
+import java.util.List;
 
+import com.facebook.react.bridge.BaseJavaModule;
+import com.facebook.react.bridge.ReactMethod;
 import com.facebook.react.bridge.ReadableNativeArray;
+import com.facebook.react.bridge.Promise;
+import com.facebook.react.bridge.NativeArgumentsParseException;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -27,7 +32,7 @@ import org.robolectric.RobolectricTestRunner;
 import com.facebook.soloader.SoLoader;
 
 /**
- * Tests for {@link BaseJavaModule}
+ * Tests for {@link BaseJavaModule} and {@link JavaModuleWrapper}
  */
 @PrepareForTest({ReadableNativeArray.class, SoLoader.class})
 @PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"})
@@ -37,43 +42,70 @@ public class BaseJavaModuleTest {
   @Rule
   public PowerMockRule rule = new PowerMockRule();
 
-  private Map<String, NativeModule.NativeMethod> mMethods;
+  private List<JavaModuleWrapper.MethodDescriptor> mMethods;
+  private JavaModuleWrapper mWrapper;
   private ReadableNativeArray mArguments;
 
   @Before
   public void setup() {
-    mMethods = new MethodsModule().getMethods();
+    ModuleHolder moduleHolder = new ModuleHolder("MethodsModule",
+      false,
+      false,
+      false,
+      false,
+      new Provider<MethodsModule>() {
+        MethodsModule mModule;
+        @Override
+        public MethodsModule get() {
+          mModule = new MethodsModule();
+          return mModule;
+        }
+      });
+    mWrapper = new JavaModuleWrapper(null, MethodsModule.class, moduleHolder);
+    mMethods = mWrapper.getMethodDescriptors();
     PowerMockito.mockStatic(SoLoader.class);
     mArguments = PowerMockito.mock(ReadableNativeArray.class);
   }
 
+  private int findMethod(String mname, List<JavaModuleWrapper.MethodDescriptor> methods) {
+    int posn = -1;
+    for (int i = 0; i< methods.size(); i++) {
+      JavaModuleWrapper.MethodDescriptor md = methods.get(i);
+      if (md.name == mname) {
+        posn = i;
+        break;
+      }
+    }
+    return posn;
+  }
+
   @Test(expected = NativeArgumentsParseException.class)
   public void testCallMethodWithoutEnoughArgs() throws Exception {
-    BaseJavaModule.NativeMethod regularMethod = mMethods.get("regularMethod");
+    int methodId = findMethod("regularMethod",mMethods);
     Mockito.stub(mArguments.size()).toReturn(1);
-    regularMethod.invoke(null, null, mArguments);
+    mWrapper.invoke(null, methodId, mArguments);
   }
 
   @Test
   public void testCallMethodWithEnoughArgs() {
-    BaseJavaModule.NativeMethod regularMethod = mMethods.get("regularMethod");
+    int methodId = findMethod("regularMethod", mMethods);
     Mockito.stub(mArguments.size()).toReturn(2);
-    regularMethod.invoke(null, null, mArguments);
+    mWrapper.invoke(null, methodId, mArguments);
   }
 
   @Test
   public void testCallAsyncMethodWithEnoughArgs() {
     // Promise block evaluates to 2 args needing to be passed from JS
-    BaseJavaModule.NativeMethod asyncMethod = mMethods.get("asyncMethod");
+    int methodId = findMethod("asyncMethod", mMethods);
     Mockito.stub(mArguments.size()).toReturn(3);
-    asyncMethod.invoke(null, null, mArguments);
+    mWrapper.invoke(null, methodId, mArguments);
   }
 
   @Test
   public void testCallSyncMethod() {
-    BaseJavaModule.NativeMethod syncMethod = mMethods.get("syncMethod");
+    int methodId = findMethod("syncMethod", mMethods);
     Mockito.stub(mArguments.size()).toReturn(2);
-    syncMethod.invoke(null, null, mArguments);
+    mWrapper.invoke(null, methodId, mArguments);
   }
 
   private static class MethodsModule extends BaseJavaModule {