Skip to content

Commit

Permalink
BREAKING: Only call batchDidComplete when there were actually native …
Browse files Browse the repository at this point in the history
…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 facebook-github-bot committed Mar 16, 2017
1 parent d5dda1b commit c8d922b
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion ReactCommon/cxxreact/NativeToJsBridge.cpp
Expand Up @@ -52,6 +52,8 @@ class JsToNativeBridge : public react::ExecutorDelegate {
"native module calls cannot be completed with no native modules";
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.
Expand All @@ -60,7 +62,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();
}
});
Expand Down Expand Up @@ -88,6 +93,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(
Expand Down

0 comments on commit c8d922b

Please sign in to comment.