Skip to content

Commit 1433e43

Browse files
zeyapmeta-codesync[bot]
zeyap
authored and
meta-codesync[bot]
committed
Clear AnimatedPropsRegistry on surface stop (facebook#56485)
Summary: Pull Request resolved: facebook#56485 ## 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 fbshipit-source-id: c7668f728050a0d3f5c09ad5ca51e48450c1b87e
1 parent 46c0177 commit 1433e43

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;
@@ -5226,6 +5228,7 @@ class facebook::react::UIManagerAnimationBackend {
52265228
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
52275229
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52285230
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
5231+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
52295232
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
52305233
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52315234
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -13650,4 +13653,4 @@ struct std::hash<facebook::react::jsinspector_modern::UniqueMonostate<key>> {
1365013653
}
1365113654

1365213655

13653-
void YGJNIVanilla::registerNatives(JNIEnv* env);
13656+
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;
@@ -5217,6 +5219,7 @@ class facebook::react::UIManagerAnimationBackend {
52175219
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
52185220
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52195221
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
5222+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
52205223
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
52215224
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
52225225
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -13506,4 +13509,4 @@ struct std::hash<facebook::react::jsinspector_modern::UniqueMonostate<key>> {
1350613509
}
1350713510

1350813511

13509-
void YGJNIVanilla::registerNatives(JNIEnv* env);
13512+
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;
@@ -7791,6 +7793,7 @@ class facebook::react::UIManagerAnimationBackend {
77917793
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
77927794
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77937795
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
7796+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
77947797
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
77957798
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77967799
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -16242,4 +16245,4 @@ class FB::LazyVector {
1624216245
public using reference = T;
1624316246
public using size_type = std::int32_t;
1624416247
public using value_type = T;
16245-
}
16248+
}

‎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;
@@ -7782,6 +7784,7 @@ class facebook::react::UIManagerAnimationBackend {
77827784
public using Callback = std::function<facebook::react::AnimationMutations(facebook::react::AnimationTimestamp)>;
77837785
public virtual facebook::react::CallbackId start(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77847786
public virtual void clearRegistry(facebook::react::SurfaceId surfaceId) = 0;
7787+
public virtual void clearRegistryOnSurfaceStop(facebook::react::SurfaceId surfaceId) = 0;
77857788
public virtual void onAnimationFrame(facebook::react::AnimationTimestamp timestamp) = 0;
77867789
public virtual void pushAnimationMutations(const facebook::react::UIManagerAnimationBackend::Callback& callback) = 0;
77877790
public virtual void registerJSInvoker(std::shared_ptr<facebook::react::CallInvoker> jsInvoker) = 0;
@@ -16108,4 +16111,4 @@ class FB::LazyVector {
1610816111
public using reference = T;
1610916112
public using size_type = std::int32_t;
1611016113
public using value_type = T;
16111-
}
16114+
}

0 commit comments

Comments
 (0)