Title: [291030] trunk
- Revision
- 291030
- Author
- [email protected]
- Date
- 2022-03-08 22:13:25 -0800 (Tue, 08 Mar 2022)
Log Message
IntersectionObserver is causing massive document leaks on haaretz.co.il
https://bugs.webkit.org/show_bug.cgi?id=237619
<rdar://problem/89989815>
Reviewed by Simon Fraser.
Source/WebCore:
On haaretz.co.il, many of the iframe documents (for Google ad frames) were leaking due to
IntersectionObserver. In particular, IntersectionObserver::isReachableFromOpaqueRoots()
was returning true because m_targetsWaitingForFirstObservation was non-empty. This indicates
that the IntersectionObserver is waiting for its first observation since that target was
added. However, the observation is not coming because we navigated away from the document.
To address the issue, I updated Document::commonTeardown() to disconnect all
IntersectionObservers (and the very similar ResizeObservers), right after stopping all
ActiveDOMObject.
Test: fast/dom/intersection-observer-document-leak.html
* dom/Document.cpp:
(WebCore::Document::commonTeardown):
* dom/Document.h:
* page/IntersectionObserver.cpp:
(WebCore::IntersectionObserver::disconnect):
No longer return early if hasObservationTargets() returns false. This is because
LayoutTests:
Add layout test coverage.
* fast/dom/intersection-observer-document-leak-expected.txt: Added.
* fast/dom/intersection-observer-document-leak.html: Added.
* fast/dom/resources/intersection-observer-document-leak-popup.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (291029 => 291030)
--- trunk/LayoutTests/ChangeLog 2022-03-09 03:34:18 UTC (rev 291029)
+++ trunk/LayoutTests/ChangeLog 2022-03-09 06:13:25 UTC (rev 291030)
@@ -1,3 +1,17 @@
+2022-03-08 Chris Dumez <[email protected]>
+
+ IntersectionObserver is causing massive document leaks on haaretz.co.il
+ https://bugs.webkit.org/show_bug.cgi?id=237619
+ <rdar://problem/89989815>
+
+ Reviewed by Simon Fraser.
+
+ Add layout test coverage.
+
+ * fast/dom/intersection-observer-document-leak-expected.txt: Added.
+ * fast/dom/intersection-observer-document-leak.html: Added.
+ * fast/dom/resources/intersection-observer-document-leak-popup.html: Added.
+
2022-03-08 Eric Carlson <[email protected]>
[Cocoa] metadata cue endTime may not be updated
Added: trunk/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt (0 => 291030)
--- trunk/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt 2022-03-09 06:13:25 UTC (rev 291030)
@@ -0,0 +1,12 @@
+main frame - has 1 onunload handler(s)
+Tests that IntersectionObserver doesn't cause Document leaks.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.isDocumentAlive(frameDocumentID) is true
+PASS The iframe document didn't leak.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/intersection-observer-document-leak.html (0 => 291030)
--- trunk/LayoutTests/fast/dom/intersection-observer-document-leak.html (rev 0)
+++ trunk/LayoutTests/fast/dom/intersection-observer-document-leak.html 2022-03-09 06:13:25 UTC (rev 291030)
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+description("Tests that IntersectionObserver doesn't cause Document leaks.");
+jsTestIsAsync = true;
+
+frameDocumentID = 0;
+checkCount = 0;
+
+_onload_ = () => {
+ window.open("resources/intersection-observer-document-leak-popup.html");
+};
+
+function popupDocumentWasCreated(frameDocument)
+{
+ if (!window.internals)
+ return;
+
+ frameDocumentID = internals.documentIdentifier(frameDocument);
+ shouldBeTrue("internals.isDocumentAlive(frameDocumentID)");
+ handle = setInterval(() => {
+ gc();
+ if (!internals.isDocumentAlive(frameDocumentID)) {
+ clearInterval(handle);
+ testPassed("The iframe document didn't leak.");
+ finishJSTest();
+ }
+ checkCount++;
+ if (checkCount > 500) {
+ clearInterval(handle);
+ testFailed("The iframe document leaked.");
+ finishJSTest();
+ }
+ }, 10);
+}
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html (0 => 291030)
--- trunk/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html (rev 0)
+++ trunk/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html 2022-03-09 06:13:25 UTC (rev 291030)
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+function handleIntersect(entries, observer)
+{
+}
+
+_onunload_ = () => {
+ if (!window.IntersectionObserver)
+ return;
+
+ observer = new IntersectionObserver(handleIntersect);
+ observer.observe(document.body);
+};
+
+_onload_ = () => {
+ opener.popupDocumentWasCreated(document);
+ close();
+};
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (291029 => 291030)
--- trunk/Source/WebCore/ChangeLog 2022-03-09 03:34:18 UTC (rev 291029)
+++ trunk/Source/WebCore/ChangeLog 2022-03-09 06:13:25 UTC (rev 291030)
@@ -1,3 +1,30 @@
+2022-03-08 Chris Dumez <[email protected]>
+
+ IntersectionObserver is causing massive document leaks on haaretz.co.il
+ https://bugs.webkit.org/show_bug.cgi?id=237619
+ <rdar://problem/89989815>
+
+ Reviewed by Simon Fraser.
+
+ On haaretz.co.il, many of the iframe documents (for Google ad frames) were leaking due to
+ IntersectionObserver. In particular, IntersectionObserver::isReachableFromOpaqueRoots()
+ was returning true because m_targetsWaitingForFirstObservation was non-empty. This indicates
+ that the IntersectionObserver is waiting for its first observation since that target was
+ added. However, the observation is not coming because we navigated away from the document.
+
+ To address the issue, I updated Document::commonTeardown() to disconnect all
+ IntersectionObservers (and the very similar ResizeObservers), right after stopping all
+ ActiveDOMObject.
+
+ Test: fast/dom/intersection-observer-document-leak.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::commonTeardown):
+ * dom/Document.h:
+ * page/IntersectionObserver.cpp:
+ (WebCore::IntersectionObserver::disconnect):
+ No longer return early if hasObservationTargets() returns false. This is because
+
2022-03-08 Eric Carlson <[email protected]>
[Cocoa] metadata cue endTime may not be updated
Modified: trunk/Source/WebCore/dom/Document.cpp (291029 => 291030)
--- trunk/Source/WebCore/dom/Document.cpp 2022-03-09 03:34:18 UTC (rev 291029)
+++ trunk/Source/WebCore/dom/Document.cpp 2022-03-09 06:13:25 UTC (rev 291030)
@@ -818,6 +818,18 @@
m_documentFragmentForInnerOuterHTML = nullptr;
+ auto intersectionObservers = m_intersectionObservers;
+ for (auto& intersectionObserver : intersectionObservers) {
+ if (intersectionObserver)
+ intersectionObserver->disconnect();
+ }
+
+ auto resizeObservers = m_resizeObservers;
+ for (auto& resizeObserver : resizeObservers) {
+ if (resizeObserver)
+ resizeObserver->disconnect();
+ }
+
if (m_highlightRegister)
m_highlightRegister->clear();
if (m_fragmentHighlightRegister)
Modified: trunk/Source/WebCore/dom/Document.h (291029 => 291030)
--- trunk/Source/WebCore/dom/Document.h 2022-03-09 03:34:18 UTC (rev 291029)
+++ trunk/Source/WebCore/dom/Document.h 2022-03-09 06:13:25 UTC (rev 291030)
@@ -1969,7 +1969,6 @@
WeakHashSet<HTMLImageElement> m_dynamicMediaQueryDependentImages;
Vector<WeakPtr<IntersectionObserver>> m_intersectionObservers;
- Vector<WeakPtr<IntersectionObserver>> m_intersectionObserversWithPendingNotifications;
Timer m_intersectionObserversInitialUpdateTimer;
// This is only non-null when this document is an explicit root.
std::unique_ptr<IntersectionObserverData> m_intersectionObserverData;
Modified: trunk/Source/WebCore/page/IntersectionObserver.cpp (291029 => 291030)
--- trunk/Source/WebCore/page/IntersectionObserver.cpp 2022-03-09 03:34:18 UTC (rev 291029)
+++ trunk/Source/WebCore/page/IntersectionObserver.cpp 2022-03-09 06:13:25 UTC (rev 291030)
@@ -197,8 +197,10 @@
void IntersectionObserver::disconnect()
{
- if (!hasObservationTargets())
+ if (!hasObservationTargets()) {
+ ASSERT(m_targetsWaitingForFirstObservation.isEmpty());
return;
+ }
removeAllTargets();
if (auto* document = trackingDocument())
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
