Permalink
Browse files

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
  • Loading branch information...
astreet authored and grabbou committed Mar 16, 2017
1 parent 34d8a2b commit 5f09ca4273a3bef6a907cce9843f5a8cde04ac23
Showing with 7 additions and 1 deletion.
  1. +7 −1 ReactCommon/cxxreact/NativeToJsBridge.cpp
@@ -49,6 +49,8 @@ class JsToNativeBridge : public react::ExecutorDelegate {
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 @@ class JsToNativeBridge : public react::ExecutorDelegate {
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 @@ class JsToNativeBridge : public react::ExecutorDelegate {
std::shared_ptr<ModuleRegistry> m_registry;
std::unique_ptr<MessageQueueThread> m_nativeQueue;
std::shared_ptr<InstanceCallback> m_callback;
bool m_batchHadNativeModuleCalls = false;
};
NativeToJsBridge::NativeToJsBridge(

0 comments on commit 5f09ca4

Please sign in to comment.