Skip to content

Commit 8a538b0

Browse files
zeyapfacebook-github-bot
zeyap
authored and
facebook-github-bot
committed
Clear AnimatedPropsRegistry on surface stop (facebook#56485)
Summary: ## Changelog: [Internal] [Fixed] - Clear AnimatedPropsRegistry on surface stop When `useSharedAnimatedBackend` is enabled, the `AnimatedPropsRegistry` accumulates `SurfaceContext` entries (containing `shared_ptr<ShadowNodeFamily>` and `PropsSnapshot` data) for each surface that has animated views. These entries are never cleaned up when a surface is destroyed via `UIManager::stopSurface()`, because that method only calls the legacy `stopSurfaceForAnimationDelegate()` — the shared backend's registry is untouched. We also see increased `RetryableMountingLayerException` errors in production which stack trace shows there are mount items committing to surface that no longer exists. This change: 1. Adds `animationBackend_->clearRegistry(surfaceId)` to `UIManager::stopSurface()` 2. Changes `AnimatedPropsRegistry::clear()` to fully erase the `surfaceContexts_` map entry instead of just clearing its contents Reviewed By: christophpurrer Differential Revision: D101354994
1 parent 68debb2 commit 8a538b0

12 files changed

Lines changed: 46 additions & 10 deletions

File tree

‎packages/react-native/ReactCommon/react/renderer/animationbackend/AnimatedPropsRegistry.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,17 @@ AnimatedPropsRegistry::getMap(SurfaceId surfaceId) {
9393

9494
void AnimatedPropsRegistry::clear(SurfaceId surfaceId) {
9595
auto lock = std::lock_guard(mutex_);
96+
if (auto it = surfaceContexts_.find(surfaceId);
97+
it != surfaceContexts_.end()) {
98+
auto& surfaceContext = it->second;
99+
surfaceContext.families.clear();
100+
surfaceContext.map.clear();
101+
}
102+
}
96103

97-
auto& surfaceContext = surfaceContexts_[surfaceId];
98-
surfaceContext.families.clear();
99-
surfaceContext.map.clear();
104+
void AnimatedPropsRegistry::clearOnSurfaceStop(SurfaceId surfaceId) {
105+
auto lock = std::lock_guard(mutex_);
106+
surfaceContexts_.erase(surfaceId);
100107
}
101108

102109
} // namespace facebook::react

‎packages/react-native/ReactCommon/react/renderer/animationbackend/AnimatedPropsRegistry.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class AnimatedPropsRegistry {
3939
public:
4040
void update(const std::unordered_map<SurfaceId, SurfaceUpdates> &surfaceUpdates);
4141
void clear(SurfaceId surfaceId);
42+
void clearOnSurfaceStop(SurfaceId surfaceId);
4243
std::pair<std::unordered_set<std::shared_ptr<const ShadowNodeFamily>> &, SnapshotMap &> getMap(SurfaceId surfaceId);
4344

4445
private:

‎packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,10 @@ void AnimationBackend::clearRegistry(SurfaceId surfaceId) {
257257
animatedPropsRegistry_->clear(surfaceId);
258258
}
259259

260+
void AnimationBackend::clearRegistryOnSurfaceStop(SurfaceId surfaceId) {
261+
animatedPropsRegistry_->clearOnSurfaceStop(surfaceId);
262+
}
263+
260264
void AnimationBackend::registerJSInvoker(
261265
std::shared_ptr<CallInvoker> jsInvoker) {
262266
if (!jsInvoker_) {

‎packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class AnimationBackend : public UIManagerAnimationBackend {
5555
void synchronouslyUpdateProps(const std::unordered_map<Tag, AnimatedProps> &updates);
5656
void requestAsyncFlushForSurfaces(const std::set<SurfaceId> &surfaces);
5757
void clearRegistry(SurfaceId surfaceId) override;
58+
void clearRegistryOnSurfaceStop(SurfaceId surfaceId) override;
5859
void registerJSInvoker(std::shared_ptr<CallInvoker> jsInvoker) override;
5960

6061
void onAnimationFrame(AnimationTimestamp timestamp) override;

‎packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,13 @@ void UIManager::setSurfaceProps(
272272
ShadowTree::Unique UIManager::stopSurface(SurfaceId surfaceId) const {
273273
TraceSection s("UIManager::stopSurface");
274274

275-
// Stop any ongoing animations.
275+
// Stop any ongoing layout animations.
276276
stopSurfaceForAnimationDelegate(surfaceId);
277277

278+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
279+
animationBackend_->clearRegistryOnSurfaceStop(surfaceId);
280+
}
281+
278282
// Waiting for all concurrent commits to be finished and unregistering the
279283
// `ShadowTree`.
280284
auto shadowTree = getShadowTreeRegistry().remove(surfaceId);

‎packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerAnimationBackend.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class UIManagerAnimationBackend {
3030
virtual CallbackId start(const Callback &callback) = 0;
3131
virtual void stop(CallbackId callbackId) = 0;
3232
virtual void clearRegistry(SurfaceId surfaceId) = 0;
33+
virtual void clearRegistryOnSurfaceStop(SurfaceId surfaceId) = 0;
3334
virtual void trigger() = 0;
3435
virtual void pushAnimationMutations(const Callback &callback) = 0;
3536
virtual void registerJSInvoker(std::shared_ptr<CallInvoker> jsInvoker) = 0;

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1536,6 +1536,7 @@ class facebook::react::AnimatedNode {
15361536
class facebook::react::AnimatedPropsRegistry {
15371537
public std::pair<std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>>&, facebook::react::SnapshotMap&> getMap(facebook::react::SurfaceId surfaceId);
15381538
public void clear(facebook::react::SurfaceId surfaceId);
1539+
public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
15391540
public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
15401541
}
15411542

@@ -1545,6 +1546,7 @@ class facebook::react::AnimationBackend : public facebook::react::UIManagerAnima
15451546
public using ResumeCallback = std::function<void()>;
15461547
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
15471548
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) override;
1549+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) override;
15481550
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) override;
15491551
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
15501552
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) override;
@@ -5219,6 +5221,7 @@ class facebook::react::UIManagerAnimationBackend {
52195221
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
52205222
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52215223
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
5224+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
52225225
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
52235226
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52245227
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -13503,4 +13506,4 @@ struct std::hash<facebook::react::jsinspector_modern::UniqueMonostate<key>> {
1350313506
}
1350413507

1350513508

13506-
void YGJNIVanilla::registerNatives(JNIEnv* env);
13509+
void YGJNIVanilla::registerNatives(JNIEnv* env);

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1535,6 +1535,7 @@ class facebook::react::AnimatedNode {
15351535
class facebook::react::AnimatedPropsRegistry {
15361536
public std::pair<std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>>&, facebook::react::SnapshotMap&> getMap(facebook::react::SurfaceId surfaceId);
15371537
public void clear(facebook::react::SurfaceId surfaceId);
1538+
public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
15381539
public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
15391540
}
15401541

@@ -1544,6 +1545,7 @@ class facebook::react::AnimationBackend : public facebook::react::UIManagerAnima
15441545
public using ResumeCallback = std::function<void()>;
15451546
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
15461547
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) override;
1548+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) override;
15471549
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) override;
15481550
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
15491551
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) override;
@@ -5210,6 +5212,7 @@ class facebook::react::UIManagerAnimationBackend {
52105212
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
52115213
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52125214
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
5215+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
52135216
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
52145217
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52155218
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -13359,4 +13362,4 @@ struct std::hash<facebook::react::jsinspector_modern::UniqueMonostate<key>> {
1335913362
}
1336013363

1336113364

13362-
void YGJNIVanilla::registerNatives(JNIEnv* env);
13365+
void YGJNIVanilla::registerNatives(JNIEnv* env);

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4481,6 +4481,7 @@ class facebook::react::AnimatedNode {
44814481
class facebook::react::AnimatedPropsRegistry {
44824482
public std::pair<std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>>&, facebook::react::SnapshotMap&> getMap(facebook::react::SurfaceId surfaceId);
44834483
public void clear(facebook::react::SurfaceId surfaceId);
4484+
public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
44844485
public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
44854486
}
44864487

@@ -4490,6 +4491,7 @@ class facebook::react::AnimationBackend : public facebook::react::UIManagerAnima
44904491
public using ResumeCallback = std::function<void()>;
44914492
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
44924493
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) override;
4494+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) override;
44934495
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) override;
44944496
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
44954497
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) override;
@@ -7788,6 +7790,7 @@ class facebook::react::UIManagerAnimationBackend {
77887790
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
77897791
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77907792
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
7793+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
77917794
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
77927795
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77937796
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -16099,4 +16102,4 @@ class FB::LazyVector {
1609916102
public using reference = T;
1610016103
public using size_type = std::int32_t;
1610116104
public using value_type = T;
16102-
}
16105+
}

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4480,6 +4480,7 @@ class facebook::react::AnimatedNode {
44804480
class facebook::react::AnimatedPropsRegistry {
44814481
public std::pair<std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>>&, facebook::react::SnapshotMap&> getMap(facebook::react::SurfaceId surfaceId);
44824482
public void clear(facebook::react::SurfaceId surfaceId);
4483+
public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
44834484
public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
44844485
}
44854486

@@ -4489,6 +4490,7 @@ class facebook::react::AnimationBackend : public facebook::react::UIManagerAnima
44894490
public using ResumeCallback = std::function<void()>;
44904491
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
44914492
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) override;
4493+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) override;
44924494
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) override;
44934495
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) override;
44944496
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) override;
@@ -7779,6 +7781,7 @@ class facebook::react::UIManagerAnimationBackend {
77797781
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
77807782
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77817783
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
7784+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
77827785
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
77837786
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77847787
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -15965,4 +15968,4 @@ class FB::LazyVector {
1596515968
public using reference = T;
1596615969
public using size_type = std::int32_t;
1596715970
public using value_type = T;
15968-
}
15971+
}

0 commit comments

Comments
 (0)