Skip to content

Commit

Permalink
Add unmount method in UIManagerMountHook and use it to notify not int…
Browse files Browse the repository at this point in the history
…ersecting state in IntersectionObserver (#45186)

Summary:
Pull Request resolved: #45186

Changelog: [internal]

(this is an internal change because `IntersectionObserver` hasn't been released yet).

When testing IntersectionObserver, I realized that it wasn't triggering notifications for elements not intersecting when the surface that contained them was completely deallocated.

This is unexpected because IntersectionObserver notifications are delivered when the element is removed from the root, but not when the root itself is removed.

This fixes that behavior by:
1. Adding a method in `UIManagerMountHooks` to get a notification about the surface being unmounted. This is necessary to keep the API backwards compatible.
2. Using that method in `IntersectionObserverManager` to notify all observers (and report a change if necessary).

Differential Revision: D59061136
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jun 26, 2024
1 parent ebf1a7b commit 49ffbec
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static Rect computeIntersection(
std::optional<IntersectionObserverEntry>
IntersectionObserver::updateIntersectionObservation(
const RootShadowNode& rootShadowNode,
double mountTime) {
double time) {
const auto layoutableRootShadowNode =
dynamic_cast<const LayoutableShadowNode*>(&rootShadowNode);

Expand Down Expand Up @@ -121,22 +121,26 @@ IntersectionObserver::updateIntersectionObservation(
: intersectionRectArea / targetBoundingRectArea;

if (intersectionRatio == 0) {
return setNotIntersectingState(
rootBoundingRect, targetBoundingRect, mountTime);
return setNotIntersectingState(rootBoundingRect, targetBoundingRect, time);
}

auto highestThresholdCrossed = getHighestThresholdCrossed(intersectionRatio);
if (highestThresholdCrossed == -1) {
return setNotIntersectingState(
rootBoundingRect, targetBoundingRect, mountTime);
return setNotIntersectingState(rootBoundingRect, targetBoundingRect, time);
}

return setIntersectingState(
rootBoundingRect,
targetBoundingRect,
intersectionRect,
highestThresholdCrossed,
mountTime);
time);
}

std::optional<IntersectionObserverEntry>
IntersectionObserver::updateIntersectionObservationForSurfaceUnmount(
double time) {
return setNotIntersectingState(Rect{}, Rect{}, time);
}

Float IntersectionObserver::getHighestThresholdCrossed(
Expand All @@ -156,7 +160,7 @@ IntersectionObserver::setIntersectingState(
const Rect& targetBoundingRect,
const Rect& intersectionRect,
Float threshold,
double mountTime) {
double time) {
auto newState = IntersectionObserverState::Intersecting(threshold);

if (state_ != newState) {
Expand All @@ -168,7 +172,7 @@ IntersectionObserver::setIntersectingState(
rootBoundingRect,
intersectionRect,
true,
mountTime,
time,
};
return std::optional<IntersectionObserverEntry>{std::move(entry)};
}
Expand All @@ -180,7 +184,7 @@ std::optional<IntersectionObserverEntry>
IntersectionObserver::setNotIntersectingState(
const Rect& rootBoundingRect,
const Rect& targetBoundingRect,
double mountTime) {
double time) {
if (state_ != IntersectionObserverState::NotIntersecting()) {
state_ = IntersectionObserverState::NotIntersecting();
IntersectionObserverEntry entry{
Expand All @@ -190,7 +194,7 @@ IntersectionObserver::setNotIntersectingState(
rootBoundingRect,
std::nullopt,
false,
mountTime,
time,
};
return std::optional<IntersectionObserverEntry>(std::move(entry));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ class IntersectionObserver {
// https://w3c.github.io/IntersectionObserver/#update-intersection-observations-algo
std::optional<IntersectionObserverEntry> updateIntersectionObservation(
const RootShadowNode& rootShadowNode,
double mountTime);
double time);

std::optional<IntersectionObserverEntry>
updateIntersectionObservationForSurfaceUnmount(double time);

IntersectionObserverObserverId getIntersectionObserverId() const {
return intersectionObserverId_;
Expand All @@ -63,12 +66,12 @@ class IntersectionObserver {
const Rect& targetBoundingRect,
const Rect& intersectionRect,
Float threshold,
double mountTime);
double time);

std::optional<IntersectionObserverEntry> setNotIntersectingState(
const Rect& rootBoundingRect,
const Rect& targetBoundingRect,
double mountTime);
double time);

IntersectionObserverObserverId intersectionObserverId_;
ShadowNode::Shared targetShadowNode_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,15 +160,27 @@ IntersectionObserverManager::takeRecords() {
return entries;
}

#pragma mark - UIManagerMountHook

void IntersectionObserverManager::shadowTreeDidMount(
const RootShadowNode::Shared& rootShadowNode,
double mountTime) noexcept {
updateIntersectionObservations(*rootShadowNode, mountTime);
double time) noexcept {
updateIntersectionObservations(
rootShadowNode->getSurfaceId(), rootShadowNode, time);
}

void IntersectionObserverManager::shadowTreeDidUnmount(
SurfaceId surfaceId,
double time) noexcept {
updateIntersectionObservations(surfaceId, nullptr, time);
}

#pragma mark - Private methods

void IntersectionObserverManager::updateIntersectionObservations(
const RootShadowNode& rootShadowNode,
double mountTime) {
SurfaceId surfaceId,
const RootShadowNode::Shared& rootShadowNode,
double time) {
SystraceSection s(
"IntersectionObserverManager::updateIntersectionObservations");

Expand All @@ -178,17 +190,21 @@ void IntersectionObserverManager::updateIntersectionObservations(
{
std::shared_lock lock(observersMutex_);

auto surfaceId = rootShadowNode.getSurfaceId();

auto observersIt = observersBySurfaceId_.find(surfaceId);
if (observersIt == observersBySurfaceId_.end()) {
return;
}

auto& observers = observersIt->second;
for (auto& observer : observers) {
auto entry =
observer.updateIntersectionObservation(rootShadowNode, mountTime);
std::optional<IntersectionObserverEntry> entry;

if (rootShadowNode != nullptr) {
entry = observer.updateIntersectionObservation(*rootShadowNode, time);
} else {
entry = observer.updateIntersectionObservationForSurfaceUnmount(time);
}

if (entry) {
entries.push_back(std::move(entry).value());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ class IntersectionObserverManager final : public UIManagerMountHook {

void shadowTreeDidMount(
const RootShadowNode::Shared& rootShadowNode,
double mountTime) noexcept override;
double time) noexcept override;

void shadowTreeDidUnmount(SurfaceId surfaceId, double time) noexcept override;

private:
mutable std::unordered_map<SurfaceId, std::vector<IntersectionObserver>>
Expand All @@ -63,8 +65,9 @@ class IntersectionObserverManager final : public UIManagerMountHook {
// Equivalent to
// https://w3c.github.io/IntersectionObserver/#update-intersection-observations-algo
void updateIntersectionObservations(
const RootShadowNode& rootShadowNode,
double mountTime);
SurfaceId surfaceId,
const RootShadowNode::Shared& rootShadowNode,
double time);

const IntersectionObserver& getRegisteredIntersectionObserver(
SurfaceId surfaceId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,15 +636,15 @@ void UIManager::reportMount(SurfaceId surfaceId) const {
shadowTree.getMountingCoordinator()->getBaseRevision().rootShadowNode;
});

if (!rootShadowNode) {
return;
}

{
std::shared_lock lock(mountHookMutex_);

for (auto* mountHook : mountHooks_) {
mountHook->shadowTreeDidMount(rootShadowNode, time);
if (rootShadowNode) {
mountHook->shadowTreeDidMount(rootShadowNode, time);
} else {
mountHook->shadowTreeDidUnmount(surfaceId, time);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ class UIManagerMountHook {
const RootShadowNode::Shared& rootShadowNode,
double mountTime) noexcept = 0;

virtual void shadowTreeDidUnmount(
SurfaceId /*surfaceId*/,
double /*unmountTime*/) noexcept {
// Default no-op implementation for backwards compatibility.
}

virtual ~UIManagerMountHook() noexcept = default;
};

Expand Down

0 comments on commit 49ffbec

Please sign in to comment.