Retain cycle between CSSFontSelector -> CSSFontFaceSet -> CSSFontFace -> CSSFontSelector
Created attachment 366386 [details] WIP
Created attachment 366387 [details] Dependency graph
CSSFontFace::m_fontSelector is used for 4 purposes: 1) When a font loads, it calls CSSFontSelector::fontLoaded(), so the font selector can dispatchInvalidationCallbacks(). 2) AllowsUserInstalleFonts, FontLoadTimingOverride, and ShouldIgnoreFontLoadCompletions are queried from the document's settings 3) Starting the font load requires the document->cachedResourceLoader() 4) The FontFace object's getters need to updateStyleIfNeeded() to pull down any CSSOM modifications that may have happened in the past.
The reason we can't use a simple Client callback for 3) is that load() should still work even if the FontFace isn't attached to any FontFaceSet. It needs to issue a network request even if there is no Document involved. XHR does this via a ThreadableLoader. We should figure out whether we can use ThreadableLoader too.
Regressions: Unexpected text-only failures (2) fast/css/font-face-multiple-remote-sources.html [ Failure ] svg/batik/text/xmlSpace.svg [ Failure ] Regressions: Unexpected crashes (1) fast/text/font-face-set-cssom.html [ Crash ]
fast/text/font-face-set-cssom.html: 0 com.apple.JavaScriptCore 0x00000001688d7fd0 WTFCrash + 16 (Assertions.cpp:305) 1 com.apple.WebCore 0x000000015000c0cb WTFCrashWithInfo(int, char const*, char const*, int) + 27 2 com.apple.WebCore 0x00000001522d7219 WTF::HashTableConstIterator<WebCore::CSSFontFace::Client*, WebCore::CSSFontFace::Client*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*> >::checkValidity() const + 105 (HashTable.h:213) 3 com.apple.WebCore 0x00000001522d7389 WTF::HashTableConstIterator<WebCore::CSSFontFace::Client*, WebCore::CSSFontFace::Client*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*> >::operator++() + 25 (HashTable.h:181) 4 com.apple.WebCore 0x00000001522d6733 WTF::HashTableConstIteratorAdapter<WTF::HashTable<WebCore::CSSFontFace::Client*, WebCore::CSSFontFace::Client*, WTF::IdentityExtractor, WTF::PtrHash<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*> >, WebCore::CSSFontFace::Client*>::operator++() + 35 (HashTable.h:1506) 5 com.apple.WebCore 0x00000001522c99de void WebCore::iterateClients<WebCore::CSSFontFace::updateStyleIfNeeded()::$_29>(WTF::HashSet<WebCore::CSSFontFace::Client*, WTF::PtrHash<WebCore::CSSFontFace::Client*>, WTF::HashTraits<WebCore::CSSFontFace::Client*> >&, WebCore::CSSFontFace::updateStyleIfNeeded()::$_29) + 382 (CSSFontFace.cpp:58) 6 com.apple.WebCore 0x00000001522c9859 WebCore::CSSFontFace::updateStyleIfNeeded() + 25 (CSSFontFace.cpp:803) 7 com.apple.WebCore 0x000000015239c866 WebCore::FontFace::family() const + 54 (FontFace.cpp:301)
Comment on attachment 366386 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=366386&action=review > Source/WebCore/css/CSSFontFaceSet.h:124 > - HashSet<CSSFontFaceSetClient*> m_clients; > + HashSet<Client*> m_clients; It's not great that all these Client interfaces rely on implementations to remove themselves. At minimum, ~Client should delete itself from CSSFontFaceSet. But we should just try making Client inherit from CanMakeWeakPtr and then just use WeakPtr<Client> here instead.
Maybe <rdar://problem/46598332> ?
<rdar://problem/46598332>
Created attachment 391930 [details] Patch
Created attachment 391938 [details] Patch
Comment on attachment 391938 [details] Patch Clearing flags on attachment: 391938 Committed r257639: <https://trac.webkit.org/changeset/257639>
All reviewed patches have been landed. Closing bug.
Thanks for the fix!