From 5f09ca4273a3bef6a907cce9843f5a8cde04ac23 Mon Sep 17 00:00:00 2001 From: Andy Street Date: Thu, 16 Mar 2017 07:40:54 -0700 Subject: [PATCH] BREAKING: Only call batchDidComplete when there were actually native calls dispatched Summary: This is breaking because it affects the contract for onBatchComplete, but modules really shouldn't (and probably aren't) depending on it being called without any actual native module method calls having happened. Reviewed By: javache Differential Revision: D4715656 fbshipit-source-id: 53ddd4a26c9949de86f5111d214b3e5002ca2e94 --- ReactCommon/cxxreact/NativeToJsBridge.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ReactCommon/cxxreact/NativeToJsBridge.cpp b/ReactCommon/cxxreact/NativeToJsBridge.cpp index 347d5ad9e75..b52548d6caa 100644 --- a/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -49,6 +49,8 @@ public: JSExecutor& executor, folly::dynamic&& calls, bool isEndOfBatch) override { ExecutorToken token = m_nativeToJs->getTokenForExecutor(executor); m_nativeQueue->runOnQueue([this, token, calls=std::move(calls), isEndOfBatch] () mutable { + m_batchHadNativeModuleCalls = m_batchHadNativeModuleCalls || !calls.empty(); + // An exception anywhere in here stops processing of the batch. This // was the behavior of the Android bridge, and since exception handling // terminates the whole bridge, there's not much point in continuing. @@ -57,7 +59,10 @@ public: token, call.moduleId, call.methodId, std::move(call.arguments), call.callId); } if (isEndOfBatch) { - m_callback->onBatchComplete(); + if (m_batchHadNativeModuleCalls) { + m_callback->onBatchComplete(); + m_batchHadNativeModuleCalls = false; + } m_callback->decrementPendingJSCalls(); } }); @@ -85,6 +90,7 @@ private: std::shared_ptr m_registry; std::unique_ptr m_nativeQueue; std::shared_ptr m_callback; + bool m_batchHadNativeModuleCalls = false; }; NativeToJsBridge::NativeToJsBridge(