mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
IntersectionObserver: Switch from ShadowNode to ShadowNodeFamily (#51089)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51089 Currently, `IntersectionObserver#observe` retains a reference to the `ShadowNode` of the view that is supplied as an argument, which is used to compute intersection whenever a shadow tree is committed. However, the `shadowNode` includes all of child nodes and state at the time that `IntersectionObserver#observe` is called. This means that an active `IntersectionObserverEntry` will retain references to memory that would otherwise be deallocated (e.g. if children of the observed view are unmounted after `IntersectionObserver#observe` is called). This diff refactors `IntersectionObserver` to instead retain a reference to the `ShadowNodeFamily`, which does not retain references to child nodes and still eanbles `IntersectionObserver` to compute intersections. Changelog: [General][Changed] - Fixed `IntersectionObserver#observe` to avoid retaining memory for unmounted child nodes of observed views. Reviewed By: RSNara Differential Revision: D74130479 fbshipit-source-id: 6b581f73f06b93e6463d5004c181c39aae13896a
This commit is contained in:
committed by
Facebook GitHub Bot
parent
d94f4d8c9d
commit
d945c5863a
+1
-1
@@ -101,7 +101,7 @@ NativeIntersectionObserver::convertToNativeModuleEntry(
|
||||
|
||||
NativeIntersectionObserverEntry nativeModuleEntry = {
|
||||
entry.intersectionObserverId,
|
||||
(*entry.shadowNode).getInstanceHandle(runtime),
|
||||
(*entry.shadowNodeFamily).getInstanceHandle(runtime),
|
||||
targetRect,
|
||||
rootRect,
|
||||
intersectionRect,
|
||||
|
||||
@@ -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<ShadowNode::Unshared(const ShadowNode& oldShadowNode)>&
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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_;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
+5
-6
@@ -16,11 +16,11 @@ namespace facebook::react {
|
||||
|
||||
IntersectionObserver::IntersectionObserver(
|
||||
IntersectionObserverObserverId intersectionObserverId,
|
||||
ShadowNode::Shared targetShadowNode,
|
||||
ShadowNodeFamily::Shared targetShadowNodeFamily,
|
||||
std::vector<Float> thresholds,
|
||||
std::optional<std::vector<Float>> 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,
|
||||
|
||||
+14
-6
@@ -8,7 +8,7 @@
|
||||
#pragma once
|
||||
|
||||
#include <react/renderer/components/root/RootShadowNode.h>
|
||||
#include <react/renderer/core/ShadowNode.h>
|
||||
#include <react/renderer/core/ShadowNodeFamily.h>
|
||||
#include <react/renderer/graphics/Float.h>
|
||||
#include <react/renderer/graphics/Rect.h>
|
||||
#include <memory>
|
||||
@@ -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<Float> thresholds,
|
||||
std::optional<std::vector<Float>> 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<Float> getThresholds() const {
|
||||
@@ -75,7 +83,7 @@ class IntersectionObserver {
|
||||
double time);
|
||||
|
||||
IntersectionObserverObserverId intersectionObserverId_;
|
||||
ShadowNode::Shared targetShadowNode_;
|
||||
ShadowNodeFamily::Shared targetShadowNodeFamily_;
|
||||
std::vector<Float> thresholds_;
|
||||
std::optional<std::vector<Float>> rootThresholds_;
|
||||
mutable IntersectionObserverState state_ =
|
||||
|
||||
+8
-6
@@ -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());
|
||||
}
|
||||
|
||||
+44
@@ -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<typeof View>,
|
||||
> = instance => {
|
||||
const element = ensureReactNativeElement(instance);
|
||||
observer.observe(element);
|
||||
return () => {
|
||||
observer.unobserve(element);
|
||||
};
|
||||
};
|
||||
|
||||
function Observe({children}: $ReadOnly<{children?: React.Node}>) {
|
||||
return <View ref={observeRef}>{children}</View>;
|
||||
}
|
||||
|
||||
Fantom.runTask(() => {
|
||||
root.render(
|
||||
<Observe>
|
||||
<View ref={ref} />
|
||||
</Observe>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(getReferenceCount()).toBeGreaterThan(0);
|
||||
|
||||
Fantom.runTask(() => {
|
||||
root.render(<Observe />);
|
||||
});
|
||||
|
||||
// TODO (T223254666): Delete this and figure out why test fails.
|
||||
Fantom.runTask(() => {
|
||||
root.render(<Observe />);
|
||||
});
|
||||
|
||||
expect(getReferenceCount()).toBe(0);
|
||||
});
|
||||
|
||||
describe('rootThreshold', () => {
|
||||
it('should report partial intersecting initial state correctly', () => {
|
||||
const nodeRef = React.createRef<HostInstance>();
|
||||
|
||||
Reference in New Issue
Block a user