Remove pending records when unobserving a target in IntersectionObser…

…ver (#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
facebook · Nov 14, 2023 · b9eb6f6 · b9eb6f6
1 parent 004e290
commit b9eb6f6
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 21 deletions.
diff --git 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/...-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp b/...-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());
   }
 }