Title: [291659] branches/safari-613-branch
Revision
291659
Author
alanc...@apple.com
Date
2022-03-22 10:56:17 -0700 (Tue, 22 Mar 2022)

Log Message

Cherry-pick r291030. rdar://problem/89989815

    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.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291030 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-613-branch/LayoutTests/ChangeLog (291658 => 291659)


--- branches/safari-613-branch/LayoutTests/ChangeLog	2022-03-22 17:56:11 UTC (rev 291658)
+++ branches/safari-613-branch/LayoutTests/ChangeLog	2022-03-22 17:56:17 UTC (rev 291659)
@@ -1,5 +1,61 @@
 2022-03-21  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r291030. rdar://problem/89989815
+
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291030 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-03-08  Chris Dumez  <cdu...@apple.com>
+
+            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-21  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r291029. rdar://problem/88690874
 
     [Cocoa] metadata cue endTime may not be updated

Added: branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt (0 => 291659)


--- branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak-expected.txt	2022-03-22 17:56:17 UTC (rev 291659)
@@ -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: branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak.html (0 => 291659)


--- branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak.html	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/fast/dom/intersection-observer-document-leak.html	2022-03-22 17:56:17 UTC (rev 291659)
@@ -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: branches/safari-613-branch/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html (0 => 291659)


--- branches/safari-613-branch/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html	                        (rev 0)
+++ branches/safari-613-branch/LayoutTests/fast/dom/resources/intersection-observer-document-leak-popup.html	2022-03-22 17:56:17 UTC (rev 291659)
@@ -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: branches/safari-613-branch/Source/WebCore/ChangeLog (291658 => 291659)


--- branches/safari-613-branch/Source/WebCore/ChangeLog	2022-03-22 17:56:11 UTC (rev 291658)
+++ branches/safari-613-branch/Source/WebCore/ChangeLog	2022-03-22 17:56:17 UTC (rev 291659)
@@ -1,5 +1,74 @@
 2022-03-21  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r291030. rdar://problem/89989815
+
+    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.
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291030 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2022-03-08  Chris Dumez  <cdu...@apple.com>
+
+            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-21  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r291029. rdar://problem/88690874
 
     [Cocoa] metadata cue endTime may not be updated

Modified: branches/safari-613-branch/Source/WebCore/dom/Document.cpp (291658 => 291659)


--- branches/safari-613-branch/Source/WebCore/dom/Document.cpp	2022-03-22 17:56:11 UTC (rev 291658)
+++ branches/safari-613-branch/Source/WebCore/dom/Document.cpp	2022-03-22 17:56:17 UTC (rev 291659)
@@ -812,6 +812,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 ENABLE(APP_HIGHLIGHTS)

Modified: branches/safari-613-branch/Source/WebCore/dom/Document.h (291658 => 291659)


--- branches/safari-613-branch/Source/WebCore/dom/Document.h	2022-03-22 17:56:11 UTC (rev 291658)
+++ branches/safari-613-branch/Source/WebCore/dom/Document.h	2022-03-22 17:56:17 UTC (rev 291659)
@@ -1962,7 +1962,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: branches/safari-613-branch/Source/WebCore/page/IntersectionObserver.cpp (291658 => 291659)


--- branches/safari-613-branch/Source/WebCore/page/IntersectionObserver.cpp	2022-03-22 17:56:11 UTC (rev 291658)
+++ branches/safari-613-branch/Source/WebCore/page/IntersectionObserver.cpp	2022-03-22 17:56:17 UTC (rev 291659)
@@ -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
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to