Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 0ee8e29

Browse files
sammy-SCfacebook-github-bot
authored andcommittedMay 26, 2021
Fix for ScrollView race condition between C++ state update and onScroll
Summary: Changelog: [internal] There is a possibility of race between updating scrollview's state and virtualised list asking for layout of individual cells. To make sure the race doesn't happen, state must be updated before dispatching onScroll event. Android has implemented a different mechanism to tackle this issue in D28558380 (facebook@b161241). Reviewed By: JoshuaGross Differential Revision: D28642737 fbshipit-source-id: 33874beac69fc5a66eeb7f459fd89cd0b00dafcf
1 parent 6b91ae7 commit 0ee8e29

File tree

5 files changed

+31
-1
lines changed

5 files changed

+31
-1
lines changed
 

‎React/Base/RCTConstants.h

+6
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ RCT_EXTERN void RCTExperimentSetOnDemandViewMounting(BOOL value);
2323
RCT_EXTERN BOOL RCTExperimentGetSendScrollEventToPaper(void);
2424
RCT_EXTERN void RCTExperimentSetSendScrollEventToPaper(BOOL value);
2525

26+
/*
27+
* Enables a fix for data race between state and scroll event.
28+
*/
29+
RCT_EXTERN BOOL RCTExperimentGetScrollViewEventRaceFix(void);
30+
RCT_EXTERN void RCTExperimentSetScrollViewEventRaceFix(BOOL value);
31+
2632
/*
2733
* Preemptive View Allocation
2834
*/

‎React/Base/RCTConstants.m

+15
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,21 @@ void RCTExperimentSetSendScrollEventToPaper(BOOL value)
4040
RCTExperimentSendScrollEventToPaper = value;
4141
}
4242

43+
/*
44+
* Enable fix for data race between state and scroll event.
45+
*/
46+
static BOOL RCTExperimentScrollViewEventRaceFix = NO;
47+
48+
BOOL RCTExperimentGetScrollViewEventRaceFix()
49+
{
50+
return RCTExperimentScrollViewEventRaceFix;
51+
}
52+
53+
void RCTExperimentSetScrollViewEventRaceFix(BOOL value)
54+
{
55+
RCTExperimentScrollViewEventRaceFix = value;
56+
}
57+
4358
/*
4459
* Preemptive View Allocation
4560
*/

‎React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm

+5
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ @implementation RCTScrollViewComponentView {
8888

8989
BOOL _isOnDemandViewMountingEnabled;
9090
BOOL _sendScrollEventToPaper;
91+
BOOL _enableScrollViewEventRaceFix;
9192
CGPoint _contentOffsetWhenClipped;
9293
NSMutableArray<UIView<RCTComponentViewProtocol> *> *_childComponentViews;
9394
}
@@ -108,6 +109,7 @@ - (instancetype)initWithFrame:(CGRect)frame
108109

109110
_isOnDemandViewMountingEnabled = RCTExperimentGetOnDemandViewMounting();
110111
_sendScrollEventToPaper = RCTExperimentGetSendScrollEventToPaper();
112+
_enableScrollViewEventRaceFix = RCTExperimentGetScrollViewEventRaceFix();
111113
_childComponentViews = [[NSMutableArray alloc] init];
112114

113115
_scrollView = [[RCTEnhancedScrollView alloc] initWithFrame:self.bounds];
@@ -417,6 +419,9 @@ - (void)scrollViewDidScroll:(UIScrollView *)scrollView
417419
if ((_lastScrollEventDispatchTime == 0) || (now - _lastScrollEventDispatchTime > _scrollEventThrottle)) {
418420
_lastScrollEventDispatchTime = now;
419421
if (_eventEmitter) {
422+
if (_enableScrollViewEventRaceFix) {
423+
[self _updateStateWithContentOffset];
424+
}
420425
std::static_pointer_cast<ScrollViewEventEmitter const>(_eventEmitter)->onScroll([self _scrollViewMetrics]);
421426
}
422427
// Once Fabric implements proper NativeAnimationDriver, this should be removed.

‎React/Fabric/RCTSurfacePresenter.mm

+4
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ - (RCTScheduler *)_createScheduler
261261
RCTExperimentSetSendScrollEventToPaper(NO);
262262
}
263263

264+
if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_state_scroll_data_race_ios")) {
265+
RCTExperimentSetScrollViewEventRaceFix(YES);
266+
}
267+
264268
if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:preemptive_view_allocation_disabled_ios")) {
265269
RCTExperimentSetPreemptiveViewAllocationDisabled(YES);
266270
}

‎ReactCommon/react/renderer/core/EventQueue.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ void EventQueue::enqueueStateUpdate(StateUpdate &&stateUpdate) const {
7777
}
7878

7979
void EventQueue::onBeat(jsi::Runtime &runtime) const {
80-
flushEvents(runtime);
8180
flushStateUpdates();
81+
flushEvents(runtime);
8282
}
8383

8484
void EventQueue::flushEvents(jsi::Runtime &runtime) const {

0 commit comments

Comments
 (0)
Please sign in to comment.