From 877a7e83993143a0f76b8ca8137b847e3f648f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Tue, 14 Nov 2023 03:35:59 -0800 Subject: [PATCH] Remove pending records when unobserving a target in IntersectionObserver (#41450) Summary: `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 --- .../__mocks__/NativeIntersectionObserver.js | 10 ++++ .../IntersectionObserverManager.cpp | 58 ++++++++++++------- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js b/packages/react-native/Libraries/IntersectionObserver/__mocks__/NativeIntersectionObserver.js index e66a5e12159687..cb3d7fd9d87a0b 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 2c992c64cb1fc2..a3c9eecf4c0ea4 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; - - 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); + { + std::unique_lock lock(pendingEntriesMutex_); + + 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()); } }