diff --git a/ReactCommon/react/bridging/CallbackWrapper.h b/ReactCommon/react/bridging/CallbackWrapper.h index 885cb209997..52dd1d6103d 100644 --- a/ReactCommon/react/bridging/CallbackWrapper.h +++ b/ReactCommon/react/bridging/CallbackWrapper.h @@ -35,8 +35,8 @@ class CallbackWrapper : public LongLivedObject { jsi::Function &&callback, jsi::Runtime &runtime, std::shared_ptr jsInvoker) { - auto wrapper = std::shared_ptr( - new CallbackWrapper(std::move(callback), runtime, jsInvoker)); + auto wrapper = std::shared_ptr(new CallbackWrapper( + std::move(callback), runtime, std::move(jsInvoker))); LongLivedObjectCollection::get().add(wrapper); return wrapper; } diff --git a/ReactCommon/react/bridging/LongLivedObject.cpp b/ReactCommon/react/bridging/LongLivedObject.cpp index dc6d44dbe61..f5616fd5a87 100644 --- a/ReactCommon/react/bridging/LongLivedObject.cpp +++ b/ReactCommon/react/bridging/LongLivedObject.cpp @@ -16,27 +16,22 @@ LongLivedObjectCollection &LongLivedObjectCollection::get() { return instance; } -LongLivedObjectCollection::LongLivedObjectCollection() {} - -void LongLivedObjectCollection::add(std::shared_ptr so) const { +void LongLivedObjectCollection::add(std::shared_ptr so) { std::lock_guard lock(collectionMutex_); - collection_.insert(so); + collection_.insert(std::move(so)); } -void LongLivedObjectCollection::remove(const LongLivedObject *o) const { +void LongLivedObjectCollection::remove(const LongLivedObject *o) { std::lock_guard lock(collectionMutex_); - auto p = collection_.begin(); - for (; p != collection_.end(); p++) { + for (auto p = collection_.begin(); p != collection_.end(); p++) { if (p->get() == o) { + collection_.erase(p); break; } } - if (p != collection_.end()) { - collection_.erase(p); - } } -void LongLivedObjectCollection::clear() const { +void LongLivedObjectCollection::clear() { std::lock_guard lock(collectionMutex_); collection_.clear(); } @@ -47,8 +42,6 @@ size_t LongLivedObjectCollection::size() const { } // LongLivedObject -LongLivedObject::LongLivedObject() {} -LongLivedObject::~LongLivedObject() {} void LongLivedObject::allowRelease() { LongLivedObjectCollection::get().remove(this); diff --git a/ReactCommon/react/bridging/LongLivedObject.h b/ReactCommon/react/bridging/LongLivedObject.h index cfb10086d09..2c2b5bf026e 100644 --- a/ReactCommon/react/bridging/LongLivedObject.h +++ b/ReactCommon/react/bridging/LongLivedObject.h @@ -26,11 +26,11 @@ namespace react { */ class LongLivedObject { public: - virtual void allowRelease(); + void allowRelease(); protected: - LongLivedObject(); - virtual ~LongLivedObject(); + LongLivedObject() = default; + virtual ~LongLivedObject() = default; }; /** @@ -40,17 +40,18 @@ class LongLivedObjectCollection { public: static LongLivedObjectCollection &get(); - LongLivedObjectCollection(); LongLivedObjectCollection(LongLivedObjectCollection const &) = delete; void operator=(LongLivedObjectCollection const &) = delete; - void add(std::shared_ptr o) const; - void remove(const LongLivedObject *o) const; - void clear() const; + void add(std::shared_ptr o); + void remove(const LongLivedObject *o); + void clear(); size_t size() const; private: - mutable std::unordered_set> collection_; + LongLivedObjectCollection() = default; + + std::unordered_set> collection_; mutable std::mutex collectionMutex_; }; diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.cpp b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.cpp index cf9726aa020..866277587de 100644 --- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.cpp +++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.cpp @@ -79,13 +79,13 @@ void Promise::reject(const std::string &message) { jsi::Value createPromiseAsJSIValue( jsi::Runtime &rt, - const PromiseSetupFunctionType func) { + PromiseSetupFunctionType &&func) { jsi::Function JSPromise = rt.global().getPropertyAsFunction(rt, "Promise"); jsi::Function fn = jsi::Function::createFromHostFunction( rt, jsi::PropNameID::forAscii(rt, "fn"), 2, - [func]( + [func = std::move(func)]( jsi::Runtime &rt2, const jsi::Value &thisVal, const jsi::Value *args, diff --git a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h index 613665898dc..0bc4a864327 100644 --- a/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h +++ b/ReactCommon/react/nativemodule/core/ReactCommon/TurboModuleUtils.h @@ -36,7 +36,7 @@ using PromiseSetupFunctionType = std::function)>; jsi::Value createPromiseAsJSIValue( jsi::Runtime &rt, - const PromiseSetupFunctionType func); + PromiseSetupFunctionType &&func); class RAIICallbackWrapperDestroyer { public: diff --git a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 3240769c64a..280780b35a6 100644 --- a/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -77,12 +78,13 @@ jni::local_ref createJavaCallbackFromJSIFunction( auto callbackWrapperOwner = std::make_shared(weakWrapper); - std::function fn = - [weakWrapper, callbackWrapperOwner, wrapperWasCalled = false]( - folly::dynamic responses) mutable { + return JCxxCallbackImpl::newObjectCxxArgs( + [weakWrapper = std::move(weakWrapper), + callbackWrapperOwner = std::move(callbackWrapperOwner), + wrapperWasCalled = false](folly::dynamic responses) mutable { if (wrapperWasCalled) { throw std::runtime_error( - "callback 2 arg cannot be called more than once"); + "Callback arg cannot be called more than once"); } auto strongWrapper = weakWrapper.lock(); @@ -91,37 +93,29 @@ jni::local_ref createJavaCallbackFromJSIFunction( } strongWrapper->jsInvoker().invokeAsync( - [weakWrapper, callbackWrapperOwner, responses]() mutable { + [weakWrapper = std::move(weakWrapper), + callbackWrapperOwner = std::move(callbackWrapperOwner), + responses = std::move(responses)]() { auto strongWrapper2 = weakWrapper.lock(); if (!strongWrapper2) { return; } - // TODO (T43155926) valueFromDynamic already returns a Value - // array. Don't iterate again - jsi::Value args = - jsi::valueFromDynamic(strongWrapper2->runtime(), responses); - auto argsArray = args.getObject(strongWrapper2->runtime()) - .asArray(strongWrapper2->runtime()); - std::vector result; - for (size_t i = 0; i < argsArray.size(strongWrapper2->runtime()); - i++) { - result.emplace_back( - strongWrapper2->runtime(), - argsArray.getValueAtIndex(strongWrapper2->runtime(), i)); + std::vector args; + args.reserve(responses.size()); + for (const auto &val : responses) { + args.emplace_back( + jsi::valueFromDynamic(strongWrapper2->runtime(), val)); } + strongWrapper2->callback().call( strongWrapper2->runtime(), - (const jsi::Value *)result.data(), - result.size()); - - callbackWrapperOwner.reset(); + (const jsi::Value *)args.data(), + args.size()); }); wrapperWasCalled = true; - }; - - return JCxxCallbackImpl::newObjectCxxArgs(fn); + }); } // This is used for generating short exception strings. @@ -369,8 +363,7 @@ jsi::Value convertFromJMapToValue(JNIEnv *env, jsi::Runtime &rt, jobject arg) { jArguments, "makeNativeMap", "(Ljava/util/Map;)Lcom/facebook/react/bridge/WritableNativeMap;"); - auto constants = - (jobject)env->CallStaticObjectMethod(jArguments, jMakeNativeMap, arg); + auto constants = env->CallStaticObjectMethod(jArguments, jMakeNativeMap, arg); auto jResult = jni::adopt_local(constants); auto result = jni::static_ref_cast(jResult); return jsi::valueFromDynamic(rt, result->cthis()->consume());