- Revision
- 288234
- Author
- [email protected]
- Date
- 2022-01-19 12:43:30 -0800 (Wed, 19 Jan 2022)
Log Message
Merge r283590 - ASSERT(m_callback->hasCallback()) under IntersectionObserver::notify()
https://bugs.webkit.org/show_bug.cgi?id=231235
<rdar://80837616>
Reviewed by Ryosuke Niwa.
Source/WebCore:
IntersectionObserver's JS callback stays alive as long as its JS wrapper and
its JS wrapper's lifetime relies on the IntersectionObserver::isReachableFromOpaqueRoots()
implementation. isReachableFromOpaqueRoots() keeps the wrapper alive as long
as the JS wrappers of observation / pending targets are alive. However, as per specification,
we always need to dispatch an observation for an observation target, even if that target
is not connected. Our code was already taking care of dispatching such observation. However,
there was nothing keeping the observation target alive in this case and thus nothing keeping
the JS callback alive either.
To address the issue, I am introducing a new m_targetsWaitingForFirstObservation data member
which holds a strong ref to the observation target until the next time we call notify().
This makes sure that the observation target (and its JS wrapper) stays alive long enough for
us to dispatch the first observation for it. I also updated isReachableFromOpaqueRoots() to
return true as long as m_targetsWaitingForFirstObservation is non-empty so that the
IntersectionObserver's JS wrapper (and thus the JS callback) stay alive long enough too.
Tests: intersection-observer/observe-disconnected-target-crash.html
intersection-observer/observe-disconnected-target.html
* page/IntersectionObserver.cpp:
(WebCore::IntersectionObserver::observe):
(WebCore::IntersectionObserver::unobserve):
(WebCore::IntersectionObserver::removeAllTargets):
(WebCore::IntersectionObserver::notify):
(WebCore::IntersectionObserver::isReachableFromOpaqueRoots const):
* page/IntersectionObserver.h:
LayoutTests:
Add layout test coverage both for the crash and the Web facing behavior.
* intersection-observer/observe-disconnected-target-crash-expected.txt: Added.
* intersection-observer/observe-disconnected-target-crash.html: Added.
* intersection-observer/observe-disconnected-target-expected.txt: Added.
* intersection-observer/observe-disconnected-target.html: Added.
Modified Paths
Added Paths
Diff
Modified: releases/WebKitGTK/webkit-2.34/LayoutTests/ChangeLog (288233 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/ChangeLog 2022-01-19 20:42:12 UTC (rev 288233)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/ChangeLog 2022-01-19 20:43:30 UTC (rev 288234)
@@ -1,3 +1,18 @@
+2021-10-05 Chris Dumez <[email protected]>
+
+ ASSERT(m_callback->hasCallback()) under IntersectionObserver::notify()
+ https://bugs.webkit.org/show_bug.cgi?id=231235
+ <rdar://80837616>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add layout test coverage both for the crash and the Web facing behavior.
+
+ * intersection-observer/observe-disconnected-target-crash-expected.txt: Added.
+ * intersection-observer/observe-disconnected-target-crash.html: Added.
+ * intersection-observer/observe-disconnected-target-expected.txt: Added.
+ * intersection-observer/observe-disconnected-target.html: Added.
+
2021-09-29 Chris Dumez <[email protected]>
[ iOS Debug ] http/tests/xmlhttprequest/access-control-preflight-credential-sync.html is a flaky crash
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash-expected.txt (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash-expected.txt 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1 @@
+This test passes if it doesn't crash.
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash.html (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash.html (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-crash.html 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1,20 @@
+<p>This test passes if it doesn't crash.</p>
+<script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+ _onload_ = async () => {
+ for (let i = 0; i < 8; i++) {
+ document.createElement('div');
+ }
+ for (let i = 0; i < 8; i++) {
+ new IntersectionObserver(() => {});
+ }
+ let intersectionObserver = new IntersectionObserver(() => {});
+ await intersectionObserver.observe(document.createElement('div'));
+ new ImageData(1000, 10000);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ };
+</script>
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-expected.txt (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target-expected.txt 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1,15 @@
+Tests that an observation is received when calling IntersectionObserver.observe() with a disconnected target.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS receivedObservations is false
+PASS observations.length is 1
+PASS observations[0].target.tagName is "DIV"
+PASS observations[0].isIntersecting is false
+PASS observations[0].intersectionRatio is 0.0
+PASS observations[0].target.foo is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target.html (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target.html (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-disconnected-target.html 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+ description("Tests that an observation is received when calling IntersectionObserver.observe() with a disconnected target.");
+ jsTestIsAsync = true;
+
+ let receivedObservations = false;
+ _onload_ = () => {
+ let intersectionObserver = new IntersectionObserver((_observations) => {
+ gc();
+ observations = _observations;
+
+ shouldBeFalse("receivedObservations");
+ receivedObservations = true;
+
+ shouldBe("observations.length", "1");
+ shouldBeEqualToString("observations[0].target.tagName", "DIV");
+ shouldBeFalse("observations[0].isIntersecting");
+ shouldBe("observations[0].intersectionRatio", "0.0");
+ shouldBe("observations[0].target.foo", "1");
+ setTimeout(finishJSTest, 100);
+ });
+ let div = document.createElement('div');
+ div.foo = 1;
+ intersectionObserver.observe(div);
+ div = null;
+ gc();
+ setTimeout(gc, 0);
+ };
+</script>
+</body>
+</html>
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target-expected.txt (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target-expected.txt (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target-expected.txt 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1,18 @@
+Tests that an observation is received after disconnecting a visible target
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS observations.length is 1
+PASS observations[0].target.tagName is "DIV"
+PASS observations[0].target.foo is 1
+PASS observations[0].isIntersecting is true
+* Removing target from document
+PASS observations.length is 1
+PASS observations[0].target.tagName is "DIV"
+PASS observations[0].target.foo is 1
+PASS observations[0].isIntersecting is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target.html (0 => 288234)
--- releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target.html (rev 0)
+++ releases/WebKitGTK/webkit-2.34/LayoutTests/intersection-observer/observe-then-disconnect-target.html 2022-01-19 20:43:30 UTC (rev 288234)
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<div id="testDiv">test</div>
+<script>
+ description("Tests that an observation is received after disconnecting a visible target");
+ jsTestIsAsync = true;
+
+ let receivedInitialObservation = false;
+ _onload_ = () => {
+ let intersectionObserver = new IntersectionObserver((_observations) => {
+ gc();
+ observations = _observations;
+
+ shouldBe("observations.length", "1");
+ shouldBeEqualToString("observations[0].target.tagName", "DIV");
+ shouldBe("observations[0].target.foo", "1");
+
+ if (!receivedInitialObservation) {
+ receivedInitialObservation = true;
+ shouldBeTrue("observations[0].isIntersecting");
+ setTimeout(() => {
+ debug("* Removing target from document");
+ document.getElementById("testDiv").remove();
+ gc();
+ setTimeout(gc, 0);
+ }, 0);
+ } else {
+ shouldBeFalse("observations[0].isIntersecting");
+ setTimeout(finishJSTest, 100);
+ }
+ });
+ let div = document.getElementById("testDiv");
+ div.foo = 1;
+ intersectionObserver.observe(div);
+ div = null;
+ gc();
+ setTimeout(gc, 0);
+ };
+</script>
+</body>
+</html>
Modified: releases/WebKitGTK/webkit-2.34/Source/WebCore/ChangeLog (288233 => 288234)
--- releases/WebKitGTK/webkit-2.34/Source/WebCore/ChangeLog 2022-01-19 20:42:12 UTC (rev 288233)
+++ releases/WebKitGTK/webkit-2.34/Source/WebCore/ChangeLog 2022-01-19 20:43:30 UTC (rev 288234)
@@ -1,3 +1,38 @@
+2021-10-05 Chris Dumez <[email protected]>
+
+ ASSERT(m_callback->hasCallback()) under IntersectionObserver::notify()
+ https://bugs.webkit.org/show_bug.cgi?id=231235
+ <rdar://80837616>
+
+ Reviewed by Ryosuke Niwa.
+
+ IntersectionObserver's JS callback stays alive as long as its JS wrapper and
+ its JS wrapper's lifetime relies on the IntersectionObserver::isReachableFromOpaqueRoots()
+ implementation. isReachableFromOpaqueRoots() keeps the wrapper alive as long
+ as the JS wrappers of observation / pending targets are alive. However, as per specification,
+ we always need to dispatch an observation for an observation target, even if that target
+ is not connected. Our code was already taking care of dispatching such observation. However,
+ there was nothing keeping the observation target alive in this case and thus nothing keeping
+ the JS callback alive either.
+
+ To address the issue, I am introducing a new m_targetsWaitingForFirstObservation data member
+ which holds a strong ref to the observation target until the next time we call notify().
+ This makes sure that the observation target (and its JS wrapper) stays alive long enough for
+ us to dispatch the first observation for it. I also updated isReachableFromOpaqueRoots() to
+ return true as long as m_targetsWaitingForFirstObservation is non-empty so that the
+ IntersectionObserver's JS wrapper (and thus the JS callback) stay alive long enough too.
+
+ Tests: intersection-observer/observe-disconnected-target-crash.html
+ intersection-observer/observe-disconnected-target.html
+
+ * page/IntersectionObserver.cpp:
+ (WebCore::IntersectionObserver::observe):
+ (WebCore::IntersectionObserver::unobserve):
+ (WebCore::IntersectionObserver::removeAllTargets):
+ (WebCore::IntersectionObserver::notify):
+ (WebCore::IntersectionObserver::isReachableFromOpaqueRoots const):
+ * page/IntersectionObserver.h:
+
2021-09-28 Alicia Boya García <[email protected]>
[MSE][GStreamer] Don't create MediaSourceTrackGStreamer objects twice for the same track
Modified: releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.cpp (288233 => 288234)
--- releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.cpp 2022-01-19 20:42:12 UTC (rev 288233)
+++ releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.cpp 2022-01-19 20:43:30 UTC (rev 288234)
@@ -170,6 +170,11 @@
bool hadObservationTargets = hasObservationTargets();
m_observationTargets.append(makeWeakPtr(target));
+ // 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 IntersectionObserver's JS wrapper alive via
+ // isReachableFromOpaqueRoots(), so the callback stays alive.
+ m_targetsWaitingForFirstObservation.append(target);
+
auto* document = trackingDocument();
if (!hadObservationTargets)
document->addIntersectionObserver(*this);
@@ -183,6 +188,7 @@
bool removed = m_observationTargets.removeFirst(&target);
ASSERT_UNUSED(removed, removed);
+ m_targetsWaitingForFirstObservation.removeFirstMatching([&](auto& pendingTarget) { return pendingTarget.ptr() == ⌖ });
if (!hasObservationTargets()) {
if (auto* document = trackingDocument())
@@ -208,6 +214,7 @@
void IntersectionObserver::targetDestroyed(Element& target)
{
m_observationTargets.removeFirst(&target);
+ m_targetsWaitingForFirstObservation.removeFirstMatching([&](auto& pendingTarget) { return pendingTarget.ptr() == ⌖ });
if (!hasObservationTargets()) {
if (auto* document = trackingDocument())
document->removeIntersectionObserver(*this);
@@ -233,6 +240,7 @@
ASSERT_UNUSED(removed, removed);
}
m_observationTargets.clear();
+ m_targetsWaitingForFirstObservation.clear();
}
void IntersectionObserver::rootDestroyed()
@@ -274,6 +282,7 @@
}
auto takenRecords = takeRecords();
+ auto targetsWaitingForFirstObservation = std::exchange(m_targetsWaitingForFirstObservation, { });
// FIXME: The JSIntersectionObserver wrapper should be kept alive as long as the intersection observer can fire events.
ASSERT(m_callback->hasCallback());
@@ -299,7 +308,7 @@
if (visitor.containsOpaqueRoot(target->opaqueRoot()))
return true;
}
- return false;
+ return !m_targetsWaitingForFirstObservation.isEmpty();
}
} // namespace WebCore
Modified: releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.h (288233 => 288234)
--- releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.h 2022-01-19 20:42:12 UTC (rev 288233)
+++ releases/WebKitGTK/webkit-2.34/Source/WebCore/page/IntersectionObserver.h 2022-01-19 20:43:30 UTC (rev 288234)
@@ -122,6 +122,7 @@
Vector<WeakPtr<Element>> m_observationTargets;
Vector<GCReachableRef<Element>> m_pendingTargets;
Vector<Ref<IntersectionObserverEntry>> m_queuedEntries;
+ Vector<GCReachableRef<Element>> m_targetsWaitingForFirstObservation;
};
