Forward dynamic values where possibe

Summary: Changelog: [Internal] Reviewed By: rshest Differential Revision: D42961391 fbshipit-source-id: d07f8627a59d43e762bddb0641b3961806e9a7ec
facebook · Feb 21, 2023 · 753bccc · 753bccc
1 parent 3e88fd0
commit 753bccc
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 51 deletions.
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java
@@ -195,6 +195,7 @@ public int getInt(@NonNull String name) {
 
       final String[] iteratorKeys = mKeys;
       final Object[] iteratorValues = Assertions.assertNotNull(importValues());
+      mJniCallCounter++;
 
       return new Iterator<Map.Entry<String, Object>>() {
         int currentIndex = 0;
@@ -289,22 +290,4 @@ public boolean equals(Object obj) {
     }
     return hashMap;
   }
-
-  private static class ReadableNativeMapKeySetIterator implements ReadableMapKeySetIterator {
-    private final Iterator<String> mIterator;
-
-    public ReadableNativeMapKeySetIterator(ReadableNativeMap readableNativeMap) {
-      mIterator = readableNativeMap.getLocalMap().keySet().iterator();
-    }
-
-    @Override
-    public boolean hasNextKey() {
-      return mIterator.hasNext();
-    }
-
-    @Override
-    public String nextKey() {
-      return mIterator.next();
-    }
-  }
 }
diff --git a/ReactAndroid/src/main/jni/react/fabric/StateWrapperImpl.cpp b/ReactAndroid/src/main/jni/react/fabric/StateWrapperImpl.cpp
@@ -27,24 +27,20 @@ jni::local_ref<StateWrapperImpl::jhybriddata> StateWrapperImpl::initHybrid(
 jni::local_ref<ReadableNativeMap::jhybridobject>
 StateWrapperImpl::getStateDataImpl() {
   folly::dynamic map = state_->getDynamic();
-  local_ref<ReadableNativeMap::jhybridobject> readableNativeMap =
-      ReadableNativeMap::newObjectCxxArgs(map);
-  return readableNativeMap;
+  return ReadableNativeMap::newObjectCxxArgs(std::move(map));
 }
 
 jni::local_ref<JReadableMapBuffer::jhybridobject>
 StateWrapperImpl::getStateMapBufferDataImpl() {
   MapBuffer map = state_->getMapBuffer();
-  auto readableMapBuffer =
-      JReadableMapBuffer::createWithContents(std::move(map));
-  return readableMapBuffer;
+  return JReadableMapBuffer::createWithContents(std::move(map));
 }
 
 void StateWrapperImpl::updateStateImpl(NativeMap *map) {
   // Get folly::dynamic from map
   auto dynamicMap = map->consume();
   // Set state
-  state_->updateState(dynamicMap);
+  state_->updateState(std::move(dynamicMap));
 }
 
 void StateWrapperImpl::registerNatives() {

diff --git a/ReactAndroid/src/main/jni/react/jni/NativeArray.cpp b/ReactAndroid/src/main/jni/react/jni/NativeArray.cpp
@@ -7,16 +7,14 @@
 
 #include "NativeArray.h"
 
-#include <fbjni/fbjni.h>
 #include <folly/json.h>
 
 using namespace facebook::jni;
 
 namespace facebook {
 namespace react {
 
-NativeArray::NativeArray(folly::dynamic array)
-    : isConsumed(false), array_(std::move(array)) {
+void NativeArray::assertInternalType() {
   if (!array_.isArray()) {
     throwNewJavaException(
         exceptions::gUnexpectedNativeTypeExceptionClass,

diff --git a/ReactAndroid/src/main/jni/react/jni/NativeArray.h b/ReactAndroid/src/main/jni/react/jni/NativeArray.h
@@ -17,7 +17,7 @@ namespace react {
 
 class NativeArray : public jni::HybridClass<NativeArray> {
  public:
-  static constexpr const char *kJavaDescriptor =
+  static auto constexpr *kJavaDescriptor =
       "Lcom/facebook/react/bridge/NativeArray;";
 
   jni::local_ref<jstring> toString();
@@ -27,15 +27,25 @@ class NativeArray : public jni::HybridClass<NativeArray> {
   // Whether this array has been added to another array or map and no longer
   // has a valid array value.
   bool isConsumed;
-  void throwIfConsumed();
 
   static void registerNatives();
 
  protected:
   folly::dynamic array_;
 
   friend HybridBase;
-  explicit NativeArray(folly::dynamic array);
+
+  template <class Dyn>
+  explicit NativeArray(Dyn &&array)
+      : isConsumed(false), array_(std::forward<Dyn>(array)) {
+    assertInternalType();
+  }
+
+  void assertInternalType();
+  void throwIfConsumed();
+
+  NativeArray(const NativeArray &) = delete;
+  NativeArray &operator=(const NativeArray &) = delete;
 };
 
 } // namespace react

diff --git a/ReactAndroid/src/main/jni/react/jni/NativeMap.cpp b/ReactAndroid/src/main/jni/react/jni/NativeMap.cpp
@@ -14,9 +14,9 @@ using namespace facebook::jni;
 namespace facebook {
 namespace react {
 
-std::string NativeMap::toString() {
+local_ref<jstring> NativeMap::toString() {
   throwIfConsumed();
-  return ("{ NativeMap: " + folly::toJson(map_) + " }").c_str();
+  return make_jstring(folly::toJson(map_).c_str());
 }
 
 void NativeMap::registerNatives() {

diff --git a/ReactAndroid/src/main/jni/react/jni/NativeMap.h b/ReactAndroid/src/main/jni/react/jni/NativeMap.h
@@ -20,23 +20,29 @@ class NativeMap : public jni::HybridClass<NativeMap> {
   static auto constexpr kJavaDescriptor =
       "Lcom/facebook/react/bridge/NativeMap;";
 
-  std::string toString();
+  jni::local_ref<jstring> toString();
 
   RN_EXPORT folly::dynamic consume();
 
   // Whether this map has been added to another array or map and no longer
   // has a valid map value.
   bool isConsumed;
-  void throwIfConsumed();
 
   static void registerNatives();
 
  protected:
   folly::dynamic map_;
 
   friend HybridBase;
-  friend struct ReadableNativeMapKeySetIterator;
-  explicit NativeMap(folly::dynamic s) : isConsumed(false), map_(s) {}
+
+  template <class Dyn>
+  explicit NativeMap(Dyn &&map)
+      : isConsumed(false), map_(std::forward<Dyn>(map)) {}
+
+  void throwIfConsumed();
+
+  NativeMap(const NativeMap &) = delete;
+  NativeMap &operator=(const NativeMap &) = delete;
 };
 
 } // namespace react

diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.cpp b/ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.cpp
@@ -14,12 +14,6 @@ using namespace facebook::jni;
 namespace facebook {
 namespace react {
 
-// This attribute exports the ctor symbol, so ReadableNativeArray to be
-// constructed from other DSOs.
-__attribute__((visibility("default")))
-ReadableNativeArray::ReadableNativeArray(folly::dynamic array)
-    : HybridBase(std::move(array)) {}
-
 // TODO T112842309: Remove after fbjni upgraded in OSS
 void ReadableNativeArray::mapException(const std::exception &ex) {
   if (dynamic_cast<const folly::TypeError *>(&ex) != nullptr) {

diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.h b/ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.h
@@ -24,7 +24,10 @@ class ReadableNativeArray
     : public jni::HybridClass<ReadableNativeArray, NativeArray> {
  protected:
   friend HybridBase;
-  explicit ReadableNativeArray(folly::dynamic array);
+
+  template <class Dyn>
+  explicit ReadableNativeArray(Dyn &&array)
+      : HybridBase(std::forward<Dyn>(array)) {}
 
  public:
   static constexpr const char *kJavaDescriptor =

diff --git a/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp b/ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp
@@ -69,38 +69,42 @@ void addDynamicToJArray(
 }
 
 local_ref<JArrayClass<jstring>> ReadableNativeMap::importKeys() {
+  throwIfConsumed();
+
   keys_ = folly::dynamic::array();
   if (map_ == nullptr) {
     return JArrayClass<jstring>::newArray(0);
   }
-  auto pairs = map_.items();
-  jint size = map_.size();
-  auto jarray = JArrayClass<jstring>::newArray(size);
+  auto jarray = JArrayClass<jstring>::newArray(map_.size());
   jint i = 0;
-  for (auto &pair : pairs) {
+  for (auto &pair : map_.items()) {
     auto value = pair.first.asString();
-    keys_.value().push_back(value);
+    (*keys_).push_back(value);
     (*jarray)[i++] = make_jstring(value);
   }
 
   return jarray;
 }
 
 local_ref<JArrayClass<jobject>> ReadableNativeMap::importValues() {
+  throwIfConsumed();
+
   jint size = keys_.value().size();
   auto jarray = JArrayClass<jobject>::newArray(size);
   for (jint ii = 0; ii < size; ii++) {
-    const std::string &key = keys_.value()[ii].getString();
+    const std::string &key = (*keys_)[ii].getString();
     addDynamicToJArray(jarray, ii, map_.at(key));
   }
   return jarray;
 }
 
 local_ref<JArrayClass<jobject>> ReadableNativeMap::importTypes() {
+  throwIfConsumed();
+
   jint size = keys_.value().size();
   auto jarray = JArrayClass<jobject>::newArray(size);
   for (jint ii = 0; ii < size; ii++) {
-    const std::string &key = keys_.value()[ii].getString();
+    const std::string &key = (*keys_)[ii].getString();
     (*jarray)[ii] = ReadableType::getType(map_.at(key).type());
   }
   return jarray;