From ee38751975babd8db5d21d1c0940966adccce5fe Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Thu, 17 Sep 2020 13:17:48 -0700 Subject: [PATCH] Layout Events: throttle layout events sent to same node repeatedly Summary: Under Fabric only, we can enter an infinite layout loop where the emitted layout event oscillates between two height values that are off by a very small amount. The cause is, in part, components that use layoutEvents to determine their dimensions: for example, using onLayout event "height" parameters to determine the height of a child. If the onLayout height changes rapidly, the child's height will change, causing another layout, ad infinitum. This might seem like an extreme case but there are product use-cases where this is done in non-Fabric and layout stabilizes quickly. In Fabric, currently it may never stabilize. Part of this is due to a longstanding issue that exists in Fabric and non-Fabric, that we cannot immediately fix: If in a single frame, C++ emits 100 layout events to ReactJS, ReactJS may only process 50 before committing the root. That will trigger more layout events, even though product code has only partially processed the layout events. At the next frame, the next 50 events will be processed which may again change the layout, emitting more events... etc. In most cases the tree will converge and layout values will stabilize, but in extreme cases in Fabric, it might not. Part of this is because Fabric does not drop *stale* layout events. If 10 layout events are dispatched to the same node, it will process all 10 events in older. Non-Fabric does not have this behavior, so we're changing Fabric to drop stale events when they queue up. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D23719494 fbshipit-source-id: e44a3b3e40585b59680299db3a4efdc63cdf0de8 --- .../components/view/ViewEventEmitter.cpp | 35 +++++++++++++------ .../components/view/ViewEventEmitter.h | 1 + .../renderer/uimanager/UIManagerBinding.cpp | 5 +++ 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp b/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp index fe2a83f5f43..e26f7c143a6 100644 --- a/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp +++ b/ReactCommon/react/renderer/components/view/ViewEventEmitter.cpp @@ -46,16 +46,31 @@ void ViewEventEmitter::onLayout(const LayoutMetrics &layoutMetrics) const { lastLayoutMetrics_ = layoutMetrics; } - dispatchEvent("layout", [frame = layoutMetrics.frame](jsi::Runtime &runtime) { - auto layout = jsi::Object(runtime); - layout.setProperty(runtime, "x", frame.origin.x); - layout.setProperty(runtime, "y", frame.origin.y); - layout.setProperty(runtime, "width", frame.size.width); - layout.setProperty(runtime, "height", frame.size.height); - auto payload = jsi::Object(runtime); - payload.setProperty(runtime, "layout", std::move(layout)); - return payload; - }); + std::atomic_uint_fast8_t *eventCounter = &eventCounter_; + uint_fast8_t expectedEventCount = ++*eventCounter; + + // dispatchUniqueEvent only drops consecutive onLayout events to the same + // node. We want to drop *any* unprocessed onLayout events when there's a + // newer one. + dispatchEvent( + "layout", + [frame = layoutMetrics.frame, expectedEventCount, eventCounter]( + jsi::Runtime &runtime) { + uint_fast8_t actualEventCount = eventCounter->load(); + if (expectedEventCount != actualEventCount) { + // Drop stale events + return jsi::Value::null(); + } + + auto layout = jsi::Object(runtime); + layout.setProperty(runtime, "x", frame.origin.x); + layout.setProperty(runtime, "y", frame.origin.y); + layout.setProperty(runtime, "width", frame.size.width); + layout.setProperty(runtime, "height", frame.size.height); + auto payload = jsi::Object(runtime); + payload.setProperty(runtime, "layout", std::move(layout)); + return jsi::Value(std::move(payload)); + }); } } // namespace react diff --git a/ReactCommon/react/renderer/components/view/ViewEventEmitter.h b/ReactCommon/react/renderer/components/view/ViewEventEmitter.h index 0aa1f5e588e..237c9a53b10 100644 --- a/ReactCommon/react/renderer/components/view/ViewEventEmitter.h +++ b/ReactCommon/react/renderer/components/view/ViewEventEmitter.h @@ -40,6 +40,7 @@ class ViewEventEmitter : public TouchEventEmitter { private: mutable std::mutex layoutMetricsMutex_; mutable LayoutMetrics lastLayoutMetrics_; + mutable std::atomic_uint_fast8_t eventCounter_{0}; }; } // namespace react diff --git a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp index 10a9dce3cda..5c2765ca399 100644 --- a/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp +++ b/ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp @@ -135,6 +135,11 @@ void UIManagerBinding::dispatchEvent( auto payload = payloadFactory(runtime); + // If a payload is null, the factory has decided to cancel the event + if (payload.isNull()) { + return; + } + auto instanceHandle = eventTarget ? [&]() { auto instanceHandle = eventTarget->getInstanceHandle(runtime);