From ea913e4691cbac16b2f66a02f6439c3cccd2c28d Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Wed, 23 Nov 2016 02:12:31 -0800 Subject: [PATCH] Android: Update coalesce to prefer newest event Summary: The only callsite of `coalesce` looks like this: ``` newEvent.coalesce(oldEvent); ``` The default `coalesce` implementation returns the event with the most recent timestamp. When the events have the same timestamp then, using the variable names from above, `coalesce` returns `oldEvent`. This change updates `coalesce`'s implementation to make it explicit that it returns `this` (`newEvent` in the variable names from above) in the case of a tie. The motivation for this change is related to scroll events. In my team's app, we were seeing scroll events being emitted with the same timestamp and the coalescing logic was causing the oldest scroll event to be chosen. This was causing our JavaScript code to receive stale scroll information and the way the JavaScript code utilized this stale scroll information resulted in the ScrollView settling on the wrong scroll position. **Test plan (required)** Verified that scroll events work properly in a ScrollView in a test app. Also, my team's app uses this Closes https://github.com/facebook/react-native/pull/11080 Differential Revision: D4226152 Pulled By: andreicoman11 fbshipit-source-id: d28a2569225ca95de662f2239a0fa14de0540a7d --- .../main/java/com/facebook/react/uimanager/events/Event.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java index 4da9ed777..56eeabc50 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java @@ -67,13 +67,13 @@ public abstract class Event { /** * Given two events, coalesce them into a single event that will be sent to JS instead of two - * separate events. By default, just chooses the one the is more recent. + * separate events. By default, just chooses the one the is more recent, or {@code this} if timestamps are the same. * * Two events will only ever try to be coalesced if they have the same event name, view id, and * coalescing key. */ public T coalesce(T otherEvent) { - return (T) (getTimestampMs() > otherEvent.getTimestampMs() ? this : otherEvent); + return (T) (getTimestampMs() >= otherEvent.getTimestampMs() ? this : otherEvent); } /**