Skip to content

Commit ef6f3f6

Browse files
javachemeta-codesync[bot]
authored andcommitted
Remove move constructor from SurfaceHandler (#54108)
Summary: Pull Request resolved: #54108 Removed SurfaceHandler's move constructor, to avoid any potential issues with referencing moved-from instances. Changelog: [Internal] Reviewed By: Abbondanzo, mdvacca Differential Revision: D83977790 fbshipit-source-id: 3ea7a64f2183b7f28c62cca06537d3a26ba0bea8
1 parent 7aae556 commit ef6f3f6

5 files changed

Lines changed: 42 additions & 53 deletions

File tree

packages/react-native/React/Fabric/Surface/RCTFabricSurface.mm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ - (instancetype)initWithSurfacePresenter:(RCTSurfacePresenter *)surfacePresenter
5757
if (self = [super init]) {
5858
_surfacePresenter = surfacePresenter;
5959

60-
_surfaceHandler = SurfaceHandler{RCTStringFromNSString(moduleName), getNextRootViewTag()};
60+
_surfaceHandler.emplace(RCTStringFromNSString(moduleName), getNextRootViewTag());
6161
_surfaceHandler->setProps(convertIdToFollyDynamic(initialProperties));
6262

6363
[_surfacePresenter registerSurface:self];

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

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -166,34 +166,39 @@ void FabricUIManagerBinding::startSurface(
166166
return;
167167
}
168168

169-
auto layoutContext = LayoutContext{};
170-
layoutContext.pointScaleFactor = pointScaleFactor_;
169+
SurfaceHandler* surfaceHandler = nullptr;
170+
{
171+
std::unique_lock lock(surfaceHandlerRegistryMutex_);
172+
auto [it, _] = surfaceHandlerRegistry_.try_emplace(
173+
surfaceId,
174+
std::in_place_index<0>,
175+
moduleName->toStdString(),
176+
surfaceId);
177+
surfaceHandler = &std::get<SurfaceHandler>(it->second);
178+
}
171179

172-
auto surfaceHandler = SurfaceHandler{moduleName->toStdString(), surfaceId};
173-
surfaceHandler.setContextContainer(scheduler->getContextContainer());
180+
surfaceHandler->setContextContainer(scheduler->getContextContainer());
174181
if (initialProps != nullptr) {
175-
surfaceHandler.setProps(initialProps->consume());
182+
surfaceHandler->setProps(initialProps->consume());
176183
}
177-
surfaceHandler.constraintLayout({}, layoutContext);
178184

179-
scheduler->registerSurface(surfaceHandler);
185+
auto layoutContext = LayoutContext{};
186+
layoutContext.pointScaleFactor = pointScaleFactor_;
187+
surfaceHandler->constraintLayout({}, layoutContext);
188+
189+
scheduler->registerSurface(*surfaceHandler);
180190

181191
auto mountingManager = getMountingManager("startSurface");
182192
if (mountingManager != nullptr) {
183193
mountingManager->onSurfaceStart(surfaceId);
184194
}
185195

186-
surfaceHandler.start();
196+
surfaceHandler->start();
187197

188198
if (ReactNativeFeatureFlags::enableLayoutAnimationsOnAndroid()) {
189-
surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate(
199+
surfaceHandler->getMountingCoordinator()->setMountingOverrideDelegate(
190200
animationDriver_);
191201
}
192-
193-
{
194-
std::unique_lock lock(surfaceHandlerRegistryMutex_);
195-
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
196-
}
197202
}
198203

199204
jint FabricUIManagerBinding::findNextFocusableElement(
@@ -338,31 +343,36 @@ void FabricUIManagerBinding::startSurfaceWithConstraints(
338343
constraints.layoutDirection =
339344
isRTL != 0 ? LayoutDirection::RightToLeft : LayoutDirection::LeftToRight;
340345

341-
auto surfaceHandler = SurfaceHandler{moduleName->toStdString(), surfaceId};
342-
surfaceHandler.setContextContainer(scheduler->getContextContainer());
346+
SurfaceHandler* surfaceHandler = nullptr;
347+
{
348+
std::unique_lock lock(surfaceHandlerRegistryMutex_);
349+
auto [it, _] = surfaceHandlerRegistry_.try_emplace(
350+
surfaceId,
351+
std::in_place_index<0>,
352+
moduleName->toStdString(),
353+
surfaceId);
354+
surfaceHandler = &std::get<SurfaceHandler>(it->second);
355+
}
356+
357+
surfaceHandler->setContextContainer(scheduler->getContextContainer());
343358
if (initialProps != nullptr) {
344-
surfaceHandler.setProps(initialProps->consume());
359+
surfaceHandler->setProps(initialProps->consume());
345360
}
346-
surfaceHandler.constraintLayout(constraints, context);
361+
surfaceHandler->constraintLayout(constraints, context);
347362

348-
scheduler->registerSurface(surfaceHandler);
363+
scheduler->registerSurface(*surfaceHandler);
349364

350365
auto mountingManager = getMountingManager("startSurfaceWithConstraints");
351366
if (mountingManager != nullptr) {
352367
mountingManager->onSurfaceStart(surfaceId);
353368
}
354369

355-
surfaceHandler.start();
370+
surfaceHandler->start();
356371

357372
if (ReactNativeFeatureFlags::enableLayoutAnimationsOnAndroid()) {
358-
surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate(
373+
surfaceHandler->getMountingCoordinator()->setMountingOverrideDelegate(
359374
animationDriver_);
360375
}
361-
362-
{
363-
std::unique_lock lock(surfaceHandlerRegistryMutex_);
364-
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
365-
}
366376
}
367377

368378
// Used by non-bridgeless+Fabric

packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.cpp

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,6 @@ SurfaceHandler::SurfaceHandler(
2323
parameters_.surfaceId = surfaceId;
2424
}
2525

26-
SurfaceHandler::SurfaceHandler(SurfaceHandler&& other) noexcept {
27-
operator=(std::move(other));
28-
}
29-
30-
SurfaceHandler& SurfaceHandler::operator=(SurfaceHandler&& other) noexcept {
31-
std::unique_lock lock1(linkMutex_, std::defer_lock);
32-
std::unique_lock lock2(parametersMutex_, std::defer_lock);
33-
std::unique_lock lock3(other.linkMutex_, std::defer_lock);
34-
std::unique_lock lock4(other.parametersMutex_, std::defer_lock);
35-
std::lock(lock1, lock2, lock3, lock4);
36-
37-
link_ = other.link_;
38-
parameters_ = other.parameters_;
39-
40-
other.link_ = Link{};
41-
other.parameters_ = Parameters{};
42-
other.parameters_.contextContainer = parameters_.contextContainer;
43-
return *this;
44-
}
45-
4626
#pragma mark - Surface Life-Cycle Management
4727

4828
void SurfaceHandler::setContextContainer(

packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceHandler.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ class SurfaceHandler {
6767
virtual ~SurfaceHandler() noexcept;
6868

6969
/*
70-
* Movable-only.
70+
* Not moveable or copyable
7171
*/
72-
SurfaceHandler(SurfaceHandler &&other) noexcept;
72+
SurfaceHandler(SurfaceHandler &&other) noexcept = delete;
7373
SurfaceHandler(const SurfaceHandler &SurfaceHandler) noexcept = delete;
74-
SurfaceHandler &operator=(SurfaceHandler &&other) noexcept;
74+
SurfaceHandler &operator=(SurfaceHandler &&other) noexcept = delete;
7575
SurfaceHandler &operator=(const SurfaceHandler &other) noexcept = delete;
7676

7777
#pragma mark - Surface Life-Cycle Management

packages/react-native/ReactCommon/react/renderer/scheduler/SurfaceManager.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,8 @@ void SurfaceManager::startSurface(
2929
const LayoutContext& layoutContext) noexcept {
3030
{
3131
std::unique_lock lock(mutex_);
32-
auto surfaceHandler = SurfaceHandler{moduleName, surfaceId};
33-
surfaceHandler.setContextContainer(scheduler_.getContextContainer());
34-
registry_.emplace(surfaceId, std::move(surfaceHandler));
32+
auto [it, _] = registry_.try_emplace(surfaceId, moduleName, surfaceId);
33+
it->second.setContextContainer(scheduler_.getContextContainer());
3534
}
3635

3736
visit(surfaceId, [&](const SurfaceHandler& surfaceHandler) {

0 commit comments

Comments
 (0)