From 53a7d5822b053af42bb1ad6033ae91d59f5763b6 Mon Sep 17 00:00:00 2001
From: Aaron Chiu <aaronechiu@fb.com>
Date: Thu, 9 Feb 2017 22:16:14 -0800
Subject: [PATCH] don't call OnBatchComplete on specific modules unless we need
 to

Summary:
Don't call OnBatchComplete on NativeModules that have'nt been initialized.
Also a follow up to D4479604. This also removes the extra allocation of OnBatchCompleteListener per NativeModule that implements OnBatchCompleteListener.

This means NativeModules which implement OnBatchCompleteListener will have to be explicitly initialized or called into before the OnBatchCompleteListener of that NativeModule will be triggered.

Reviewed By: javache

Differential Revision: D4483682

fbshipit-source-id: 6a9431c82f72d17605d1c1e0ee9194f8d8fc2ddc
---
 .../react/NativeModuleRegistryBuilder.java         | 14 +++-----------
 .../com/facebook/react/cxxbridge/ModuleHolder.java |  4 ++++
 .../react/cxxbridge/NativeModuleRegistry.java      | 11 ++++++-----
 3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java
index 4055ec671..2e2be320b 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/NativeModuleRegistryBuilder.java
@@ -113,18 +113,10 @@ public class NativeModuleRegistryBuilder {
   }
 
   public NativeModuleRegistry build() {
-    ArrayList<OnBatchCompleteListener> batchCompleteListenerModules = new ArrayList<>();
+    ArrayList<ModuleHolder> batchCompleteListenerModules = new ArrayList<>();
     for (Map.Entry<Class<? extends NativeModule>, ModuleHolder> entry : mModules.entrySet()) {
-      Class<? extends NativeModule> type = entry.getKey();
-      if (OnBatchCompleteListener.class.isAssignableFrom(type)) {
-        final ModuleHolder moduleHolder = entry.getValue();
-        batchCompleteListenerModules.add(new OnBatchCompleteListener() {
-          @Override
-          public void onBatchComplete() {
-            OnBatchCompleteListener listener = (OnBatchCompleteListener) moduleHolder.getModule();
-            listener.onBatchComplete();
-          }
-        });
+      if (OnBatchCompleteListener.class.isAssignableFrom(entry.getKey())) {
+        batchCompleteListenerModules.add(entry.getValue());
       }
     }
 
diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java
index 449fbcf3b..42ab92753 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/ModuleHolder.java
@@ -67,6 +67,10 @@ public class ModuleHolder {
     }
   }
 
+  public synchronized boolean isInitialized() {
+    return mModule != null;
+  }
+
   public synchronized void destroy() {
     if (mModule != null) {
       mModule.onCatalystInstanceDestroy();
diff --git a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java
index c8a02744c..f20d0d32c 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/cxxbridge/NativeModuleRegistry.java
@@ -15,7 +15,6 @@ import java.util.List;
 import java.util.Map;
 
 import com.facebook.infer.annotation.Assertions;
-import com.facebook.react.bridge.BaseJavaModule;
 import com.facebook.react.bridge.NativeModule;
 import com.facebook.react.bridge.OnBatchCompleteListener;
 import com.facebook.react.bridge.ReactMarker;
@@ -28,11 +27,11 @@ import com.facebook.systrace.Systrace;
 public class NativeModuleRegistry {
 
   private final Map<Class<? extends NativeModule>, ModuleHolder> mModules;
-  private final ArrayList<OnBatchCompleteListener> mBatchCompleteListenerModules;
+  private final ArrayList<ModuleHolder> mBatchCompleteListenerModules;
 
   public NativeModuleRegistry(
     Map<Class<? extends NativeModule>, ModuleHolder> modules,
-    ArrayList<OnBatchCompleteListener> batchCompleteListenerModules) {
+    ArrayList<ModuleHolder> batchCompleteListenerModules) {
     mModules = modules;
     mBatchCompleteListenerModules = batchCompleteListenerModules;
   }
@@ -92,8 +91,10 @@ public class NativeModuleRegistry {
   }
 
   public void onBatchComplete() {
-    for (int i = 0; i < mBatchCompleteListenerModules.size(); i++) {
-      mBatchCompleteListenerModules.get(i).onBatchComplete();
+    for (ModuleHolder moduleHolder : mBatchCompleteListenerModules) {
+      if (moduleHolder.isInitialized()) {
+        ((OnBatchCompleteListener) moduleHolder.getModule()).onBatchComplete();
+      }
     }
   }