diff --git a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp index 531d9ae416e..ab948bf017f 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/intersectionobserver/NativeIntersectionObserver.cpp @@ -101,7 +101,7 @@ NativeIntersectionObserver::convertToNativeModuleEntry( NativeIntersectionObserverEntry nativeModuleEntry = { entry.intersectionObserverId, - (*entry.shadowNode).getInstanceHandle(runtime), + (*entry.shadowNodeFamily).getInstanceHandle(runtime), targetRect, rootRect, intersectionRect, diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp index 3af382c1dd6..8bda5b733d0 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -191,12 +191,7 @@ const SharedEventEmitter& ShadowNode::getEventEmitter() const { } jsi::Value ShadowNode::getInstanceHandle(jsi::Runtime& runtime) const { - auto instanceHandle = family_->instanceHandle_; - if (instanceHandle == nullptr) { - return jsi::Value::null(); - } - - return instanceHandle->getInstanceHandle(runtime); + return family_->getInstanceHandle(runtime); } Tag ShadowNode::getTag() const { @@ -341,6 +336,10 @@ const ShadowNodeFamily& ShadowNode::getFamily() const { return *family_; } +ShadowNodeFamily::Shared ShadowNode::getFamilyShared() const { + return family_; +} + ShadowNode::Unshared ShadowNode::cloneTree( const ShadowNodeFamily& shadowNodeFamily, const std::function& diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h index ae648988875..63c21a99617 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNode.h @@ -159,6 +159,8 @@ class ShadowNode : public Sealable, const ShadowNodeFamily& getFamily() const; + ShadowNodeFamily::Shared getFamilyShared() const; + #pragma mark - Mutating Methods virtual void appendChild(const Shared& child); diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.cpp b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.cpp index 543ce2abe84..bf486802cca 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.cpp @@ -76,6 +76,14 @@ Tag ShadowNodeFamily::getTag() const { return tag_; } +jsi::Value ShadowNodeFamily::getInstanceHandle(jsi::Runtime& runtime) const { + if (instanceHandle_ == nullptr) { + return jsi::Value::null(); + } + + return instanceHandle_->getInstanceHandle(runtime); +} + InstanceHandle::Shared ShadowNodeFamily::getInstanceHandle() const { return instanceHandle_; } diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h index 203f895ff7c..e733ac57bc6 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h @@ -122,6 +122,7 @@ class ShadowNodeFamily final { */ Tag getTag() const; + jsi::Value getInstanceHandle(jsi::Runtime& runtime) const; InstanceHandle::Shared getInstanceHandle() const; void setInstanceHandle(InstanceHandle::Shared& instanceHandle) const; diff --git a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.cpp b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.cpp index 2589ee749ab..bc8d0c875a5 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.cpp +++ b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.cpp @@ -16,11 +16,11 @@ namespace facebook::react { IntersectionObserver::IntersectionObserver( IntersectionObserverObserverId intersectionObserverId, - ShadowNode::Shared targetShadowNode, + ShadowNodeFamily::Shared targetShadowNodeFamily, std::vector thresholds, std::optional> rootThresholds) : intersectionObserverId_(intersectionObserverId), - targetShadowNode_(std::move(targetShadowNode)), + targetShadowNodeFamily_(std::move(targetShadowNodeFamily)), thresholds_(std::move(thresholds)), rootThresholds_(std::move(rootThresholds)) {} @@ -112,8 +112,7 @@ IntersectionObserver::updateIntersectionObservation( layoutableRootShadowNode != nullptr && "RootShadowNode instances must always inherit from LayoutableShadowNode."); - auto targetAncestors = - targetShadowNode_->getFamily().getAncestors(rootShadowNode); + auto targetAncestors = targetShadowNodeFamily_->getAncestors(rootShadowNode); // Absolute coordinates of the root auto rootBoundingRect = getRootBoundingRect(*layoutableRootShadowNode); @@ -189,7 +188,7 @@ IntersectionObserver::setIntersectingState( state_ = newState; IntersectionObserverEntry entry{ intersectionObserverId_, - targetShadowNode_, + targetShadowNodeFamily_, targetBoundingRect, rootBoundingRect, intersectionRect, @@ -212,7 +211,7 @@ IntersectionObserver::setNotIntersectingState( state_ = IntersectionObserverState::NotIntersecting(); IntersectionObserverEntry entry{ intersectionObserverId_, - targetShadowNode_, + targetShadowNodeFamily_, targetBoundingRect, rootBoundingRect, intersectionRect, diff --git a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.h b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.h index ba6a4449b36..89b45d83b65 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.h +++ b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserver.h @@ -8,7 +8,7 @@ #pragma once #include -#include +#include #include #include #include @@ -20,7 +20,7 @@ using IntersectionObserverObserverId = int32_t; struct IntersectionObserverEntry { IntersectionObserverObserverId intersectionObserverId; - ShadowNode::Shared shadowNode; + ShadowNodeFamily::Shared shadowNodeFamily; Rect targetRect; Rect rootRect; Rect intersectionRect; @@ -28,13 +28,19 @@ struct IntersectionObserverEntry { // TODO(T156529385) Define `DOMHighResTimeStamp` as an alias for `double` and // use it here. double time; + + bool sameShadowNodeFamily( + const ShadowNodeFamily& otherShadowNodeFamily) const { + return std::addressof(*shadowNodeFamily) == + std::addressof(otherShadowNodeFamily); + } }; class IntersectionObserver { public: IntersectionObserver( IntersectionObserverObserverId intersectionObserverId, - ShadowNode::Shared targetShadowNode, + ShadowNodeFamily::Shared targetShadowNodeFamily, std::vector thresholds, std::optional> rootThresholds = std::nullopt); @@ -51,8 +57,10 @@ class IntersectionObserver { return intersectionObserverId_; } - const ShadowNode& getTargetShadowNode() const { - return *targetShadowNode_; + bool isTargetShadowNodeFamily( + const ShadowNodeFamily& shadowNodeFamily) const { + return std::addressof(*targetShadowNodeFamily_) == + std::addressof(shadowNodeFamily); } std::vector getThresholds() const { @@ -75,7 +83,7 @@ class IntersectionObserver { double time); IntersectionObserverObserverId intersectionObserverId_; - ShadowNode::Shared targetShadowNode_; + ShadowNodeFamily::Shared targetShadowNodeFamily_; std::vector thresholds_; std::optional> rootThresholds_; mutable IntersectionObserverState state_ = 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 7fa55a5a470..04058e668ca 100644 --- a/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/observers/intersection/IntersectionObserverManager.cpp @@ -24,6 +24,7 @@ void IntersectionObserverManager::observe( TraceSection s("IntersectionObserverManager::observe"); auto surfaceId = shadowNode->getSurfaceId(); + auto shadowNodeFamily = shadowNode->getFamilyShared(); // The actual observer lives in the array, so we need to create it there and // then get a reference. Otherwise we only update its state in a copy. @@ -36,7 +37,7 @@ void IntersectionObserverManager::observe( auto& observers = observersBySurfaceId_[surfaceId]; observers.emplace_back(IntersectionObserver{ intersectionObserverId, - shadowNode, + shadowNodeFamily, std::move(thresholds), std::move(rootThresholds)}); observer = &observers.back(); @@ -80,6 +81,8 @@ void IntersectionObserverManager::unobserve( const ShadowNode& shadowNode) { TraceSection s("IntersectionObserverManager::unobserve"); + auto& shadowNodeFamily = shadowNode.getFamily(); + { std::unique_lock lock(observersMutex_); @@ -96,11 +99,10 @@ void IntersectionObserverManager::unobserve( std::remove_if( observers.begin(), observers.end(), - [intersectionObserverId, &shadowNode](const auto& observer) { + [intersectionObserverId, &shadowNodeFamily](const auto& observer) { return observer.getIntersectionObserverId() == intersectionObserverId && - ShadowNode::sameFamily( - observer.getTargetShadowNode(), shadowNode); + observer.isTargetShadowNodeFamily(shadowNodeFamily); }), observers.end()); @@ -116,9 +118,9 @@ void IntersectionObserverManager::unobserve( std::remove_if( pendingEntries_.begin(), pendingEntries_.end(), - [intersectionObserverId, &shadowNode](const auto& entry) { + [intersectionObserverId, &shadowNodeFamily](const auto& entry) { return entry.intersectionObserverId == intersectionObserverId && - ShadowNode::sameFamily(*entry.shadowNode, shadowNode); + entry.sameShadowNodeFamily(shadowNodeFamily); }), pendingEntries_.end()); } diff --git a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js index 742ea105baf..c6ddb1d712e 100644 --- a/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js +++ b/packages/react-native/src/private/webapis/intersectionobserver/__tests__/IntersectionObserver-itest.js @@ -15,6 +15,7 @@ import type {HostInstance} from 'react-native'; import type IntersectionObserverType from 'react-native/src/private/webapis/intersectionobserver/IntersectionObserver'; import ensureInstance from '../../../__tests__/utilities/ensureInstance'; +import {createShadowNodeReferenceCountingRef} from '../../../__tests__/utilities/ShadowNodeReferenceCounter'; import * as Fantom from '@react-native/fantom'; import * as React from 'react'; import {ScrollView, View} from 'react-native'; @@ -843,6 +844,49 @@ describe('IntersectionObserver', () => { }); }); + // TODO (T223234714): Fix memory leak and enable this test. + it.skip('should not retain initial children of observed targets', () => { + const root = Fantom.createRoot(); + observer = new IntersectionObserver(() => {}); + + const [getReferenceCount, ref] = createShadowNodeReferenceCountingRef(); + + const observeRef: React.RefSetter< + React.ElementRef, + > = instance => { + const element = ensureReactNativeElement(instance); + observer.observe(element); + return () => { + observer.unobserve(element); + }; + }; + + function Observe({children}: $ReadOnly<{children?: React.Node}>) { + return {children}; + } + + Fantom.runTask(() => { + root.render( + + + , + ); + }); + + expect(getReferenceCount()).toBeGreaterThan(0); + + Fantom.runTask(() => { + root.render(); + }); + + // TODO (T223254666): Delete this and figure out why test fails. + Fantom.runTask(() => { + root.render(); + }); + + expect(getReferenceCount()).toBe(0); + }); + describe('rootThreshold', () => { it('should report partial intersecting initial state correctly', () => { const nodeRef = React.createRef();