From 1a1b6e314fb6a9299ba279116a9d9f7a083ba6a1 Mon Sep 17 00:00:00 2001 From: Tom Duncalf Date: Tue, 26 Apr 2022 12:04:35 +0100 Subject: [PATCH] Tidy up PR --- .../xcshareddata/IDEWorkspaceChecks.plist | 8 ---- react-native/ios/RealmReact/RealmReact.mm | 39 ++++++++++++------- src/android/jsc_override.cpp | 3 ++ src/jsc/jsc_function.hpp | 2 +- src/jsc/jsc_init.h | 2 +- 5 files changed, 29 insertions(+), 25 deletions(-) delete mode 100644 example/ios/RealmTsTemplate.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist diff --git a/example/ios/RealmTsTemplate.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist b/example/ios/RealmTsTemplate.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist deleted file mode 100644 index 18d981003d..0000000000 --- a/example/ios/RealmTsTemplate.xcworkspace/xcshareddata/IDEWorkspaceChecks.plist +++ /dev/null @@ -1,8 +0,0 @@ - - - - - IDEDidComputeMac32BitWarning - - - diff --git a/react-native/ios/RealmReact/RealmReact.mm b/react-native/ios/RealmReact/RealmReact.mm index 0880c42876..dc620a5233 100644 --- a/react-native/ios/RealmReact/RealmReact.mm +++ b/react-native/ios/RealmReact/RealmReact.mm @@ -52,6 +52,7 @@ - (JSContext *)context; @interface RCTBridge (Realm_RCTCxxBridge) - (JSGlobalContextRef)jsContextRef; - (void *)runtime; +// Expose the CallInvoker so that we can call `invokeAsync` - (std::shared_ptr)jsCallInvoker; @end @@ -85,7 +86,8 @@ @interface RealmReact () @implementation RealmReact { NSMutableDictionary *_eventHandlers; - bool waitingForFlush; + // Keep track of whether we are already waiting for the React Native UI queue to be flushed asynchronously + bool waitingForUiFlush; #if DEBUG GCDWebServer *_webServer; @@ -111,7 +113,7 @@ - (instancetype)init { self = [super init]; if (self) { _eventHandlers = [[NSMutableDictionary alloc] init]; - waitingForFlush = false; + waitingForUiFlush = false; } return self; } @@ -278,7 +280,7 @@ - (void)dealloc { typedef JSGlobalContextRef (^JSContextRefExtractor)(); -void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::function flushUiQueue) { +void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::function flushUiQueueFn) { // Make sure the previous JS thread is completely finished before continuing. static __weak NSThread *s_currentJSThread; while (s_currentJSThread && !s_currentJSThread.finished) { @@ -286,7 +288,7 @@ void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::functi } s_currentJSThread = [NSThread currentThread]; - RJSInitializeInContext(jsContextExtractor(), flushUiQueue); + RJSInitializeInContext(jsContextExtractor(), flushUiQueueFn); } - (void)setBridge:(RCTBridge *)bridge { @@ -330,20 +332,27 @@ - (void)setBridge:(RCTBridge *)bridge { }; return static_cast(bridge.runtime)->ctx_; }, ^{ - // Calling jsCallInvokver->invokeAsync causes React Native to flush any pending UI - // updates. We call this after we have called into JS from C++, in order to ensure - // the UI updates in response to any changes, as we bypass the "normal" RN calling - // mechanisms (see #4389, facebook/react-native#33006). + // Calling jsCallInvokver->invokeAsync tells React Native to execute the lambda passed + // in on the JS thread, and then flush the internal "microtask queue", which has the + // effect of flushing any pending UI updates. // - // Calls are debounced using the waitingForFlush flag, so if an async flush is already - // pending when another JS to C++ call happens, we don't call invokeAsync again. It - // might be possible to further optimize this, e.g. only call max once per frame. - if (!waitingForFlush) { - waitingForFlush = true; + // We call this after we have called into JS from C++, in order to ensure that the RN + // UI updates in response to any changes from Realm. We need to do this as we bypass + // the usual RN bridge mechanism for communicating between C++ and JS, so without doing + // this RN has no way to know that a change has occurred which might require an update + // (see #4389, facebook/react-native#33006). + // + // Calls are debounced using the waitingForUiFlush flag, so if an async flush is already + // pending when another JS to C++ call happens, we don't call invokeAsync again. This works + // because the work is performed before the microtask queue is flushed - see sequence + // diagram at https://bit.ly/3kexhHm. It might be possible to further optimize this, + // e.g. only flush the queue a maximum of once per frame, but this seems reasonable. + if (!waitingForUiFlush) { + waitingForUiFlush = true; [bridge jsCallInvoker]->invokeAsync([&](){ - waitingForFlush = false; + waitingForUiFlush = false; - if (waitingForFlush) { + if (waitingForUiFlush) { [bridge jsCallInvoker]->invokeAsync([&](){}); } }); diff --git a/src/android/jsc_override.cpp b/src/android/jsc_override.cpp index 450897ad0f..580a42e2bd 100644 --- a/src/android/jsc_override.cpp +++ b/src/android/jsc_override.cpp @@ -144,6 +144,9 @@ static JSGlobalContextRef create_context(JSContextGroupRef group, JSClassRef glo // Clear cache from previous instances. RJSInvalidateCaches(); + // We pass a no-op lambda for the `flushUiQueue` function, as there's no easy way to + // access the React Native context from here and the UI flush bug (#4389) has not been + // seen on Android. We should be able to do this properly on Hermes. RJSInitializeInContext(ctx, []() {}); realmContextInjected = true; return ctx; diff --git a/src/jsc/jsc_function.hpp b/src/jsc/jsc_function.hpp index d920009eac..4ce04ef477 100644 --- a/src/jsc/jsc_function.hpp +++ b/src/jsc/jsc_function.hpp @@ -23,7 +23,7 @@ namespace realm { namespace js { -// TODO +// Function passed in from the React Native initialisation code to flush the UI microtask queue extern std::function flush_ui_queue; template <> diff --git a/src/jsc/jsc_init.h b/src/jsc/jsc_init.h index 8d88e4d462..96f7ddb168 100644 --- a/src/jsc/jsc_init.h +++ b/src/jsc/jsc_init.h @@ -26,7 +26,7 @@ extern "C" { #endif JSObjectRef RJSConstructorCreate(JSContextRef ctx); -void RJSInitializeInContext(JSContextRef ctx, std::function send_dummy_event); +void RJSInitializeInContext(JSContextRef ctx, std::function flush_ui_queue); void RJSInvalidateCaches(); #ifdef __cplusplus