Skip to content

Commit 7259cb3

Browse files
javachefacebook-github-bot
authored andcommitted
Forward-declare imports in Binding and FabricMountingManager (facebook#36609)
Summary: Pull Request resolved: facebook#36609 Some random cleanup as I prepare to make these classes a better injection point for future experiments. * Forward-declare classes where possible to reduce header import * Return references to shared_ptr instead of copies when there are no lifetime concerns * Use a shared JClass instance in JFabricUIManager Changelog: [Internal] Reviewed By: rshest Differential Revision: D44221018 fbshipit-source-id: 1660cac964abd10ce798473e26841503430efdfe
1 parent 3759a26 commit 7259cb3

File tree

10 files changed

+122
-153
lines changed

10 files changed

+122
-153
lines changed

packages/react-native/React/Fabric/RCTScheduler.mm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
public:
2525
SchedulerDelegateProxy(void *scheduler) : scheduler_(scheduler) {}
2626

27-
void schedulerDidFinishTransaction(MountingCoordinator::Shared mountingCoordinator) override
27+
void schedulerDidFinishTransaction(const MountingCoordinator::Shared &mountingCoordinator) override
2828
{
2929
RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
30-
[scheduler.delegate schedulerDidFinishTransaction:std::move(mountingCoordinator)];
30+
[scheduler.delegate schedulerDidFinishTransaction:mountingCoordinator];
3131
}
3232

3333
void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowNode &shadowNode) override

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/Binding.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public Binding() {
4040
private native void installFabricUIManager(
4141
RuntimeExecutor runtimeExecutor,
4242
RuntimeScheduler runtimeScheduler,
43-
Object uiManager,
43+
FabricUIManager uiManager,
4444
EventBeatManager eventBeatManager,
4545
ComponentFactory componentsRegistry,
4646
Object reactNativeConfig);

packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,24 @@
88
#include "Binding.h"
99

1010
#include "AsyncEventBeat.h"
11+
#include "ComponentFactory.h"
12+
#include "EventBeatManager.h"
1113
#include "EventEmitterWrapper.h"
14+
#include "FabricMountingManager.h"
1215
#include "JBackgroundExecutor.h"
1316
#include "ReactNativeConfigHolder.h"
1417
#include "StateWrapperImpl.h"
18+
#include "SurfaceHandlerBinding.h"
1519

1620
#include <cfenv>
1721
#include <cmath>
1822

1923
#include <fbjni/fbjni.h>
24+
#include <glog/logging.h>
2025
#include <jsi/JSIDynamic.h>
2126
#include <jsi/jsi.h>
2227
#include <react/renderer/animations/LayoutAnimationDriver.h>
2328
#include <react/renderer/componentregistry/ComponentDescriptorFactory.h>
24-
#include <react/renderer/components/scrollview/ScrollViewProps.h>
2529
#include <react/renderer/core/CoreFeatures.h>
2630
#include <react/renderer/core/EventBeat.h>
2731
#include <react/renderer/core/EventEmitter.h>
@@ -33,14 +37,6 @@
3337
#include <react/renderer/uimanager/primitives.h>
3438
#include <react/utils/ContextContainer.h>
3539

36-
// Included to set BaseTextProps config; can be deleted later.
37-
#include <react/renderer/components/text/BaseTextProps.h>
38-
39-
#include <glog/logging.h>
40-
41-
using namespace facebook::jni;
42-
using namespace facebook::jsi;
43-
4440
namespace facebook {
4541
namespace react {
4642

@@ -50,15 +46,15 @@ jni::local_ref<Binding::jhybriddata> Binding::initHybrid(
5046
}
5147

5248
// Thread-safe getter
53-
std::shared_ptr<Scheduler> Binding::getScheduler() {
49+
const std::shared_ptr<Scheduler> &Binding::getScheduler() {
5450
std::shared_lock lock(installMutex_);
5551
return scheduler_;
5652
}
5753

5854
jni::local_ref<ReadableNativeMap::jhybridobject>
5955
Binding::getInspectorDataForInstance(
6056
jni::alias_ref<EventEmitterWrapper::javaobject> eventEmitterWrapper) {
61-
std::shared_ptr<Scheduler> scheduler = getScheduler();
57+
auto &scheduler = getScheduler();
6258
if (!scheduler) {
6359
LOG(ERROR) << "Binding::startSurface: scheduler disappeared";
6460
return ReadableNativeMap::newObjectCxxArgs(folly::dynamic::object());
@@ -82,15 +78,14 @@ Binding::getInspectorDataForInstance(
8278
return ReadableNativeMap::newObjectCxxArgs(result);
8379
}
8480

85-
constexpr static auto ReactFeatureFlagsJavaDescriptor =
81+
constexpr static auto kReactFeatureFlagsJavaDescriptor =
8682
"com/facebook/react/config/ReactFeatureFlags";
8783

8884
static bool getFeatureFlagValue(const char *name) {
89-
static const auto reactFeatureFlagsJavaDescriptor =
90-
jni::findClassStatic(ReactFeatureFlagsJavaDescriptor);
91-
const auto field =
92-
reactFeatureFlagsJavaDescriptor->getStaticField<jboolean>(name);
93-
return reactFeatureFlagsJavaDescriptor->getStaticFieldValue(field);
85+
static const auto reactFeatureFlagsClass =
86+
jni::findClassStatic(kReactFeatureFlagsJavaDescriptor);
87+
const auto field = reactFeatureFlagsClass->getStaticField<jboolean>(name);
88+
return reactFeatureFlagsClass->getStaticFieldValue(field);
9489
}
9590

9691
void Binding::setPixelDensity(float pointScaleFactor) {
@@ -109,7 +104,7 @@ void Binding::startSurface(
109104
NativeMap *initialProps) {
110105
SystraceSection s("FabricUIManagerBinding::startSurface");
111106

112-
std::shared_ptr<Scheduler> scheduler = getScheduler();
107+
auto &scheduler = getScheduler();
113108
if (!scheduler) {
114109
LOG(ERROR) << "Binding::startSurface: scheduler disappeared";
115110
return;
@@ -137,7 +132,7 @@ void Binding::startSurface(
137132
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
138133
}
139134

140-
auto mountingManager =
135+
auto &mountingManager =
141136
verifyMountingManager("FabricUIManagerBinding::startSurface");
142137
if (!mountingManager) {
143138
return;
@@ -165,7 +160,7 @@ void Binding::startSurfaceWithConstraints(
165160
<< this << ", surfaceId: " << surfaceId << ").";
166161
}
167162

168-
std::shared_ptr<Scheduler> scheduler = getScheduler();
163+
auto &scheduler = getScheduler();
169164
if (!scheduler) {
170165
LOG(ERROR) << "Binding::startSurfaceWithConstraints: scheduler disappeared";
171166
return;
@@ -208,7 +203,7 @@ void Binding::startSurfaceWithConstraints(
208203
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
209204
}
210205

211-
auto mountingManager = verifyMountingManager(
206+
auto &mountingManager = verifyMountingManager(
212207
"FabricUIManagerBinding::startSurfaceWithConstraints");
213208
if (!mountingManager) {
214209
return;
@@ -219,13 +214,13 @@ void Binding::startSurfaceWithConstraints(
219214
void Binding::renderTemplateToSurface(jint surfaceId, jstring uiTemplate) {
220215
SystraceSection s("FabricUIManagerBinding::renderTemplateToSurface");
221216

222-
std::shared_ptr<Scheduler> scheduler = getScheduler();
217+
auto &scheduler = getScheduler();
223218
if (!scheduler) {
224219
LOG(ERROR) << "Binding::renderTemplateToSurface: scheduler disappeared";
225220
return;
226221
}
227222

228-
auto env = Environment::current();
223+
auto env = jni::Environment::current();
229224
const char *nativeString = env->GetStringUTFChars(uiTemplate, JNI_FALSE);
230225
scheduler->renderTemplateToSurface(surfaceId, nativeString);
231226
env->ReleaseStringUTFChars(uiTemplate, nativeString);
@@ -239,7 +234,7 @@ void Binding::stopSurface(jint surfaceId) {
239234
<< ", surfaceId: " << surfaceId << ").";
240235
}
241236

242-
std::shared_ptr<Scheduler> scheduler = getScheduler();
237+
auto &scheduler = getScheduler();
243238
if (!scheduler) {
244239
LOG(ERROR) << "Binding::stopSurface: scheduler disappeared";
245240
return;
@@ -261,7 +256,7 @@ void Binding::stopSurface(jint surfaceId) {
261256
scheduler->unregisterSurface(surfaceHandler);
262257
}
263258

264-
auto mountingManager =
259+
auto &mountingManager =
265260
verifyMountingManager("FabricUIManagerBinding::stopSurface");
266261
if (!mountingManager) {
267262
return;
@@ -278,7 +273,7 @@ void Binding::registerSurface(SurfaceHandlerBinding *surfaceHandlerBinding) {
278273
}
279274
scheduler->registerSurface(surfaceHandler);
280275

281-
auto mountingManager =
276+
auto &mountingManager =
282277
verifyMountingManager("FabricUIManagerBinding::registerSurface");
283278
if (!mountingManager) {
284279
return;
@@ -295,7 +290,7 @@ void Binding::unregisterSurface(SurfaceHandlerBinding *surfaceHandlerBinding) {
295290
}
296291
scheduler->unregisterSurface(surfaceHandler);
297292

298-
auto mountingManager =
293+
auto &mountingManager =
299294
verifyMountingManager("FabricUIManagerBinding::unregisterSurface");
300295
if (!mountingManager) {
301296
return;
@@ -315,7 +310,7 @@ void Binding::setConstraints(
315310
jboolean doLeftAndRightSwapInRTL) {
316311
SystraceSection s("FabricUIManagerBinding::setConstraints");
317312

318-
std::shared_ptr<Scheduler> scheduler = getScheduler();
313+
auto &scheduler = getScheduler();
319314
if (!scheduler) {
320315
LOG(ERROR) << "Binding::setConstraints: scheduler disappeared";
321316
return;
@@ -358,7 +353,7 @@ void Binding::setConstraints(
358353
void Binding::installFabricUIManager(
359354
jni::alias_ref<JRuntimeExecutor::javaobject> runtimeExecutorHolder,
360355
jni::alias_ref<JRuntimeScheduler::javaobject> runtimeSchedulerHolder,
361-
jni::alias_ref<jobject> javaUIManager,
356+
jni::alias_ref<JFabricUIManager::javaobject> javaUIManager,
362357
EventBeatManager *eventBeatManager,
363358
ComponentFactory *componentsRegistry,
364359
jni::alias_ref<jobject> reactNativeConfig) {
@@ -375,8 +370,6 @@ void Binding::installFabricUIManager(
375370
<< this << ").";
376371
}
377372

378-
// Use std::lock and std::adopt_lock to prevent deadlocks by locking mutexes
379-
// at the same time
380373
std::unique_lock lock(installMutex_);
381374

382375
auto globalJavaUiManager = make_global(javaUIManager);
@@ -431,13 +424,10 @@ void Binding::installFabricUIManager(
431424

432425
CoreFeatures::cacheLastTextMeasurement =
433426
getFeatureFlagValue("enableTextMeasureCachePerShadowNode");
434-
435-
// Props setter pattern feature
436427
CoreFeatures::enablePropIteratorSetter =
437428
getFeatureFlagValue("enableCppPropsIteratorSetter");
438-
439-
// NativeState experiment
440429
CoreFeatures::useNativeState = getFeatureFlagValue("useNativeState");
430+
CoreFeatures::enableMapBuffer = getFeatureFlagValue("useMapBufferProps");
441431

442432
// RemoveDelete mega-op
443433
ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction =
@@ -480,24 +470,23 @@ void Binding::uninstallFabricUIManager() {
480470
reactNativeConfig_ = nullptr;
481471
}
482472

483-
std::shared_ptr<FabricMountingManager> Binding::verifyMountingManager(
484-
std::string const &hint) {
473+
const std::shared_ptr<FabricMountingManager> &Binding::verifyMountingManager(
474+
const char *locationHint) {
485475
std::shared_lock lock(installMutex_);
486476
if (!mountingManager_) {
487-
LOG(ERROR) << hint << " mounting manager disappeared.";
477+
LOG(ERROR) << locationHint << " mounting manager disappeared.";
488478
}
489479
return mountingManager_;
490480
}
491481

492482
void Binding::schedulerDidFinishTransaction(
493-
MountingCoordinator::Shared mountingCoordinator) {
494-
auto mountingManager =
483+
const MountingCoordinator::Shared &mountingCoordinator) {
484+
auto &mountingManager =
495485
verifyMountingManager("Binding::schedulerDidFinishTransaction");
496486
if (!mountingManager) {
497487
return;
498488
}
499-
500-
mountingManager->executeMount(std::move(mountingCoordinator));
489+
mountingManager->executeMount(mountingCoordinator);
501490
}
502491

503492
void Binding::schedulerDidRequestPreliminaryViewAllocation(
@@ -515,68 +504,62 @@ void Binding::preallocateView(
515504
ShadowNode const &shadowNode) {
516505
auto name = std::string(shadowNode.getComponentName());
517506
auto shadowView = ShadowView(shadowNode);
518-
auto mountingManager = verifyMountingManager("Binding::preallocateView");
507+
auto &mountingManager = verifyMountingManager("Binding::preallocateView");
519508
if (!mountingManager) {
520509
return;
521510
}
522-
523511
mountingManager->preallocateShadowView(surfaceId, shadowView);
524512
}
525513

526514
void Binding::schedulerDidDispatchCommand(
527515
const ShadowView &shadowView,
528516
std::string const &commandName,
529517
folly::dynamic const &args) {
530-
auto mountingManager =
518+
auto &mountingManager =
531519
verifyMountingManager("Binding::schedulerDidDispatchCommand");
532520
if (!mountingManager) {
533521
return;
534522
}
535-
536523
mountingManager->dispatchCommand(shadowView, commandName, args);
537524
}
538525

539526
void Binding::schedulerDidSendAccessibilityEvent(
540527
const ShadowView &shadowView,
541528
std::string const &eventType) {
542-
auto mountingManager =
529+
auto &mountingManager =
543530
verifyMountingManager("Binding::schedulerDidSendAccessibilityEvent");
544531
if (!mountingManager) {
545532
return;
546533
}
547-
548534
mountingManager->sendAccessibilityEvent(shadowView, eventType);
549535
}
550536

551537
void Binding::schedulerDidSetIsJSResponder(
552538
ShadowView const &shadowView,
553539
bool isJSResponder,
554540
bool blockNativeResponder) {
555-
auto mountingManager =
541+
auto &mountingManager =
556542
verifyMountingManager("Binding::schedulerDidSetIsJSResponder");
557543
if (!mountingManager) {
558544
return;
559545
}
560-
561546
mountingManager->setIsJSResponder(
562547
shadowView, isJSResponder, blockNativeResponder);
563548
}
564549

565550
void Binding::onAnimationStarted() {
566-
auto mountingManager = verifyMountingManager("Binding::onAnimationStarted");
551+
auto &mountingManager = verifyMountingManager("Binding::onAnimationStarted");
567552
if (!mountingManager) {
568553
return;
569554
}
570-
571555
mountingManager->onAnimationStarted();
572556
}
573557

574558
void Binding::onAllAnimationsComplete() {
575-
auto mountingManager = verifyMountingManager("Binding::onAnimationComplete");
559+
auto &mountingManager = verifyMountingManager("Binding::onAnimationComplete");
576560
if (!mountingManager) {
577561
return;
578562
}
579-
580563
mountingManager->onAllAnimationsComplete();
581564
}
582565

0 commit comments

Comments
 (0)