commit bca6a17c8003e5706cd9431626529283bc9ea2b4
author cdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Fri Feb 28 16:47:08 2020 +0000
committer cdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Fri Feb 28 16:47:08 2020 +0000
tree 0858c35e3444c521b3af18ae6aa6124ef908dd66
parent 4c14b76131ea65c797ef98fa94bcccd820e06759

Retain cycle between CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
https://bugs.webkit.org/show_bug.cgi?id=196437
<rdar://problem/46598332>

Reviewed by Alex Christensen.

Break the reference cycle using a WeakPtr. The leak was reproducible by browsing CNN.com
and then navigating to about:blank (those objects would stay around, even after memory
pressure signal).

* css/CSSFontFace.cpp:
(WebCore::CSSFontFace::CSSFontFace):
(WebCore::CSSFontFace::fontLoadEventOccurred):
* css/CSSFontFace.h:


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

Source/WebCore/ChangeLog

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 22a3827..25dbdad 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,20 @@
+2020-02-28  Chris Dumez  <cdumez@apple.com>
+
+        Retain cycle between CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
+        https://bugs.webkit.org/show_bug.cgi?id=196437
+        <rdar://problem/46598332>
+
+        Reviewed by Alex Christensen.
+
+        Break the reference cycle using a WeakPtr. The leak was reproducible by browsing CNN.com
+        and then navigating to about:blank (those objects would stay around, even after memory
+        pressure signal).
+
+        * css/CSSFontFace.cpp:
+        (WebCore::CSSFontFace::CSSFontFace):
+        (WebCore::CSSFontFace::fontLoadEventOccurred):
+        * css/CSSFontFace.h:
+
 2020-02-28  Alberto Garcia  <berto@igalia.com>
 
         [SOUP] Unreviewed. Fix unused parameter warning

Source/WebCore/css/CSSFontFace.cpp

diff --git a/Source/WebCore/css/CSSFontFace.cpp b/Source/WebCore/css/CSSFontFace.cpp
index 824b4eb..3c5d0ad 100644
--- a/Source/WebCore/css/CSSFontFace.cpp
+++ b/Source/WebCore/css/CSSFontFace.cpp
@@ -92,7 +92,7 @@
 }
 
 CSSFontFace::CSSFontFace(CSSFontSelector* fontSelector, StyleRuleFontFace* cssConnection, FontFace* wrapper, bool isLocalFallback)
-    : m_fontSelector(fontSelector)
+    : m_fontSelector(makeWeakPtr(fontSelector))
     , m_cssConnection(cssConnection)
     , m_wrapper(makeWeakPtr(wrapper))
     , m_isLocalFallback(isLocalFallback)
@@ -346,8 +346,8 @@
     if (m_sourcesPopulated)
         pump(ExternalResourceDownloadPolicy::Forbid);
 
-    ASSERT(m_fontSelector);
-    m_fontSelector->fontLoaded();
+    if (m_fontSelector)
+        m_fontSelector->fontLoaded();
 
     iterateClients(m_clients, [&](Client& client) {
         client.fontLoaded(*this);

Source/WebCore/css/CSSFontFace.h

diff --git a/Source/WebCore/css/CSSFontFace.h b/Source/WebCore/css/CSSFontFace.h
index c2c3650..e3d14bd 100644
--- a/Source/WebCore/css/CSSFontFace.h
+++ b/Source/WebCore/css/CSSFontFace.h
@@ -184,7 +184,7 @@
     FontLoadingBehavior m_loadingBehavior { FontLoadingBehavior::Auto };
 
     Vector<std::unique_ptr<CSSFontFaceSource>, 0, CrashOnOverflow, 0> m_sources;
-    RefPtr<CSSFontSelector> m_fontSelector; // FIXME: https://bugs.webkit.org/show_bug.cgi?id=196437 There's a retain cycle: CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
+    WeakPtr<CSSFontSelector> m_fontSelector; // FIXME: Ideally this data member would go away (https://bugs.webkit.org/show_bug.cgi?id=208351).
     RefPtr<StyleRuleFontFace> m_cssConnection;
     HashSet<Client*> m_clients;
     WeakPtr<FontFace> m_wrapper;