Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[ResizeObserver] The initial last reported size of ResizeObservation …
…should be -1x-1 which always notify observed elements

https://bugs.webkit.org/show_bug.cgi?id=251015

Reviewed by Oriol Brufau and Ryosuke Niwa.

This patch initializes the lastReportedSize of ResizeObservation to -1 x -1, which is the conclusion of [1],
it indicates that -1 x -1 is the initial size when first observing an element with ResizeObserver.

The previous change makes every ResizeObservation be delivered at least once, even if the target is a disconnected element.
When GC happens between observe and gather ResizeObservations, JSC::Weak<JSC::JSObject> m_callback inside JSCallbackDataWeak
could be released, and the element and JSResizeObserver might not be released, for they are not strong connected, this scenario
would trigger the ASSERT in ResizeObserver::deliverObservations. To fix it, this patch adds m_targetsWaitingForFirstObservation
to extend the life cycle of ResizeObserver to the first time of deliverObservations. The fix is similar to
what IntersectionObserver does in bug 231235.

The test6 of resize-observer/notify.html still fails in mac-wk1, because updaterendering is not triggered properly.
Filed [2] to track it.

[1] w3c/csswg-drafts#3664
[2] https://bugs.webkit.org/show_bug.cgi?id=250900

* LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/observe-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt: Copied from LayoutTests/imported/w3c/web-platform-tests/resize-observer/notify-expected.txt.
* LayoutTests/platform/mac/imported/w3c/web-platform-tests/resize-observer/svg-expected.txt:
* Source/WebCore/page/ResizeObservation.cpp:
* Source/WebCore/page/ResizeObserver.cpp:
(WebCore::ResizeObserver::observe):
(WebCore::ResizeObserver::deliverObservations):
(WebCore::ResizeObserver::isReachableFromOpaqueRoots const):
(WebCore::ResizeObserver::removeAllTargets):
(WebCore::ResizeObserver::removeObservation):
* Source/WebCore/page/ResizeObserver.h:

Canonical link: https://commits.webkit.org/259673@main
  • Loading branch information
cathiechen committed Feb 1, 2023
1 parent ad178f9 commit 2861d8c
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 9 deletions.
Expand Up @@ -2,17 +2,22 @@ ResizeObserver tests

t1
t2
t3 inline

Harness Error (TIMEOUT), message = null
t3
inline

PASS ResizeObserver implemented
NOTRUN guard
PASS guard
PASS test0: notification ordering
PASS test1: display:none triggers notification
PASS test2: remove/appendChild trigger notification
PASS test3: dimensions match
PASS test4: transform do not cause notifications
PASS test5: moving an element does not trigger notifications
FAIL test6: inline element notifies once with 0x0. assert_unreached: Timed out waiting for notification. (1000ms) Reached unreachable code
PASS test6: inline element notifies once with 0x0.
PASS test7: unobserve inside notify callback
PASS test8: observe inside notify callback
PASS test9: disconnect inside notify callback
PASS test10: element notifies when parent removed
PASS test11: display:none element should be notified
PASS test12: element sized 0x0 should be notified

Expand Up @@ -19,7 +19,7 @@ PASS test12: no observation is fired after the change of writing mode when box's
PASS test13: an observation is fired after the change of writing mode when box's specified size comes from physical size properties.
PASS test14: observe the same target but using a different box should override the previous one
PASS test15: an observation is fired with box dimensions 0 when element's display property is set to inline
FAIL test16: observations fire once with 0x0 size for non-replaced inline elements assert_unreached: Timed out waiting for notification. (1000ms) Reached unreachable code
PASS test16: observations fire once with 0x0 size for non-replaced inline elements
PASS test17: Box sizing snd Resize Observer notifications
FAIL test18: an observation is fired when device-pixel-content-box is being observed assert_unreached: Caught a throw, possible syntax error Reached unreachable code

Expand Up @@ -19,5 +19,10 @@ PASS test7: observe svg:polyline
PASS test8: observe svg:rect
PASS test9: observe svg:text
PASS test10: observe svg:svg, top/left is 0 even with padding
FAIL test11: observe svg non-displayable element assert_unreached: Timed out waiting for notification. (1000ms) Reached unreachable code
PASS test11: observe svg non-displayable element
PASS test12: observe svg:rect content box
PASS test13: observe svg:rect border box
PASS test14: observe g:rect content and border box
PASS test15: observe svg:text content and border box
FAIL test16: observe g:rect content, border and device-pixel-content boxes assert_unreached: Caught a throw, possible syntax error Reached unreachable code

@@ -0,0 +1,23 @@
ResizeObserver tests

t1
t2
t3
inline

PASS ResizeObserver implemented
PASS guard
PASS test0: notification ordering
PASS test1: display:none triggers notification
PASS test2: remove/appendChild trigger notification
PASS test3: dimensions match
PASS test4: transform do not cause notifications
PASS test5: moving an element does not trigger notifications
FAIL test6: inline element notifies once with 0x0. assert_unreached: Timed out waiting for notification. (1000ms) Reached unreachable code
PASS test7: unobserve inside notify callback
PASS test8: observe inside notify callback
PASS test9: disconnect inside notify callback
PASS test10: element notifies when parent removed
PASS test11: display:none element should be notified
PASS test12: element sized 0x0 should be notified

Expand Up @@ -19,5 +19,10 @@ PASS test7: observe svg:polyline
PASS test8: observe svg:rect
PASS test9: observe svg:text
PASS test10: observe svg:svg, top/left is 0 even with padding
FAIL test11: observe svg non-displayable element assert_unreached: Timed out waiting for notification. (1000ms) Reached unreachable code
PASS test11: observe svg non-displayable element
PASS test12: observe svg:rect content box
PASS test13: observe svg:rect border box
PASS test14: observe g:rect content and border box
FAIL test15: observe svg:text content and border box assert_equals: expected 30 but got 30.015625
FAIL test16: observe g:rect content, border and device-pixel-content boxes assert_unreached: Caught a throw, possible syntax error Reached unreachable code

1 change: 1 addition & 0 deletions Source/WebCore/page/ResizeObservation.cpp
Expand Up @@ -41,6 +41,7 @@ Ref<ResizeObservation> ResizeObservation::create(Element& target, ResizeObserver

ResizeObservation::ResizeObservation(Element& element, ResizeObserverBoxOptions observedBox)
: m_target { element }
, m_lastObservationSizes { LayoutSize(-1, -1), LayoutSize(-1, -1), LayoutSize(-1, -1) }
, m_observedBox { observedBox }
{
}
Expand Down
13 changes: 12 additions & 1 deletion Source/WebCore/page/ResizeObserver.cpp
Expand Up @@ -82,6 +82,11 @@ void ResizeObserver::observe(Element& target, const ResizeObserverOptions& optio

m_observations.append(ResizeObservation::create(target, options.box));

// Per the specification, we should dispatch at least one observation for the target. For this reason, we make sure to keep the
// target alive until this first observation. This, in turn, will keep the ResizeObserver's JS wrapper alive via
// isReachableFromOpaqueRoots(), so the callback stays alive.
m_targetsWaitingForFirstObservation.append(target);

if (m_document) {
m_document->addResizeObserver(*this);
m_document->scheduleRenderingUpdate(RenderingUpdateStep::ResizeObservations);
Expand Down Expand Up @@ -141,6 +146,8 @@ void ResizeObserver::deliverObservations()
m_activeObservations.clear();
auto activeObservationTargets = std::exchange(m_activeObservationTargets, { });

auto targetsWaitingForFirstObservation = std::exchange(m_targetsWaitingForFirstObservation, { });

// FIXME: The JSResizeObserver wrapper should be kept alive as long as the resize observer can fire events.
ASSERT(m_callback->hasCallback());
if (!m_callback->hasCallback())
Expand All @@ -165,7 +172,7 @@ bool ResizeObserver::isReachableFromOpaqueRoots(JSC::AbstractSlotVisitor& visito
if (containsWebCoreOpaqueRoot(visitor, target.get()))
return true;
}
return false;
return !m_targetsWaitingForFirstObservation.isEmpty();
}

bool ResizeObserver::removeTarget(Element& target)
Expand All @@ -186,11 +193,15 @@ void ResizeObserver::removeAllTargets()
}
m_activeObservationTargets.clear();
m_activeObservations.clear();
m_targetsWaitingForFirstObservation.clear();
m_observations.clear();
}

bool ResizeObserver::removeObservation(const Element& target)
{
m_targetsWaitingForFirstObservation.removeFirstMatching([&target](auto& pendingTarget) {
return pendingTarget.ptr() == &target;
});
return m_observations.removeFirstMatching([&target](auto& observation) {
return observation->target() == &target;
});
Expand Down
2 changes: 2 additions & 0 deletions Source/WebCore/page/ResizeObserver.h
Expand Up @@ -84,6 +84,8 @@ class ResizeObserver : public RefCounted<ResizeObserver>, public CanMakeWeakPtr<

Vector<Ref<ResizeObservation>> m_activeObservations;
Vector<GCReachableRef<Element>> m_activeObservationTargets;
Vector<GCReachableRef<Element>> m_targetsWaitingForFirstObservation;

bool m_hasSkippedObservations { false };
};

Expand Down

0 comments on commit 2861d8c

Please sign in to comment.