Fix event ordering when combining coalescable and non-coalescable events (#24693)

Summary:
Fixes an issue introduced in https://github.com/facebook/react-native/pull/15894 that can cause events to be dispatched out of order.

Also reverted `viewTag` moved to optional property as it didn't actually work and makes code more complex.

[iOS] [Fixed] - Fix event ordering when combining coalescable and non-coalescable events
Pull Request resolved: https://github.com/facebook/react-native/pull/24693

Differential Revision: D15279513

Pulled By: shergin

fbshipit-source-id: 3c64aba6d644ea9564572e6de8330b59b51cf4a9

React/Base/RCTEventDispatcher.h

@@ -35,6 +35,7 @@ RCT_EXTERN NSString *RCTNormalizeInputEventName(NSString *eventName);
 @protocol RCTEvent <NSObject>
 @required
 
+@property (nonatomic, strong, readonly) NSNumber *viewTag;
 @property (nonatomic, copy, readonly) NSString *eventName;
 
 - (BOOL)canCoalesce;
@@ -47,12 +48,6 @@ RCT_EXTERN NSString *RCTNormalizeInputEventName(NSString *eventName);
 
 @optional
 
-/**
- * Can be implemented for view based events that need to be coalesced
- * by it's viewTag.
- */
-@property (nonatomic, strong, readonly) NSNumber *viewTag;
-
 /**
  * Coalescing related methods must only be implemented if canCoalesce
  * returns YES.

React/Base/RCTEventDispatcher.m

@@ -27,15 +27,17 @@ NSString *RCTNormalizeInputEventName(NSString *eventName)
   return eventName;
 }
 
-static NSNumber *RCTGetEventID(id<RCTEvent> event)
+static NSNumber *RCTGetEventID(NSNumber *viewTag, NSString *eventName, uint16_t coalescingKey)
 {
   return @(
-    ([event respondsToSelector:@selector(viewTag)] ? event.viewTag.intValue : 0) |
-    (((uint64_t)event.eventName.hash & 0xFFFF) << 32) |
-    (((uint64_t)event.coalescingKey) << 48)
+    viewTag.intValue |
+    (((uint64_t)eventName.hash & 0xFFFF) << 32) |
+    (((uint64_t)coalescingKey) << 48)
   );
 }
 
+static uint16_t RCTUniqueCoalescingKeyGenerator = 0;
+
 @implementation RCTEventDispatcher
 {
   // We need this lock to protect access to _events, _eventQueue and _eventsDispatchScheduled. It's filled in on main thread and consumed on js thread.
@@ -136,38 +138,40 @@ RCT_EXPORT_MODULE()
 
   [_observersLock unlock];
 
+  [_eventQueueLock lock];
+
+  NSNumber *eventID;
   if (event.canCoalesce) {
-    [_eventQueueLock lock];
-
-    NSNumber *eventID = RCTGetEventID(event);
-
+    eventID = RCTGetEventID(event.viewTag, event.eventName, event.coalescingKey);
     id<RCTEvent> previousEvent = _events[eventID];
     if (previousEvent) {
       event = [previousEvent coalesceWithEvent:event];
     } else {
       [_eventQueue addObject:eventID];
     }
-    _events[eventID] = event;
-
-    BOOL scheduleEventsDispatch = NO;
-    if (!_eventsDispatchScheduled) {
-      _eventsDispatchScheduled = YES;
-      scheduleEventsDispatch = YES;
-    }
-
-    // We have to release the lock before dispatching block with events,
-    // since dispatchBlock: can be executed synchronously on the same queue.
-    // (This is happening when chrome debugging is turned on.)
-    [_eventQueueLock unlock];
-
-    if (scheduleEventsDispatch) {
-      [_bridge dispatchBlock:^{
-        [self flushEventsQueue];
-      } queue:RCTJSThread];
-    }
   } else {
+    id<RCTEvent> previousEvent = _events[eventID];
+    eventID = RCTGetEventID(event.viewTag, event.eventName, RCTUniqueCoalescingKeyGenerator++);
+    RCTAssert(previousEvent == nil, @"Got event %@ which cannot be coalesced, but has the same eventID %@ as the previous event %@", event, eventID, previousEvent);
+    [_eventQueue addObject:eventID];
+  }
+
+  _events[eventID] = event;
+
+  BOOL scheduleEventsDispatch = NO;
+  if (!_eventsDispatchScheduled) {
+    _eventsDispatchScheduled = YES;
+    scheduleEventsDispatch = YES;
+  }
+
+  // We have to release the lock before dispatching block with events,
+  // since dispatchBlock: can be executed synchronously on the same queue.
+  // (This is happening when chrome debugging is turned on.)
+  [_eventQueueLock unlock];
+
+  if (scheduleEventsDispatch) {
     [_bridge dispatchBlock:^{
-      [self dispatchEvent:event];
+      [self flushEventsQueue];
     } queue:RCTJSThread];
   }
 }