From a493ea43ff8cfa9d19245c5fb04faea73590411b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 14 Nov 2023 06:11:13 -0800 Subject: [PATCH] Remove pending records when unobserving a target in IntersectionObserver (#41450) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/41450 `IntersectionObserver` shouldn't report entries for targets that are no longer being observed by the observer. This wasn't the case before because it was possible to create an intersection observer entry, then unobserve the target and then dispatch the pending entries (including the unobserved target). This fixes that issue to align with Web browsers. Changelog: [internal] Reviewed By: rshest Differential Revision: D51256827 fbshipit-source-id: 28035f00bcb05a8ca53140719019032b3399436c --- .../__mocks__/NativeIntersectionObserver.js | 10 ++++ .../IntersectionObserverManager.cpp | 56 ++++++++++++------- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js b/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js index e66a5e12159..cb3d7fd9d87 100644 --- a/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js +++ b/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js @@ -96,6 +96,16 @@ const NativeIntersectionObserverMock = { 'unexpected duplicate call to unobserve', ); observations.splice(observationIndex, 1); + + pendingRecords = pendingRecords.filter( + record => + record.intersectionObserverId !== intersectionObserverId || + record.targetInstanceHandle !== + FabricUIManagerMock.__getInstanceHandleFromNode( + // $FlowExpectedError[incompatible-call] + targetShadowNode, + ), + ); }, connect: (notifyIntersectionObserversCallback: () => void): void => { invariant(callback == null, 'unexpected call to connect'); diff --git a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp index 2c992c64cb1..a3c9eecf4c0 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp @@ -70,31 +70,47 @@ void IntersectionObserverManager::unobserve( const ShadowNode& shadowNode) { SystraceSection s("IntersectionObserverManager::unobserve"); - std::unique_lock lock(observersMutex_); + { + std::unique_lock lock(observersMutex_); - auto surfaceId = shadowNode.getSurfaceId(); + auto surfaceId = shadowNode.getSurfaceId(); - auto observersIt = observersBySurfaceId_.find(surfaceId); - if (observersIt == observersBySurfaceId_.end()) { - return; + auto observersIt = observersBySurfaceId_.find(surfaceId); + if (observersIt == observersBySurfaceId_.end()) { + return; + } + + auto& observers = observersIt->second; + + observers.erase( + std::remove_if( + observers.begin(), + observers.end(), + [intersectionObserverId, &shadowNode](const auto& observer) { + return observer.getIntersectionObserverId() == + intersectionObserverId && + ShadowNode::sameFamily( + observer.getTargetShadowNode(), shadowNode); + }), + observers.end()); + + if (observers.empty()) { + observersBySurfaceId_.erase(surfaceId); + } } - auto& observers = observersIt->second; + { + std::unique_lock lock(pendingEntriesMutex_); - observers.erase( - std::remove_if( - observers.begin(), - observers.end(), - [intersectionObserverId, &shadowNode](const auto& observer) { - return observer.getIntersectionObserverId() == - intersectionObserverId && - ShadowNode::sameFamily( - observer.getTargetShadowNode(), shadowNode); - }), - observers.end()); - - if (observers.empty()) { - observersBySurfaceId_.erase(surfaceId); + pendingEntries_.erase( + std::remove_if( + pendingEntries_.begin(), + pendingEntries_.end(), + [intersectionObserverId, &shadowNode](const auto& entry) { + return entry.intersectionObserverId == intersectionObserverId && + ShadowNode::sameFamily(*entry.shadowNode, shadowNode); + }), + pendingEntries_.end()); } }